diff --git a/atom/browser/api/atom_api_web_contents.cc b/atom/browser/api/atom_api_web_contents.cc index bedc0976969..fedb5af844f 100644 --- a/atom/browser/api/atom_api_web_contents.cc +++ b/atom/browser/api/atom_api_web_contents.cc @@ -13,6 +13,7 @@ #include "atom/browser/atom_browser_client.h" #include "atom/browser/atom_browser_context.h" #include "atom/browser/atom_browser_main_parts.h" +#include "atom/browser/child_web_contents_tracker.h" #include "atom/browser/lib/bluetooth_chooser.h" #include "atom/browser/native_window.h" #include "atom/browser/net/atom_network_delegate.h" @@ -485,6 +486,7 @@ void WebContents::AddNewContents(content::WebContents* source, const gfx::Rect& initial_rect, bool user_gesture, bool* was_blocked) { + new ChildWebContentsTracker(new_contents); v8::Locker locker(isolate()); v8::HandleScope handle_scope(isolate()); auto api_web_contents = CreateFrom(isolate(), new_contents); diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 6addfb5c7cb..6cede9c0112 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -15,6 +15,7 @@ #include "atom/browser/atom_quota_permission_context.h" #include "atom/browser/atom_resource_dispatcher_host_delegate.h" #include "atom/browser/atom_speech_recognition_manager_delegate.h" +#include "atom/browser/child_web_contents_tracker.h" #include "atom/browser/native_window.h" #include "atom/browser/web_contents_permission_helper.h" #include "atom/browser/web_contents_preferences.h" @@ -87,17 +88,31 @@ content::WebContents* AtomBrowserClient::GetWebContentsFromProcessID( } bool AtomBrowserClient::ShouldCreateNewSiteInstance( + content::RenderFrameHost* render_frame_host, content::BrowserContext* browser_context, content::SiteInstance* current_instance, const GURL& url) { - if (url.SchemeIs(url::kJavaScriptScheme)) // "javacript:" scheme should always use same SiteInstance return false; - if (!IsRendererSandboxed(current_instance->GetProcess()->GetID())) - // non-sandboxed renderers should always create a new SiteInstance - return true; + int process_id = current_instance->GetProcess()->GetID(); + if (!IsRendererSandboxed(process_id)) { + if (!RendererUsesNativeWindowOpen(process_id)) { + // non-sandboxed renderers without native window.open should always create + // a new SiteInstance + return true; + } + auto web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + if (!ChildWebContentsTracker::IsChildWebContents(web_contents)) { + // Root WebContents should always create new process to make sure + // native addons are loaded correctly after reload / navigation. + // (Non-root WebContents opened by window.open() should try to + // reuse process to allow synchronous cross-window scripting.) + return true; + } + } // Create new a SiteInstance if navigating to a different site. auto src_url = current_instance->GetSiteURL(); @@ -109,19 +124,27 @@ bool AtomBrowserClient::ShouldCreateNewSiteInstance( content::SiteInstance::GetSiteForURL(browser_context, url) != src_url; } -void AtomBrowserClient::AddSandboxedRendererId(int process_id) { - base::AutoLock auto_lock(sandboxed_renderers_lock_); - sandboxed_renderers_.insert(process_id); +void AtomBrowserClient::AddProcessPreferences( + int process_id, AtomBrowserClient::ProcessPreferences prefs) { + base::AutoLock auto_lock(process_preferences_lock_); + process_preferences_[process_id] = prefs; } -void AtomBrowserClient::RemoveSandboxedRendererId(int process_id) { - base::AutoLock auto_lock(sandboxed_renderers_lock_); - sandboxed_renderers_.erase(process_id); +void AtomBrowserClient::RemoveProcessPreferences(int process_id) { + base::AutoLock auto_lock(process_preferences_lock_); + process_preferences_.erase(process_id); } bool AtomBrowserClient::IsRendererSandboxed(int process_id) { - base::AutoLock auto_lock(sandboxed_renderers_lock_); - return sandboxed_renderers_.count(process_id); + base::AutoLock auto_lock(process_preferences_lock_); + auto it = process_preferences_.find(process_id); + return it != process_preferences_.end() && it->second.sandbox; +} + +bool AtomBrowserClient::RendererUsesNativeWindowOpen(int process_id) { + base::AutoLock auto_lock(process_preferences_lock_); + auto it = process_preferences_.find(process_id); + return it != process_preferences_.end() && it->second.native_window_open; } void AtomBrowserClient::RenderProcessWillLaunch( @@ -133,11 +156,13 @@ void AtomBrowserClient::RenderProcessWillLaunch( new WidevineCdmMessageFilter(process_id, host->GetBrowserContext())); content::WebContents* web_contents = GetWebContentsFromProcessID(process_id); - if (WebContentsPreferences::IsSandboxed(web_contents)) { - AddSandboxedRendererId(host->GetID()); - // ensure the sandboxed renderer id is removed later - host->AddObserver(this); - } + ProcessPreferences process_prefs; + process_prefs.sandbox = WebContentsPreferences::IsSandboxed(web_contents); + process_prefs.native_window_open + = WebContentsPreferences::UsesNativeWindowOpen(web_contents); + AddProcessPreferences(host->GetID(), process_prefs); + // ensure the ProcessPreferences is removed later + host->AddObserver(this); } content::SpeechRecognitionManagerDelegate* @@ -182,7 +207,8 @@ void AtomBrowserClient::OverrideSiteInstanceForNavigation( return; } - if (!ShouldCreateNewSiteInstance(browser_context, current_instance, url)) + if (!ShouldCreateNewSiteInstance(render_frame_host, browser_context, + current_instance, url)) return; scoped_refptr site_instance = @@ -315,7 +341,8 @@ bool AtomBrowserClient::CanCreateWindow( bool* no_javascript_access) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); - if (IsRendererSandboxed(opener_render_process_id)) { + if (IsRendererSandboxed(opener_render_process_id) + || RendererUsesNativeWindowOpen(opener_render_process_id)) { *no_javascript_access = false; return true; } @@ -380,7 +407,7 @@ void AtomBrowserClient::RenderProcessHostDestroyed( break; } } - RemoveSandboxedRendererId(process_id); + RemoveProcessPreferences(process_id); } } // namespace atom diff --git a/atom/browser/atom_browser_client.h b/atom/browser/atom_browser_client.h index ba1bd86fcad..c9a58981a2c 100644 --- a/atom/browser/atom_browser_client.h +++ b/atom/browser/atom_browser_client.h @@ -111,19 +111,24 @@ class AtomBrowserClient : public brightray::BrowserClient, void RenderProcessHostDestroyed(content::RenderProcessHost* host) override; private: - bool ShouldCreateNewSiteInstance(content::BrowserContext* browser_context, + bool ShouldCreateNewSiteInstance(content::RenderFrameHost* render_frame_host, + content::BrowserContext* browser_context, content::SiteInstance* current_instance, const GURL& dest_url); - // Add/remove a process id to `sandboxed_renderers_`. - void AddSandboxedRendererId(int process_id); - void RemoveSandboxedRendererId(int process_id); + struct ProcessPreferences { + bool sandbox; + bool native_window_open; + }; + void AddProcessPreferences(int process_id, ProcessPreferences prefs); + void RemoveProcessPreferences(int process_id); bool IsRendererSandboxed(int process_id); + bool RendererUsesNativeWindowOpen(int process_id); // pending_render_process => current_render_process. std::map pending_processes_; - // Set that contains the process ids of all sandboxed renderers - std::set sandboxed_renderers_; - base::Lock sandboxed_renderers_lock_; + + std::map process_preferences_; + base::Lock process_preferences_lock_; std::unique_ptr resource_dispatcher_host_delegate_; diff --git a/atom/browser/child_web_contents_tracker.cc b/atom/browser/child_web_contents_tracker.cc new file mode 100644 index 00000000000..3c9eb0d3f04 --- /dev/null +++ b/atom/browser/child_web_contents_tracker.cc @@ -0,0 +1,33 @@ +// Copyright (c) 2017 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#include "atom/browser/child_web_contents_tracker.h" + +#include + +namespace atom { + +namespace { + +std::unordered_set g_child_web_contents; + +} // namespace + +ChildWebContentsTracker::ChildWebContentsTracker( + content::WebContents* web_contents) + : content::WebContentsObserver(web_contents) { + g_child_web_contents.insert(web_contents); +} + +bool ChildWebContentsTracker::IsChildWebContents( + content::WebContents* web_contents) { + return g_child_web_contents.find(web_contents) != g_child_web_contents.end(); +} + +void ChildWebContentsTracker::WebContentsDestroyed() { + g_child_web_contents.erase(web_contents()); + delete this; +} + +} // namespace atom diff --git a/atom/browser/child_web_contents_tracker.h b/atom/browser/child_web_contents_tracker.h new file mode 100644 index 00000000000..785509e6ea7 --- /dev/null +++ b/atom/browser/child_web_contents_tracker.h @@ -0,0 +1,25 @@ +// Copyright (c) 2017 GitHub, Inc. +// Use of this source code is governed by the MIT license that can be +// found in the LICENSE file. + +#ifndef ATOM_BROWSER_CHILD_WEB_CONTENTS_TRACKER_H_ +#define ATOM_BROWSER_CHILD_WEB_CONTENTS_TRACKER_H_ + +#include "content/public/browser/web_contents_observer.h" + +namespace atom { + +// ChildWebContentsTracker tracks child WebContents +// created by native `window.open()` +class ChildWebContentsTracker : public content::WebContentsObserver { + public: + explicit ChildWebContentsTracker(content::WebContents* web_contents); + static bool IsChildWebContents(content::WebContents* web_contents); + + protected: + void WebContentsDestroyed() override; +}; + +} // namespace atom + +#endif // ATOM_BROWSER_CHILD_WEB_CONTENTS_TRACKER_H_ diff --git a/atom/browser/web_contents_preferences.cc b/atom/browser/web_contents_preferences.cc index 7bb8605039a..adab0bf85ac 100644 --- a/atom/browser/web_contents_preferences.cc +++ b/atom/browser/web_contents_preferences.cc @@ -106,6 +106,8 @@ void WebContentsPreferences::AppendExtraCommandLineSwitches( // integration. if (IsSandboxed(web_contents)) command_line->AppendSwitch(switches::kEnableSandbox); + if (web_preferences.GetBoolean("nativeWindowOpen", &b) && b) + command_line->AppendSwitch(switches::kNativeWindowOpen); // The preload script. base::FilePath::StringType preload; @@ -212,6 +214,22 @@ bool WebContentsPreferences::IsSandboxed(content::WebContents* web_contents) { return sandboxed; } +bool WebContentsPreferences::UsesNativeWindowOpen( + content::WebContents* web_contents) { + WebContentsPreferences* self; + if (!web_contents) + return false; + + self = FromWebContents(web_contents); + if (!self) + return false; + + base::DictionaryValue& web_preferences = self->web_preferences_; + bool use = false; + web_preferences.GetBoolean("nativeWindowOpen", &use); + return use; +} + // static void WebContentsPreferences::OverrideWebkitPrefs( content::WebContents* web_contents, content::WebPreferences* prefs) { diff --git a/atom/browser/web_contents_preferences.h b/atom/browser/web_contents_preferences.h index f6e44c51b10..f046cdfc99a 100644 --- a/atom/browser/web_contents_preferences.h +++ b/atom/browser/web_contents_preferences.h @@ -38,6 +38,7 @@ class WebContentsPreferences content::WebContents* web_contents, base::CommandLine* command_line); static bool IsSandboxed(content::WebContents* web_contents); + static bool UsesNativeWindowOpen(content::WebContents* web_contents); // Modify the WebPreferences according to |web_contents|'s preferences. static void OverrideWebkitPrefs( diff --git a/atom/common/options_switches.cc b/atom/common/options_switches.cc index 2f1c0368f35..657a36ac41c 100644 --- a/atom/common/options_switches.cc +++ b/atom/common/options_switches.cc @@ -172,6 +172,7 @@ const char kGuestInstanceID[] = "guest-instance-id"; const char kOpenerID[] = "opener-id"; const char kScrollBounce[] = "scroll-bounce"; const char kHiddenPage[] = "hidden-page"; +const char kNativeWindowOpen[] = "native-window-open"; // Command switch passed to renderer process to control nodeIntegration. const char kNodeIntegrationInWorker[] = "node-integration-in-worker"; diff --git a/atom/common/options_switches.h b/atom/common/options_switches.h index 69e7af029e1..057aa26e080 100644 --- a/atom/common/options_switches.h +++ b/atom/common/options_switches.h @@ -92,6 +92,7 @@ extern const char kGuestInstanceID[]; extern const char kOpenerID[]; extern const char kScrollBounce[]; extern const char kHiddenPage[]; +extern const char kNativeWindowOpen[]; extern const char kNodeIntegrationInWorker[]; extern const char kWidevineCdmPath[]; diff --git a/atom/renderer/atom_renderer_client.cc b/atom/renderer/atom_renderer_client.cc index ce36f9520e5..393b864031a 100644 --- a/atom/renderer/atom_renderer_client.cc +++ b/atom/renderer/atom_renderer_client.cc @@ -204,6 +204,8 @@ void AtomRendererClient::SetupMainWorldOverrides( dict.Set(options::kOpenerID, command_line->GetSwitchValueASCII(switches::kOpenerID)); dict.Set("hiddenPage", command_line->HasSwitch(switches::kHiddenPage)); + dict.Set("nativeWindowOpen", + command_line->HasSwitch(switches::kNativeWindowOpen)); v8::Local args[] = { binding }; ignore_result(func->Call(context, v8::Null(isolate), 1, args)); diff --git a/docs/api/browser-window.md b/docs/api/browser-window.md index 8fcc3f8213d..f3e041778ec 100644 --- a/docs/api/browser-window.md +++ b/docs/api/browser-window.md @@ -307,6 +307,7 @@ It creates a new `BrowserWindow` with native properties as set by the `options`. 'Electron Isolated Context' entry in the combo box at the top of the Console tab. **Note:** This option is currently experimental and may change or be removed in future Electron releases. + * `nativeWindowOpen` Boolean (optional) - Whether to use native `window.open()`. Defaults to `false`. When setting minimum or maximum window size with `minWidth`/`maxWidth`/ `minHeight`/`maxHeight`, it only constrains the users. It won't prevent you from diff --git a/docs/api/window-open.md b/docs/api/window-open.md index 41332aa6cfd..30ea8511945 100644 --- a/docs/api/window-open.md +++ b/docs/api/window-open.md @@ -45,3 +45,40 @@ has to be a field of `BrowserWindow`'s options. Sends a message to the parent window with the specified origin or `*` for no origin preference. + +### Use Native `window.open()` + +If you want to use native `window.open()` implementation, pass `useNativeWindowOpen: true` in `webPreferences` option. +Native `window.open()` allows synchronous access to opened windows so it is convenient choice if you need to open a dialog or a preferences window. + +The creation of the `BrowserWindow` is customizable in `WebContents`'s `new-window` event. + +```javascript +// main process +const mainWindow = new BrowserWindow({ + width: 800, + height: 600, + webPreferences: { + nativeWindowOpen: true + } +}) +mainWindow.webContents.on('new-window', (event, url, frameName, disposition, options, additionalFeatures) => { + if (frameName === 'modal') { + // open window as modal + event.preventDefault() + Object.assign(options, { + modal: true, + parent: mainWindow, + width: 100, + height: 100 + }) + event.newGuest = new BrowserWindow(options) + } +}) +``` + +```javascript +// renderer process (mainWindow) +let modal = window.open('', 'modal') +modal.document.write('

