diff --git a/atom/browser/native_window_mac.mm b/atom/browser/native_window_mac.mm index 70f7f88384e78..c6e0acaea21e9 100644 --- a/atom/browser/native_window_mac.mm +++ b/atom/browser/native_window_mac.mm @@ -480,7 +480,8 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { } NativeWindowMac::~NativeWindowMac() { - [NSEvent removeMonitor:wheel_event_monitor_]; + if (wheel_event_monitor_) + [NSEvent removeMonitor:wheel_event_monitor_]; } void NativeWindowMac::SetContentView(views::View* view) { @@ -517,6 +518,13 @@ void ViewDidMoveToSuperview(NSView* self, SEL _cmd) { } void NativeWindowMac::CloseImmediately() { + // Remove event monitor before destroying window, otherwise the monitor may + // call its callback after window has been destroyed. + if (wheel_event_monitor_) { + [NSEvent removeMonitor:wheel_event_monitor_]; + wheel_event_monitor_ = nil; + } + // Retain the child window before closing it. If the last reference to the // NSWindow goes away inside -[NSWindow close], then bad stuff can happen. // See e.g. http://crbug.com/616701. diff --git a/lib/browser/api/browser-window.js b/lib/browser/api/browser-window.js index a957eb390d3d0..3694acd119c99 100644 --- a/lib/browser/api/browser-window.js +++ b/lib/browser/api/browser-window.js @@ -3,7 +3,6 @@ const electron = require('electron') const { WebContentsView, TopLevelWindow } = electron const { BrowserWindow } = process.atomBinding('window') -const ipcMain = require('@electron/internal/browser/ipc-main-internal') Object.setPrototypeOf(BrowserWindow.prototype, TopLevelWindow.prototype) @@ -26,43 +25,6 @@ BrowserWindow.prototype._init = function () { nativeSetBounds.call(this, bounds, ...opts) } - // Make new windows requested by links behave like "window.open" - this.webContents.on('-new-window', (event, url, frameName, disposition, - additionalFeatures, postData, - referrer) => { - const options = { - show: true, - width: 800, - height: 600 - } - ipcMain.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN', - event, url, referrer, frameName, disposition, - options, additionalFeatures, postData) - }) - - // Create a new browser window for the native implementation of - // "window.open", used in sandbox and nativeWindowOpen mode - this.webContents.on('-add-new-contents', (event, webContents, disposition, - userGesture, left, top, width, height, url, frameName) => { - if ((disposition !== 'foreground-tab' && disposition !== 'new-window' && - disposition !== 'background-tab')) { - event.preventDefault() - return - } - - const options = { - show: true, - x: left, - y: top, - width: width || 800, - height: height || 600, - webContents: webContents - } - const referrer = { url: '', policy: 'default' } - ipcMain.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN', - event, url, referrer, frameName, disposition, options) - }) - // window.resizeTo(...) // window.moveTo(...) this.webContents.on('move', (event, size) => { diff --git a/lib/browser/api/web-contents.js b/lib/browser/api/web-contents.js index 1fdb177cb4f88..e94c2ac2d9d99 100644 --- a/lib/browser/api/web-contents.js +++ b/lib/browser/api/web-contents.js @@ -376,6 +376,46 @@ WebContents.prototype._init = function () { this.reload() }) + // Handle window.open for BrowserWindow and BrowserView. + if (['browserview', 'window'].includes(this.getType())) { + // Make new windows requested by links behave like "window.open". + this.on('-new-window', (event, url, frameName, disposition, + additionalFeatures, postData, + referrer) => { + const options = { + show: true, + width: 800, + height: 600 + } + ipcMainInternal.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN', + event, url, referrer, frameName, disposition, + options, additionalFeatures, postData) + }) + + // Create a new browser window for the native implementation of + // "window.open", used in sandbox and nativeWindowOpen mode. + this.on('-add-new-contents', (event, webContents, disposition, + userGesture, left, top, width, height, url, frameName) => { + if ((disposition !== 'foreground-tab' && disposition !== 'new-window' && + disposition !== 'background-tab')) { + event.preventDefault() + return + } + + const options = { + show: true, + x: left, + y: top, + width: width || 800, + height: height || 600, + webContents + } + const referrer = { url: '', policy: 'default' } + ipcMainInternal.emit('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN', + event, url, referrer, frameName, disposition, options) + }) + } + app.emit('web-contents-created', {}, this) } diff --git a/spec/api-browser-view-spec.js b/spec/api-browser-view-spec.js index 5c27132aed902..515d9f926d2c5 100644 --- a/spec/api-browser-view-spec.js +++ b/spec/api-browser-view-spec.js @@ -1,5 +1,6 @@ 'use strict' +const assert = require('assert') const chai = require('chai') const ChildProcess = require('child_process') const dirtyChai = require('dirty-chai') @@ -14,6 +15,8 @@ const { expect } = chai chai.use(dirtyChai) describe('BrowserView module', () => { + const fixtures = path.resolve(__dirname, 'fixtures') + let w = null let view = null @@ -178,11 +181,26 @@ describe('BrowserView module', () => { describe('new BrowserView()', () => { it('does not crash on exit', async () => { - const appPath = path.join(__dirname, 'fixtures', 'api', 'leak-exit-browserview.js') + const appPath = path.join(fixtures, 'api', 'leak-exit-browserview.js') const electronPath = remote.getGlobal('process').execPath const appProcess = ChildProcess.spawn(electronPath, [appPath]) const [code] = await emittedOnce(appProcess, 'close') expect(code).to.equal(0) }) }) + + describe('window.open()', () => { + it('works in BrowserView', (done) => { + view = new BrowserView() + w.setBrowserView(view) + view.webContents.once('new-window', (e, url, frameName, disposition, options, additionalFeatures) => { + e.preventDefault() + assert.strictEqual(url, 'http://host/') + assert.strictEqual(frameName, 'host') + assert.strictEqual(additionalFeatures[0], 'this-is-not-a-standard-feature') + done() + }) + view.webContents.loadFile(path.join(fixtures, 'pages', 'window-open.html')) + }) + }) })