Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions shell/renderer/electron_render_frame_observer.cc
Expand Up @@ -96,15 +96,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;
}
Expand Down
29 changes: 29 additions & 0 deletions spec/guest-window-manager-spec.ts
Expand Up @@ -176,6 +176,35 @@
await once(browserWindow.webContents, 'did-create-window');
});

it('does not crash when used in conjunction with the vm module', async () => {
const w = new BrowserWindow({

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 22-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 23-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 23-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 23-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 23-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 24-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 24-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 24-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()

Check failure on line 180 in spec/guest-window-manager-spec.ts

View check run for this annotation

trop / Backportable? - 24-x-y

spec/guest-window-manager-spec.ts#L180

Patch Conflict
Raw output
++<<<<<<< HEAD
 +  it('can change webPreferences of child windows', (done) => {
++=======
+   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 () => {
++>>>>>>> fix: crash when using vm after window.open()
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 } } }));

Expand Down