Skip to content

Commit

Permalink
fix: crash when using vm after window.open()
Browse files Browse the repository at this point in the history
  • Loading branch information
@codebytere
codebytere committed Mar 6, 2023
1 parent 829fb4f commit 0ff6c2a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 2 deletions.
8 changes: 6 additions & 2 deletions shell/renderer/electron_render_frame_observer.cc
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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({

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

0 comments on commit 0ff6c2a

Please sign in to comment.