Home Explore 引导页 在线工具 Blog
Register Sign In
Pchen0
/
electron
1
0
Fork 0
Files Issues 0 Pull Requests 0 Wiki
Browse Source

fix: microtasks policy in CreateEnvironment (#29807)

* fix: microtasks policy in CreateEnvironment

Microtasks policy should not be updated for the renderer because
`NodeBindings::CreateEnvironment` might be entered with or without
`UvRunOnce()` on stack. One of the examples of such calls is
`window.open()` which is possible to invoke while `uv_run()` is still
running (e.g. with `setImmediate()`).

All in all, it doesn't matter that much which policy we use since
`v8::MicrotasksScope` has a check for the policy in its destructor and
no commits will be made if the policy is `kExplicit`. It is important,
however, to not change the policy in the middle of `UvRunOnce()` so we
should respect whatever we currently have and move on.

Fix: #29463

* Move test to a better place

* Update spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html

Co-authored-by: Jeremy Rose <[email protected]>

* Update spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html

Co-authored-by: Jeremy Rose <[email protected]>

* simplify crash-case

* comment

* fix comment

Co-authored-by: Fedor Indutny <[email protected]>
Co-authored-by: Fedor Indutny <[email protected]>
Co-authored-by: Jeremy Rose <[email protected]>
Co-authored-by: Fedor Indutny <[email protected]>
trop[bot] 3 years ago
parent
308470c98d
commit
845716019a
3 changed files with 49 additions and 3 deletions
Split View Show Diff Stats
  1. 7 3
      shell/common/node_bindings.cc
  2. 21 0
      spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html
  3. 21 0
      spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.js

+ 7 - 3
shell/common/node_bindings.cc
View File 

@@ -489,9 +489,13 @@ node::Environment* NodeBindings::CreateEnvironment(
 
     // Node.js requires that microtask checkpoints be explicitly invoked.
 
     is.policy = v8::MicrotasksPolicy::kExplicit;
 
   } else {
 
-    // Match Blink's behavior by allowing microtasks invocation to be controlled
 
-    // by MicrotasksScope objects.
 
-    is.policy = v8::MicrotasksPolicy::kScoped;
 
+    // Blink expects the microtasks policy to be kScoped, but Node.js expects it
 
+    // to be kExplicit. In the renderer, there can be many contexts within the
 
+    // same isolate, so we don't want to change the existing policy here, which
 
+    // could be either kExplicit or kScoped depending on whether we're executing
 
+    // from within a Node.js or a Blink entrypoint. Instead, the policy is
 
+    // toggled to kExplicit when entering Node.js through UvRunOnce.
 
+    is.policy = context->GetIsolate()->GetMicrotasksPolicy();
 
 
     // We do not want to use Node.js' message listener as it interferes with
 
     // Blink's.

+ 21 - 0
spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.html
View File 

@@ -0,0 +1,21 @@
 
+<html>
 
+<body>
 
+<script>
 
+// `setImmediate` schedules a function to be run on the next UV tick, which
 
+//  ensures that `window.open` is run from `UvRunOnce()`.
 
+setImmediate(async () => {
 
+  if (!location.hash) {
 
+    const p = new Promise(resolve => {
 
+      window.addEventListener('message', resolve, { once: true });
 
+    });
 
+    window.open('#foo', '', 'nodeIntegration=no,show=no');
 
+    const e = await p;
 
+
 
+    window.close();
 
+  } else {
 
+    window.opener.postMessage('foo', '*');
 
+  }
 
+});
 
+</script>
 
+</body>
 
+</html>

+ 21 - 0
spec-main/fixtures/crash-cases/setimmediate-window-open-crash/index.js
View File 

@@ -0,0 +1,21 @@
 
+const { app, BrowserWindow } = require('electron');
 
+
 
+function createWindow () {
 
+  const mainWindow = new BrowserWindow({
 
+    webPreferences: {
 
+      nodeIntegration: true,
 
+      contextIsolation: false,
 
+      nativeWindowOpen: true
 
+    }
 
+  });
 
+
 
+  mainWindow.on('close', () => {
 
+    app.quit();
 
+  });
 
+
 
+  mainWindow.loadFile('index.html');
 
+}
 
+
 
+app.whenReady().then(() => {
 
+  createWindow();
 
+});