diff --git a/patches/node/.patches b/patches/node/.patches index 7d119718f66fb..de5f8e0afdee4 100644 --- a/patches/node/.patches +++ b/patches/node/.patches @@ -39,3 +39,4 @@ chore_split_createenvironment_into_createenvironment_and.patch chore_handle_default_configuration_not_being_set_in_the_electron_env.patch revert_crypto_add_outputlength_option_to_crypto_createhash.patch add_openssl_is_boringssl_guard_to_oaep_hash_check.patch +fix_microtasks.patch diff --git a/patches/node/fix_microtasks.patch b/patches/node/fix_microtasks.patch new file mode 100644 index 0000000000000..c48c63dd852c3 --- /dev/null +++ b/patches/node/fix_microtasks.patch @@ -0,0 +1,58 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Jeremy Apthorp +Date: Mon, 16 Sep 2019 17:50:28 -0400 +Subject: fix microtasks + +backports https://github.com/nodejs/node/pull/29581 and https://github.com/nodejs/node/pull/29434 + +diff --git a/src/api/callback.cc b/src/api/callback.cc +index 43ccfafd9f2c85e23a9ea6277e88e4864e287905..3c518870c9c8d92f3dfcd6c270f5e023e3b69633 100644 +--- a/src/api/callback.cc ++++ b/src/api/callback.cc +@@ -12,6 +12,7 @@ using v8::HandleScope; + using v8::Isolate; + using v8::Local; + using v8::MaybeLocal; ++using v8::MicrotasksScope; + using v8::NewStringType; + using v8::Object; + using v8::String; +@@ -100,7 +101,7 @@ void InternalCallbackScope::Close() { + + if (!env_->can_call_into_js()) return; + if (!tick_info->has_tick_scheduled()) { +- env_->isolate()->RunMicrotasks(); ++ MicrotasksScope::PerformCheckpoint(env_->isolate()); + } + + #if 0 // FIXME(codebytere): figure out why this check fails/causes crash +diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc +index e6b4d0b8e211cdb1fef4759457c2550e28448360..918796ba77d80cf66324164a930f8068e0622ccb 100644 +--- a/src/node_task_queue.cc ++++ b/src/node_task_queue.cc +@@ -21,6 +21,7 @@ using v8::kPromiseRejectWithNoHandler; + using v8::kPromiseResolveAfterResolved; + using v8::Local; + using v8::Message; ++using v8::MicrotasksScope; + using v8::Number; + using v8::Object; + using v8::Promise; +@@ -43,7 +44,7 @@ static void EnqueueMicrotask(const FunctionCallbackInfo& args) { + bool RunNextTicksNative(Environment* env) { + TickInfo* tick_info = env->tick_info(); + if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn()) +- env->isolate()->RunMicrotasks(); ++ MicrotasksScope::PerformCheckpoint(env->isolate()); + if (!tick_info->has_tick_scheduled() && !tick_info->has_rejection_to_warn()) + return true; + +@@ -54,7 +55,7 @@ bool RunNextTicksNative(Environment* env) { + } + + static void RunMicrotasks(const FunctionCallbackInfo& args) { +- args.GetIsolate()->RunMicrotasks(); ++ MicrotasksScope::PerformCheckpoint(args.GetIsolate()); + } + + static void SetTickCallback(const FunctionCallbackInfo& args) { diff --git a/shell/browser/net/node_stream_loader.cc b/shell/browser/net/node_stream_loader.cc index cc737b68ca476..8c07c696cdc2d 100644 --- a/shell/browser/net/node_stream_loader.cc +++ b/shell/browser/net/node_stream_loader.cc @@ -91,10 +91,18 @@ void NodeStreamLoader::NotifyComplete(int result) { } void NodeStreamLoader::ReadMore() { + if (is_reading_) { + // Calling read() can trigger the "readable" event again, making this + // function re-entrant. If we're already reading, we don't want to start + // a nested read, so short-circuit. + return; + } is_reading_ = true; + auto weak = weak_factory_.GetWeakPtr(); // buffer = emitter.read() v8::MaybeLocal ret = node::MakeCallback( isolate_, emitter_.Get(isolate_), "read", 0, nullptr, {0, 0}); + DCHECK(weak) << "We shouldn't have been destroyed when calling read()"; // If there is no buffer read, wait until |readable| is emitted again. v8::Local buffer; @@ -110,13 +118,12 @@ void NodeStreamLoader::ReadMore() { // Write buffer to mojo pipe asyncronously. is_reading_ = false; is_writing_ = true; - producer_->Write( - std::make_unique( - base::StringPiece(node::Buffer::Data(buffer), - node::Buffer::Length(buffer)), - mojo::StringDataSource::AsyncWritingMode:: - STRING_STAYS_VALID_UNTIL_COMPLETION), - base::BindOnce(&NodeStreamLoader::DidWrite, weak_factory_.GetWeakPtr())); + producer_->Write(std::make_unique( + base::StringPiece(node::Buffer::Data(buffer), + node::Buffer::Length(buffer)), + mojo::StringDataSource::AsyncWritingMode:: + STRING_STAYS_VALID_UNTIL_COMPLETION), + base::BindOnce(&NodeStreamLoader::DidWrite, weak)); } void NodeStreamLoader::DidWrite(MojoResult result) { diff --git a/spec-main/api-session-spec.ts b/spec-main/api-session-spec.ts index eecc02f53a299..f22566bcc4ff7 100644 --- a/spec-main/api-session-spec.ts +++ b/spec-main/api-session-spec.ts @@ -240,17 +240,16 @@ describe('session module', () => { const port = (downloadServer.address() as AddressInfo).port const url = `http://127.0.0.1:${port}/` - const downloadPrevented: Promise = new Promise(resolve => { + const downloadPrevented: Promise<{itemUrl: string, itemFilename: string, item: Electron.DownloadItem}> = new Promise(resolve => { w.webContents.session.once('will-download', function (e, item) { e.preventDefault() - resolve(item) + resolve({itemUrl: item.getURL(), itemFilename: item.getFilename(), item}) }) }) w.loadURL(url) - const item = await downloadPrevented - expect(item.getURL()).to.equal(url) - expect(item.getFilename()).to.equal('mockFile.txt') - await new Promise(setImmediate) + const {item, itemUrl, itemFilename} = await downloadPrevented + expect(itemUrl).to.equal(url) + expect(itemFilename).to.equal('mockFile.txt') expect(() => item.getURL()).to.throw('Object has been destroyed') }) }) diff --git a/spec-main/api-web-contents-spec.ts b/spec-main/api-web-contents-spec.ts index 35f89fd0dc182..d20039b10deb4 100644 --- a/spec-main/api-web-contents-spec.ts +++ b/spec-main/api-web-contents-spec.ts @@ -359,7 +359,6 @@ describe('webContents module', () => { // For some reason we have to wait for two focused events...? await emittedOnce(w.webContents, 'devtools-focused') - await emittedOnce(w.webContents, 'devtools-focused') expect(() => { webContents.getFocusedWebContents() }).to.not.throw() @@ -444,9 +443,10 @@ describe('webContents module', () => { await focused expect(w.isFocused()).to.be.true() w.webContents.openDevTools({ mode: 'detach', activate: true }) - await emittedOnce(w.webContents, 'devtools-focused') - await emittedOnce(w.webContents, 'devtools-focused') - await emittedOnce(w.webContents, 'devtools-opened') + await Promise.all([ + emittedOnce(w.webContents, 'devtools-opened'), + emittedOnce(w.webContents, 'devtools-focused'), + ]) await new Promise(resolve => setTimeout(resolve, 0)) expect(w.isFocused()).to.be.false() }) @@ -572,8 +572,6 @@ describe('webContents module', () => { const [, zoomDirection] = await emittedOnce(w.webContents, 'zoom-changed') expect(zoomDirection).to.equal(zoomingIn ? 'in' : 'out') - // Apparently we get two zoom-changed events?? - await emittedOnce(w.webContents, 'zoom-changed') } await testZoomChanged({ zoomingIn: true })