diff --git a/atom/browser/api/atom_api_window.cc b/atom/browser/api/atom_api_window.cc index f48026ea9374..34366e2db5d4 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,43 @@ 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 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; + 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); } diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 065909dc40fb..61c97f2c35ce 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" @@ -78,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. @@ -230,19 +232,20 @@ 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( 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. @@ -275,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( @@ -419,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_; diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index 99bcacf5cb85..bd9473dda879 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -11,7 +11,9 @@ const frameToGuest = new Map() const inheritedWebPreferences = new Map([ ['contextIsolation', true], ['javascript', false], + ['nativeWindowOpen', true], ['nodeIntegration', false], + ['sandbox', true], ['webviewTag', false] ]) @@ -78,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 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. diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index 7cd4980cb30c..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 @@ -1066,6 +1068,43 @@ 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({ + 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, 'foo', 'bar') + ipcMain.once('answer', (event, args, webPreferences) => { + assert.equal(webPreferences.foo, 'bar') + 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({ @@ -1326,6 +1365,68 @@ 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')}`) + }) + + 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')}`) + }) + }) }) }) @@ -2706,3 +2807,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/new-window-preload.js b/spec/fixtures/api/new-window-preload.js new file mode 100644 index 000000000000..0364593734e6 --- /dev/null +++ b/spec/fixtures/api/new-window-preload.js @@ -0,0 +1,4 @@ +const {ipcRenderer, remote} = require('electron') + +ipcRenderer.send('answer', process.argv, remote.getCurrentWindow().webContents.getWebPreferences()) +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/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() + } +}) diff --git a/spec/static/main.js b/spec/static/main.js index 3d1f7c0a95b9..b1af37065434 100644 --- a/spec/static/main.js +++ b/spec/static/main.js @@ -264,6 +264,12 @@ 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 + }) +}) + ipcMain.on('prevent-next-will-attach-webview', (event) => { event.sender.once('will-attach-webview', event => event.preventDefault()) })