Hello

') +``` diff --git a/filenames.gypi b/filenames.gypi index 8408b273e66..54ddf4dde19 100644 --- a/filenames.gypi +++ b/filenames.gypi @@ -202,6 +202,8 @@ 'atom/browser/browser_mac.mm', 'atom/browser/browser_win.cc', 'atom/browser/browser_observer.h', + 'atom/browser/child_web_contents_tracker.cc', + 'atom/browser/child_web_contents_tracker.h', 'atom/browser/common_web_contents_delegate_mac.mm', 'atom/browser/common_web_contents_delegate_views.cc', 'atom/browser/common_web_contents_delegate.cc', diff --git a/lib/browser/guest-window-manager.js b/lib/browser/guest-window-manager.js index ecf4093dcf0..305da92f9e6 100644 --- a/lib/browser/guest-window-manager.js +++ b/lib/browser/guest-window-manager.js @@ -255,6 +255,11 @@ ipcMain.on('ELECTRON_GUEST_WINDOW_MANAGER_INTERNAL_WINDOW_OPEN', function (event const newGuest = event.newGuest if ((event.sender.isGuest() && !event.sender.allowPopups) || event.defaultPrevented) { if (newGuest !== undefined && newGuest !== null) { + if (options.webContents === newGuest.webContents) { + // the webContents is not changed, so set defaultPrevented to false to + // stop the callers of this event from destroying the webContents. + event.defaultPrevented = false + } event.returnValue = setupGuest(event.sender, frameName, newGuest, options) } else { event.returnValue = null diff --git a/lib/isolated_renderer/init.js b/lib/isolated_renderer/init.js index 943713d985b..1536ab10638 100644 --- a/lib/isolated_renderer/init.js +++ b/lib/isolated_renderer/init.js @@ -19,8 +19,8 @@ const ipcRenderer = { once () {} } -let {guestInstanceId, hiddenPage, openerId} = binding +let {guestInstanceId, hiddenPage, openerId, nativeWindowOpen} = binding if (guestInstanceId != null) guestInstanceId = parseInt(guestInstanceId) if (openerId != null) openerId = parseInt(openerId) -require('../renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, hiddenPage) +require('../renderer/window-setup')(ipcRenderer, guestInstanceId, openerId, hiddenPage, nativeWindowOpen) diff --git a/lib/renderer/override.js b/lib/renderer/override.js index f31e9c0e8cd..fdeb6605a5c 100644 --- a/lib/renderer/override.js +++ b/lib/renderer/override.js @@ -4,5 +4,6 @@ const {ipcRenderer} = require('electron') const {guestInstanceId, openerId} = process const hiddenPage = process.argv.includes('--hidden-page') +const usesNativeWindowOpen = process.argv.includes('--native-window-open') -require('./window-setup')(ipcRenderer, guestInstanceId, openerId, hiddenPage) +require('./window-setup')(ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNativeWindowOpen) diff --git a/lib/renderer/window-setup.js b/lib/renderer/window-setup.js index 99f7aef5423..b1187e96f18 100644 --- a/lib/renderer/window-setup.js +++ b/lib/renderer/window-setup.js @@ -106,7 +106,7 @@ const getHistoryOperation = function (ipcRenderer, ...args) { return ipcRenderer.sendSync('ELECTRON_SYNC_NAVIGATION_CONTROLLER', ...args) } -module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage) => { +module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage, usesNativeWindowOpen) => { if (guestInstanceId == null) { // Override default window.close. window.close = function () { @@ -114,16 +114,18 @@ module.exports = (ipcRenderer, guestInstanceId, openerId, hiddenPage) => { } } - // Make the browser window or guest view emit "new-window" event. - window.open = function (url, frameName, features) { - if (url != null && url !== '') { - url = resolveURL(url) - } - const guestId = ipcRenderer.sendSync('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', url, toString(frameName), toString(features)) - if (guestId != null) { - return getOrCreateProxy(ipcRenderer, guestId) - } else { - return null + if (!usesNativeWindowOpen) { + // Make the browser window or guest view emit "new-window" event. + window.open = function (url, frameName, features) { + if (url != null && url !== '') { + url = resolveURL(url) + } + const guestId = ipcRenderer.sendSync('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_OPEN', url, toString(frameName), toString(features)) + if (guestId != null) { + return getOrCreateProxy(ipcRenderer, guestId) + } else { + return null + } } } diff --git a/spec/api-browser-window-spec.js b/spec/api-browser-window-spec.js index ff2734b5ea2..8d5f329a14c 100644 --- a/spec/api-browser-window-spec.js +++ b/spec/api-browser-window-spec.js @@ -1174,6 +1174,70 @@ describe('BrowserWindow module', function () { }) }) }) + + describe('nativeWindowOpen option', () => { + beforeEach(() => { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + nativeWindowOpen: true + } + }) + }) + + it('opens window of about:blank with cross-scripting enabled', (done) => { + ipcMain.once('answer', (event, content) => { + assert.equal(content, 'Hello') + done() + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'native-window-open-blank.html')) + }) + + it('opens window of same domain with cross-scripting enabled', (done) => { + ipcMain.once('answer', (event, content) => { + assert.equal(content, 'Hello') + done() + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'native-window-open-file.html')) + }) + + if (process.platform !== 'win32' || process.execPath.toLowerCase().indexOf('\\out\\d\\') === -1) { + it('loads native addons correctly after reload', (done) => { + ipcMain.once('answer', (event, content) => { + assert.equal(content, 'function') + ipcMain.once('answer', (event, content) => { + assert.equal(content, 'function') + done() + }) + w.reload() + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'native-window-open-native-addon.html')) + }) + } + }) + }) + + describe('nativeWindowOpen + contextIsolation options', () => { + beforeEach(() => { + w.destroy() + w = new BrowserWindow({ + show: false, + webPreferences: { + nativeWindowOpen: true, + contextIsolation: true, + preload: path.join(fixtures, 'api', 'native-window-open-isolated-preload.js') + } + }) + }) + + it('opens window with cross-scripting enabled from isolated context', (done) => { + ipcMain.once('answer', (event, content) => { + assert.equal(content, 'Hello') + done() + }) + w.loadURL('file://' + path.join(fixtures, 'api', 'native-window-open-isolated.html')) + }) }) describe('beforeunload handler', function () { diff --git a/spec/fixtures/api/native-window-open-blank.html b/spec/fixtures/api/native-window-open-blank.html new file mode 100644 index 00000000000..7174594e244 --- /dev/null +++ b/spec/fixtures/api/native-window-open-blank.html @@ -0,0 +1,11 @@ + + + + + diff --git a/spec/fixtures/api/native-window-open-child.html b/spec/fixtures/api/native-window-open-child.html new file mode 100644 index 00000000000..986a4a1a25b --- /dev/null +++ b/spec/fixtures/api/native-window-open-child.html @@ -0,0 +1 @@ +

Hello

diff --git a/spec/fixtures/api/native-window-open-file.html b/spec/fixtures/api/native-window-open-file.html new file mode 100644 index 00000000000..30f2c61c9e0 --- /dev/null +++ b/spec/fixtures/api/native-window-open-file.html @@ -0,0 +1,12 @@ + + + + + diff --git a/spec/fixtures/api/native-window-open-isolated-preload.js b/spec/fixtures/api/native-window-open-isolated-preload.js new file mode 100644 index 00000000000..aa214166021 --- /dev/null +++ b/spec/fixtures/api/native-window-open-isolated-preload.js @@ -0,0 +1,5 @@ +const {ipcRenderer} = require('electron') + +window.addEventListener('message', (event) => { + ipcRenderer.send('answer', event.data) +}) diff --git a/spec/fixtures/api/native-window-open-isolated.html b/spec/fixtures/api/native-window-open-isolated.html new file mode 100644 index 00000000000..706209f1908 --- /dev/null +++ b/spec/fixtures/api/native-window-open-isolated.html @@ -0,0 +1,10 @@ + + + + + diff --git a/spec/fixtures/api/native-window-open-native-addon.html b/spec/fixtures/api/native-window-open-native-addon.html new file mode 100644 index 00000000000..c1f75872ef9 --- /dev/null +++ b/spec/fixtures/api/native-window-open-native-addon.html @@ -0,0 +1,9 @@ + + + + +