From 3ac2f674bc61807ba94a496601bf9cb447cfef74 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Sat, 29 Jul 2017 19:21:28 +1000 Subject: [PATCH 1/3] Fix nativeWindowOpen's opener being null --- atom/browser/atom_browser_client.cc | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 61c97f2c35..c0b86b04cb 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -100,13 +100,14 @@ bool AtomBrowserClient::ShouldCreateNewSiteInstance( int process_id = current_instance->GetProcess()->GetID(); if (!IsRendererSandboxed(process_id)) { - if (!RendererUsesNativeWindowOpen(process_id)) { + auto web_contents = + content::WebContents::FromRenderFrameHost(render_frame_host); + if (!WebContentsPreferences::UsesNativeWindowOpen(web_contents)) { // 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. @@ -114,6 +115,14 @@ bool AtomBrowserClient::ShouldCreateNewSiteInstance( // reuse process to allow synchronous cross-window scripting.) return true; } + + // In a non-sandboxed renderer with native window open we should + // reuse the same site to allow cross-window scripting. We do + // not need to check urls / domains as native window open logic + // handles cross site scripting protection. + if (WebContentsPreferences::UsesNativeWindowOpen(web_contents)) { + return false; + } } // Create new a SiteInstance if navigating to a different site. From 32327b77a5a75d90ba14e40264da2aa512ea3a91 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Mon, 31 Jul 2017 10:47:25 +1000 Subject: [PATCH 2/3] Remove unneeded check but document why it is not there now --- atom/browser/atom_browser_client.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index c0b86b04cb..2213908af2 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -120,9 +120,12 @@ bool AtomBrowserClient::ShouldCreateNewSiteInstance( // reuse the same site to allow cross-window scripting. We do // not need to check urls / domains as native window open logic // handles cross site scripting protection. - if (WebContentsPreferences::UsesNativeWindowOpen(web_contents)) { - return false; - } + // + // NOTE: We know that nativeWindowOpen is enabled at this point + // because we check if it is NOT enabled above this point. We + // will only reach this return if sanbox is disabled but + // nativeWindowOpen is enabled. + return false; } // Create new a SiteInstance if navigating to a different site. From bd99bcbf64cc72ce82757ef19920dddd68ab6365 Mon Sep 17 00:00:00 2001 From: Samuel Attard Date: Mon, 31 Jul 2017 11:00:00 +1000 Subject: [PATCH 3/3] Fix typo in comment --- atom/browser/atom_browser_client.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/atom/browser/atom_browser_client.cc b/atom/browser/atom_browser_client.cc index 2213908af2..3401ef5818 100644 --- a/atom/browser/atom_browser_client.cc +++ b/atom/browser/atom_browser_client.cc @@ -123,7 +123,7 @@ bool AtomBrowserClient::ShouldCreateNewSiteInstance( // // NOTE: We know that nativeWindowOpen is enabled at this point // because we check if it is NOT enabled above this point. We - // will only reach this return if sanbox is disabled but + // will only reach this return if sandbox is disabled but // nativeWindowOpen is enabled. return false; }