From 25e1ed97b5e506d1663b5a70b54332d14d9ce177 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Fri, 7 Jul 2017 15:14:25 -0700 Subject: [PATCH 01/13] Use constants from content_switches.h --- atom/browser/atom_browser_client.cc | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 065909dc40fb..80532ab49d43 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -38,6 +38,7 @@ #include "content/public/browser/resource_dispatcher_host.h" #include "content/public/browser/site_instance.h" #include "content/public/browser/web_contents.h" +#include "content/public/common/content_switches.h" #include "content/public/common/url_constants.h" #include "content/public/common/web_preferences.h" #include "net/ssl/ssl_cert_request_info.h" @@ -241,8 +242,9 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation( void AtomBrowserClient::AppendExtraCommandLineSwitches( base::CommandLine* command_line, int process_id) { - std::string process_type = command_line->GetSwitchValueASCII("type"); - if (process_type != "renderer") + std::string process_type = + command_line->GetSwitchValueASCII(::switches::kProcessType); + if (process_type != ::switches::kRendererProcess) return; // Copy following switches to child process. From 553021bc9ca04dc612c1d5aabf4b25d759d4fa3a Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 15:44:07 -0700 Subject: [PATCH 02/13] Only assign opener when not using nativeWindowOpen --- lib/renderer/window-setup.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/renderer/window-setup.js b/lib/renderer/window-setup.js index 31767a290d4d..b94eace5aeca 100644 --- a/lib/renderer/window-setup.js +++ b/lib/renderer/window-setup.js @@ -127,6 +127,10 @@ module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNative return null } } + + if (openerId != null) { + window.opener = getOrCreateProxy(ipcRenderer, openerId) + } } window.alert = function (message, title) { @@ -142,10 +146,6 @@ module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNative throw new Error('prompt() is and will not be supported.') } - if (openerId != null) { - window.opener = getOrCreateProxy(ipcRenderer, openerId) - } - ipcRenderer.on('ELECTRON_GUEST_WINDOW_POSTMESSAGE', function (event, sourceId, message, sourceOrigin) { // Manually dispatch event instead of using postMessage because we also need to // set event.source. From 61167ca296344cc6a28697ba53212fb20dafaa8f Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 15:44:40 -0700 Subject: [PATCH 03/13] Inherit nativeWindowOpen from parent window --- lib/browser/guest-window-manager.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index 99bcacf5cb85..e2ec4a96a8c9 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -11,6 +11,7 @@ const frameToGuest = new Map() const inheritedWebPreferences = new Map([ ['contextIsolation', true], ['javascript', false], + ['nativeWindowOpen', true], ['nodeIntegration', false], ['webviewTag', false] ]) From 34c5abfe43cc7d4f76b879d24e3d25d32266f5f3 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 15:45:48 -0700 Subject: [PATCH 04/13] Update web preferences when creating with existing webContents --- atom/browser/api/atom_api_window.cc | 46 ++++++++++++++++++----------- 1 file changed, 28 insertions(+), 18 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index f48026ea9374..040c3ef78695 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -10,6 +10,7 @@ #include "atom/browser/api/atom_api_web_contents.h" #include "atom/browser/browser.h" #include "atom/browser/native_window.h" +#include "atom/browser/web_contents_preferences.h" #include "atom/common/native_mate_converters/callback.h" #include "atom/common/native_mate_converters/file_path_converter.h" #include "atom/common/native_mate_converters/gfx_converter.h" @@ -77,30 +78,39 @@ v8::Local ToBuffer(v8::Isolate* isolate, void* val, int size) { Window::Window(v8::Isolate* isolate, v8::Local wrapper, const mate::Dictionary& options) { mate::Handle web_contents; - // If no WebContents was passed to the constructor, create it from options. - if (!options.Get("webContents", &web_contents)) { - // Use options.webPreferences to create WebContents. - mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate); - options.Get(options::kWebPreferences, &web_preferences); - // Copy the backgroundColor to webContents. - v8::Local value; - if (options.Get(options::kBackgroundColor, &value)) - web_preferences.Set(options::kBackgroundColor, value); + // Use options.webPreferences in WebContents. + mate::Dictionary web_preferences = mate::Dictionary::CreateEmpty(isolate); + options.Get(options::kWebPreferences, &web_preferences); - v8::Local transparent; - if (options.Get("transparent", &transparent)) - web_preferences.Set("transparent", transparent); + // Copy the backgroundColor to webContents. + v8::Local value; + if (options.Get(options::kBackgroundColor, &value)) + web_preferences.Set(options::kBackgroundColor, value); + + v8::Local transparent; + if (options.Get("transparent", &transparent)) + web_preferences.Set("transparent", transparent); #if defined(ENABLE_OSR) - // Offscreen windows are always created frameless. - bool offscreen; - if (web_preferences.Get("offscreen", &offscreen) && offscreen) { - auto window_options = const_cast(options); - window_options.Set(options::kFrame, false); - } + // Offscreen windows are always created frameless. + bool offscreen; + if (web_preferences.Get("offscreen", &offscreen) && offscreen) { + auto window_options = const_cast(options); + window_options.Set(options::kFrame, false); + } #endif + if (options.Get("webContents", &web_contents)) { + // Set webPreferences from options if using existing webContents + auto* existing_preferences = + WebContentsPreferences::FromWebContents(web_contents->web_contents()); + base::DictionaryValue web_preferences_dict; + mate::ConvertFromV8(isolate, web_preferences.GetHandle(), + &web_preferences_dict); + existing_preferences->web_preferences()->Clear(); + existing_preferences->Merge(web_preferences_dict); + } else { // Creates the WebContents used by BrowserWindow. web_contents = WebContents::Create(isolate, web_preferences); } From 06e60e5d4d369fe587136f4ccabc381d46cccab5 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 15:48:12 -0700 Subject: [PATCH 05/13] Map pending process id to webContents for frame host initiating navigation --- atom/browser/atom_browser_client.cc | 28 +++++++++++----------------- atom/browser/atom_browser_client.h | 4 ++-- 2 files changed, 13 insertions(+), 19 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 80532ab49d43..61c97f2c35ce 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -79,9 +79,10 @@ AtomBrowserClient::~AtomBrowserClient() { content::WebContents* AtomBrowserClient::GetWebContentsFromProcessID( int process_id) { - // If the process is a pending process, we should use the old one. + // If the process is a pending process, we should use the web contents + // for the frame host passed into OverrideSiteInstanceForNavigation. if (base::ContainsKey(pending_processes_, process_id)) - process_id = pending_processes_[process_id]; + return pending_processes_[process_id]; // Certain render process will be created with no associated render view, // for example: ServiceWorker. @@ -231,12 +232,12 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation( content::BrowserThread::UI, FROM_HERE, base::Bind(&Noop, base::RetainedRef(site_instance))); - // Remember the original renderer process of the pending renderer process. - auto current_process = current_instance->GetProcess(); + // Remember the original web contents for the pending renderer process. auto pending_process = (*new_instance)->GetProcess(); - pending_processes_[pending_process->GetID()] = current_process->GetID(); + pending_processes_[pending_process->GetID()] = + content::WebContents::FromRenderFrameHost(render_frame_host);; // Clear the entry in map when process ends. - current_process->AddObserver(this); + pending_process->AddObserver(this); } void AtomBrowserClient::AppendExtraCommandLineSwitches( @@ -277,11 +278,9 @@ void AtomBrowserClient::AppendExtraCommandLineSwitches( } content::WebContents* web_contents = GetWebContentsFromProcessID(process_id); - if (!web_contents) - return; - - WebContentsPreferences::AppendExtraCommandLineSwitches( - web_contents, command_line); + if (web_contents) + WebContentsPreferences::AppendExtraCommandLineSwitches( + web_contents, command_line); } void AtomBrowserClient::DidCreatePpapiPlugin( @@ -421,12 +420,7 @@ void AtomBrowserClient::WebNotificationAllowed( void AtomBrowserClient::RenderProcessHostDestroyed( content::RenderProcessHost* host) { int process_id = host->GetID(); - for (const auto& entry : pending_processes_) { - if (entry.first == process_id || entry.second == process_id) { - pending_processes_.erase(entry.first); - break; - } - } + pending_processes_.erase(process_id); RemoveProcessPreferences(process_id); } diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index d48865504f6c..e4b7eae5d852 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/browser/atom_browser_client.h @@ -130,8 +130,8 @@ class AtomBrowserClient : public brightray::BrowserClient, bool RendererUsesNativeWindowOpen(int process_id); bool RendererDisablesPopups(int process_id); - // pending_render_process => current_render_process. - std::map pending_processes_; + // pending_render_process => web contents. + std::map pending_processes_; std::map process_preferences_; std::map render_process_host_pids_; From c71b0a544112420459205824e11ada9743928111 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 16:16:00 -0700 Subject: [PATCH 06/13] Add spec for configuring web preferences from sandboxed new-window event --- spec/api-browser-window-spec.js | 20 ++++++++++++++++++++ spec/fixtures/api/new-window-preload.js | 4 ++++ spec/fixtures/api/new-window.html | 14 ++++++++++++++ spec/static/main.js | 8 ++++++++ 4 files changed, 46 insertions(+) create mode 100644 spec/fixtures/api/new-window-preload.js create mode 100644 spec/fixtures/api/new-window.html diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 7cd4980cb30c..67cf4f62e937 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1066,6 +1066,26 @@ describe('BrowserWindow module', function () { }) }) + it('should open windows with the options configured via new-window event listeners', function (done) { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + sandbox: true + } + }) + + const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js') + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath) + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'sandbox', true) + ipcMain.once('answer', (event, args) => { + assert.ok(args.includes('--enable-sandbox')) + assert.ok(args.includes(`--preload=${path.join(fixtures, 'api', 'new-window-preload.js')}`)) + done() + }) + w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`) + }) + it('should set ipc event sender correctly', function (done) { w.destroy() w = new BrowserWindow({ diff --git a/spec/fixtures/api/new-window-preload.js b/spec/fixtures/api/new-window-preload.js new file mode 100644 index 000000000000..2676a785ea2a --- /dev/null +++ b/spec/fixtures/api/new-window-preload.js @@ -0,0 +1,4 @@ +const {ipcRenderer} = require('electron') + +ipcRenderer.send('answer', process.argv) +window.close() diff --git a/spec/fixtures/api/new-window.html b/spec/fixtures/api/new-window.html new file mode 100644 index 000000000000..96ab169975a7 --- /dev/null +++ b/spec/fixtures/api/new-window.html @@ -0,0 +1,14 @@ + + + + + + + + + + diff --git a/spec/static/main.js b/spec/static/main.js index 3d1f7c0a95b9..0592362bfe2b 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -264,6 +264,14 @@ ipcMain.on('prevent-next-new-window', (event, id) => { webContents.fromId(id).once('new-window', event => event.preventDefault()) }) +ipcMain.on('set-web-preferences-on-next-new-window', (event, id, key, value) => { + webContents.fromId(id).once('new-window', (event, url, frameName, disposition, options) => { + options.webPreferences[key] = value + + console.log('her?', options.webPreferences) + }) +}) + ipcMain.on('prevent-next-will-attach-webview', (event) => { event.sender.once('will-attach-webview', event => event.preventDefault()) }) From 86b1deedfa4c6c0d8008dba1f35b63b0ecf1102e Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 16:23:04 -0700 Subject: [PATCH 07/13] Inherit enabled-sandbox in opened windows --- lib/browser/guest-window-manager.js | 1 + spec/api-browser-window-spec.js | 26 +++++++++++++++++++++---- spec/fixtures/api/new-window-preload.js | 4 ++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index e2ec4a96a8c9..c895266466d6 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -13,6 +13,7 @@ const inheritedWebPreferences = new Map([ ['javascript', false], ['nativeWindowOpen', true], ['nodeIntegration', false], + ['sandbox', true], ['webviewTag', false] ]) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 67cf4f62e937..3bae23b5f8e5 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1066,6 +1066,24 @@ describe('BrowserWindow module', function () { }) }) + it('should inherit the sandbox setting in opened windows', function (done) { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + sandbox: true + } + }) + + const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js') + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath) + ipcMain.once('answer', (event, args) => { + assert.equal(args.includes('--enable-sandbox'), true) + done() + }) + w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`) + }) + it('should open windows with the options configured via new-window event listeners', function (done) { w.destroy() w = new BrowserWindow({ @@ -1077,10 +1095,10 @@ describe('BrowserWindow module', function () { const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js') ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath) - ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'sandbox', true) - ipcMain.once('answer', (event, args) => { - assert.ok(args.includes('--enable-sandbox')) - assert.ok(args.includes(`--preload=${path.join(fixtures, 'api', 'new-window-preload.js')}`)) + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'foo', 'bar') + ipcMain.once('answer', (event, args, webPreferences) => { + assert.equal(args.includes('--enable-sandbox'), true) + assert.equal(webPreferences.foo, 'bar') done() }) w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`) diff --git a/spec/fixtures/api/new-window-preload.js b/spec/fixtures/api/new-window-preload.js index 2676a785ea2a..0364593734e6 100644 --- a/spec/fixtures/api/new-window-preload.js +++ b/spec/fixtures/api/new-window-preload.js @@ -1,4 +1,4 @@ -const {ipcRenderer} = require('electron') +const {ipcRenderer, remote} = require('electron') -ipcRenderer.send('answer', process.argv) +ipcRenderer.send('answer', process.argv, remote.getCurrentWindow().webContents.getWebPreferences()) window.close() From 9d7c2e22945e744cc6474f4e19c45f9e37b2ca7f Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 16:25:50 -0700 Subject: [PATCH 08/13] Add specs for nativeWindowOpen inheritance and new-window support --- spec/api-browser-window-spec.js | 38 ++++++++++++++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 3bae23b5f8e5..ef6baa4e48f8 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1097,7 +1097,6 @@ describe('BrowserWindow module', function () { ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath) ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'foo', 'bar') ipcMain.once('answer', (event, args, webPreferences) => { - assert.equal(args.includes('--enable-sandbox'), true) assert.equal(webPreferences.foo, 'bar') done() }) @@ -1364,6 +1363,43 @@ describe('BrowserWindow module', function () { }) w.loadURL('file://' + path.join(fixtures, 'api', 'native-window-open-native-addon.html')) }) + + it('should inherit the nativeWindowOpen setting in opened windows', function (done) { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + nativeWindowOpen: true + } + }) + + const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js') + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath) + ipcMain.once('answer', (event, args) => { + assert.equal(args.includes('--native-window-open'), true) + done() + }) + w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`) + }) + + it('should open windows with the options configured via new-window event listeners', function (done) { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + nativeWindowOpen: true + } + }) + + const preloadPath = path.join(fixtures, 'api', 'new-window-preload.js') + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preloadPath) + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'foo', 'bar') + ipcMain.once('answer', (event, args, webPreferences) => { + assert.equal(webPreferences.foo, 'bar') + done() + }) + w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`) + }) }) }) From 0c8f773dec6047a509a029186223fbdf572f58ea Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 16:36:53 -0700 Subject: [PATCH 09/13] Remove debug logging --- spec/static/main.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/spec/static/main.js b/spec/static/main.js index 0592362bfe2b..b1af37065434 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -267,8 +267,6 @@ ipcMain.on('prevent-next-new-window', (event, id) => { ipcMain.on('set-web-preferences-on-next-new-window', (event, id, key, value) => { webContents.fromId(id).once('new-window', (event, url, frameName, disposition, options) => { options.webPreferences[key] = value - - console.log('her?', options.webPreferences) }) }) From 3925bfde8efad0e4a2d3f86400f7d225f080a5ab Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 16:59:49 -0700 Subject: [PATCH 10/13] Add spec for reloading opened window cross-origin --- spec/api-browser-window-spec.js | 42 +++++++++++++++++++ .../api/window-open-location-change.html | 15 +++++++ .../api/window-open-location-final.html | 10 +++++ .../api/window-open-location-open.html | 12 ++++++ spec/fixtures/api/window-open-preload.js | 8 ++++ 5 files changed, 87 insertions(+) create mode 100644 spec/fixtures/api/window-open-location-change.html create mode 100644 spec/fixtures/api/window-open-location-final.html create mode 100644 spec/fixtures/api/window-open-location-open.html create mode 100644 spec/fixtures/api/window-open-preload.js diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index ef6baa4e48f8..09e13b914c00 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1400,6 +1400,31 @@ describe('BrowserWindow module', function () { }) w.loadURL(`file://${path.join(fixtures, 'api', 'new-window.html')}`) }) + + it('retains the original web preferences when window.location is changed to a new origin', async function () { + await serveFileFromProtocol('foo', path.join(fixtures, 'api', 'window-open-location-change.html')) + await serveFileFromProtocol('bar', path.join(fixtures, 'api', 'window-open-location-final.html')) + + w.destroy() + w = new BrowserWindow({ + show: true, + webPreferences: { + nodeIntegration: false, + nativeWindowOpen: true + } + }) + + return new Promise((resolve, reject) => { + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', path.join(fixtures, 'api', 'window-open-preload.js')) + ipcMain.once('answer', (event, args, typeofProcess) => { + assert.equal(args.includes('--node-integration=false'), true) + assert.equal(args.includes('--native-window-open'), true) + assert.equal(typeofProcess, 'undefined') + resolve() + }) + w.loadURL(`file://${path.join(fixtures, 'api', 'window-open-location-open.html')}`) + }) + }) }) }) @@ -2780,3 +2805,20 @@ const isScaleFactorRounding = () => { // Return true if scale factor is odd number above 2 return scaleFactor > 2 && scaleFactor % 2 === 1 } + +function serveFileFromProtocol (protocolName, filePath) { + return new Promise((resolve, reject) => { + protocol.registerBufferProtocol(protocolName, (request, callback) => { + callback({ + mimeType: 'text/html', + data: fs.readFileSync(filePath) + }) + }, (error) => { + if (error != null) { + reject(error) + } else { + resolve() + } + }) + }) +} diff --git a/spec/fixtures/api/window-open-location-change.html b/spec/fixtures/api/window-open-location-change.html new file mode 100644 index 000000000000..fbb87c9f678a --- /dev/null +++ b/spec/fixtures/api/window-open-location-change.html @@ -0,0 +1,15 @@ + + + + + + + + foo + + + diff --git a/spec/fixtures/api/window-open-location-final.html b/spec/fixtures/api/window-open-location-final.html new file mode 100644 index 000000000000..0814c3262c1e --- /dev/null +++ b/spec/fixtures/api/window-open-location-final.html @@ -0,0 +1,10 @@ + + + + + + + + bar + + diff --git a/spec/fixtures/api/window-open-location-open.html b/spec/fixtures/api/window-open-location-open.html new file mode 100644 index 000000000000..95b65652a0ad --- /dev/null +++ b/spec/fixtures/api/window-open-location-open.html @@ -0,0 +1,12 @@ + + + + + + + + + + diff --git a/spec/fixtures/api/window-open-preload.js b/spec/fixtures/api/window-open-preload.js new file mode 100644 index 000000000000..09b89285739f --- /dev/null +++ b/spec/fixtures/api/window-open-preload.js @@ -0,0 +1,8 @@ +const {ipcRenderer} = require('electron') + +setImmediate(function () { + if (window.location.toString() === 'bar://page') { + ipcRenderer.send('answer', process.argv, typeof global.process) + window.close() + } +}) From fdb1fddc26d69d6c937c20183bcffa974aa4df86 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 17:04:07 -0700 Subject: [PATCH 11/13] Only reset preferences when conversion succeeds --- atom/browser/api/atom_api_window.cc | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index 040c3ef78695..34366e2db5d4 100644 --- a/atom/browser/api/atom_api_window.cc +++ b/atom/browser/api/atom_api_window.cc @@ -102,14 +102,18 @@ Window::Window(v8::Isolate* isolate, v8::Local wrapper, #endif if (options.Get("webContents", &web_contents)) { - // Set webPreferences from options if using existing webContents + // Set webPreferences from options if using an existing webContents. + // These preferences will be used when the webContent launches new + // render processes. auto* existing_preferences = WebContentsPreferences::FromWebContents(web_contents->web_contents()); base::DictionaryValue web_preferences_dict; - mate::ConvertFromV8(isolate, web_preferences.GetHandle(), - &web_preferences_dict); - existing_preferences->web_preferences()->Clear(); - existing_preferences->Merge(web_preferences_dict); + if (mate::ConvertFromV8(isolate, web_preferences.GetHandle(), + &web_preferences_dict)) { + existing_preferences->web_preferences()->Clear(); + existing_preferences->Merge(web_preferences_dict); + } + } else { // Creates the WebContents used by BrowserWindow. web_contents = WebContents::Create(isolate, web_preferences); From 208be2a54206267182330fd59616631390b0dc50 Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 17:12:40 -0700 Subject: [PATCH 12/13] Set preload script on opened windows --- spec/api-browser-window-spec.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 09e13b914c00..d378850d88e0 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1005,6 +1005,7 @@ describe('BrowserWindow module', function () { preload: preload } }) + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preload) let htmlPath = path.join(fixtures, 'api', 'sandbox.html?window-open') const pageUrl = 'file://' + htmlPath w.loadURL(pageUrl) @@ -1035,6 +1036,7 @@ describe('BrowserWindow module', function () { preload: preload } }) + ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preload) let htmlPath = path.join(fixtures, 'api', 'sandbox.html?window-open-external') const pageUrl = 'file://' + htmlPath let popupWindow From fc53ac3d3895105f378f8bbe009c60cb86abfaed Mon Sep 17 00:00:00 2001 From: Kevin Sawicki Date: Mon, 10 Jul 2017 17:50:54 -0700 Subject: [PATCH 13/13] Always register closed listeners --- lib/browser/guest-window-manager.js | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index c895266466d6..bd9473dda879 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -80,19 +80,8 @@ const setupGuest = function (embedder, frameName, guest, options) { embedder.send('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSED_' + guestId) embedder.removeListener('render-view-deleted', closedByEmbedder) } - if (!options.webPreferences.sandbox) { - // These events should only be handled when the guest window is opened by a - // non-sandboxed renderer for two reasons: - // - // - `render-view-deleted` is emitted when the popup is closed by the user, - // and that will eventually result in NativeWindow::NotifyWindowClosed - // using a dangling pointer since `destroy()` would have been called by - // `closeByEmbedded` - // - No need to emit `ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSED_` since - // there's no renderer code listening to it., - embedder.once('render-view-deleted', closedByEmbedder) - guest.once('closed', closedByUser) - } + embedder.once('render-view-deleted', closedByEmbedder) + guest.once('closed', closedByUser) if (frameName) { frameToGuest.set(frameName, guest) guest.frameName = frameName