fix: crash when using vm after window.open()

electron · Mar 6, 2023 · 8d1c375 · 8d1c375
1 parent 829fb4f
commit 8d1c375
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 2 deletions.
diff --git a/shell/renderer/electron_render_frame_observer.cc b/shell/renderer/electron_render_frame_observer.cc
@@ -63,6 +63,7 @@ void ElectronRenderFrameObserver::DidClearWindowObject() {
   // Check DidInstallConditionalFeatures below for the background.
   auto* web_frame =
       static_cast<blink::WebLocalFrameImpl*>(render_frame_->GetWebFrame());
+  GURL url = web_frame->GetDocument().Url();
   if (has_delayed_node_initialization_ && web_frame->Opener() &&
       !web_frame->IsOnInitialEmptyDocument()) {
     v8::Isolate* isolate = blink::MainThreadIsolate();
@@ -96,15 +97,19 @@ void ElectronRenderFrameObserver::DidInstallConditionalFeatures(
   // actual page has started to load.
   auto* web_frame =
       static_cast<blink::WebLocalFrameImpl*>(render_frame_->GetWebFrame());
-  if (web_frame->Opener() && web_frame->IsOnInitialEmptyDocument()) {
+  if (web_frame->Opener()) {
     // FIXME(zcbenz): Chromium does not do any browser side navigation for
     // window.open('about:blank'), so there is no way to override WebPreferences
     // of it. We should not delay Node.js initialization as there will be no
     // further loadings.
     // Please check http://crbug.com/1215096 for updates which may help remove
     // this hack.
     GURL url = web_frame->GetDocument().Url();
-    if (!url.IsAboutBlank()) {
+    bool is_only_initially_blank =
+        web_frame->IsOnInitialEmptyDocument() && !url.IsAboutBlank();
+    bool is_intentionally_blank =
+        !web_frame->IsOnInitialEmptyDocument() && url.IsAboutBlank();
+    if (is_only_initially_blank || is_intentionally_blank) {
       has_delayed_node_initialization_ = true;
       return;
     }

diff --git a/spec/guest-window-manager-spec.ts b/spec/guest-window-manager-spec.ts
@@ -176,6 +176,35 @@ describe('webContents.setWindowOpenHandler', () => {
     await once(browserWindow.webContents, 'did-create-window');
   });
 
+  it('does not crash when used in conjunction with the vm module', async () => {
+    const w = new BrowserWindow({
+      show: false,
+      webPreferences: {
+        contextIsolation: false,
+        nodeIntegration: true
+      }
+    });
+
+    await w.loadURL('about:blank');
+
+    const didCreateWindow = once(w.webContents, 'did-create-window');
+    w.webContents.executeJavaScript("window.open('')");
+    await didCreateWindow;
+
+    const result = await w.webContents.executeJavaScript(`
+      const vm = require('node:vm')
+      const run = () => {
+          const context = { x: 2 }
+          vm.createContext(context)
+          vm.runInContext('x += 40', context)
+          return context.x
+      }
+      run()
+    `);
+
+    expect(result).to.equal(42);
+  });
+
   it('can change webPreferences of child windows', async () => {
     browserWindow.webContents.setWindowOpenHandler(() => ({ action: 'allow', overrideBrowserWindowOptions: { webPreferences: { defaultFontSize: 30 } } }));