Browse Source

Revert "fix: use appropriate site instance for cross-site nav's (backport: 4-0-x) (#15969)" (#15995)

This reverts commit 3021870de462e715ef6b7620b439912223e0485b.
Shelley Vohr 6 years ago
parent
commit
366bc82c8f

+ 0 - 21
atom/browser/api/atom_api_web_contents.cc

@@ -788,29 +788,8 @@ void WebContents::RenderViewCreated(content::RenderViewHost* render_view_host) {
     impl->disable_hidden_ = !background_throttling_;
 }
 
-void WebContents::RenderViewHostChanged(content::RenderViewHost* old_host,
-                                        content::RenderViewHost* new_host) {
-  currently_committed_process_id_ = new_host->GetProcess()->GetID();
-}
-
 void WebContents::RenderViewDeleted(content::RenderViewHost* render_view_host) {
-  // This event is necessary for tracking any states with respect to
-  // intermediate render view hosts aka speculative render view hosts. Currently
-  // used by object-registry.js to ref count remote objects.
   Emit("render-view-deleted", render_view_host->GetProcess()->GetID());
-
-  if (-1 == currently_committed_process_id_ ||
-      render_view_host->GetProcess()->GetID() ==
-          currently_committed_process_id_) {
-    currently_committed_process_id_ = -1;
-
-    // When the RVH that has been deleted is the current RVH it means that the
-    // the web contents are being closed. This is communicated by this event.
-    // Currently tracked by guest-window-manager.js to destroy the
-    // BrowserWindow.
-    Emit("current-render-view-deleted",
-         render_view_host->GetProcess()->GetID());
-  }
 }
 
 void WebContents::RenderProcessGone(base::TerminationStatus status) {

+ 0 - 6
atom/browser/api/atom_api_web_contents.h

@@ -380,8 +380,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
   // content::WebContentsObserver:
   void BeforeUnloadFired(const base::TimeTicks& proceed_time) override;
   void RenderViewCreated(content::RenderViewHost*) override;
-  void RenderViewHostChanged(content::RenderViewHost* old_host,
-                             content::RenderViewHost* new_host) override;
   void RenderViewDeleted(content::RenderViewHost*) override;
   void RenderProcessGone(base::TerminationStatus status) override;
   void DocumentLoadedInFrame(
@@ -514,10 +512,6 @@ class WebContents : public mate::TrackableObject<WebContents>,
   // Observers of this WebContents.
   base::ObserverList<ExtendedWebContentsObserver> observers_;
 
-  // The ID of the process of the currently committed RenderViewHost.
-  // -1 means no speculative RVH has been committed yet.
-  int currently_committed_process_id_ = -1;
-
   DISALLOW_COPY_AND_ASSIGN(WebContents);
 };
 

+ 68 - 142
atom/browser/atom_browser_client.cc

@@ -128,7 +128,7 @@ AtomBrowserClient::~AtomBrowserClient() {}
 content::WebContents* AtomBrowserClient::GetWebContentsFromProcessID(
     int process_id) {
   // If the process is a pending process, we should use the web contents
-  // for the frame host passed into RegisterPendingProcess.
+  // for the frame host passed into OverrideSiteInstanceForNavigation.
   if (base::ContainsKey(pending_processes_, process_id))
     return pending_processes_[process_id];
 
@@ -137,36 +137,24 @@ content::WebContents* AtomBrowserClient::GetWebContentsFromProcessID(
   return WebContentsPreferences::GetWebContentsFromProcessID(process_id);
 }
 
-bool AtomBrowserClient::ShouldForceNewSiteInstance(
-    content::RenderFrameHost* current_rfh,
-    content::RenderFrameHost* speculative_rfh,
+bool AtomBrowserClient::ShouldCreateNewSiteInstance(
+    content::RenderFrameHost* render_frame_host,
     content::BrowserContext* browser_context,
-    const GURL& url,
-    bool has_response_started) const {
+    content::SiteInstance* current_instance,
+    const GURL& url) {
   if (url.SchemeIs(url::kJavaScriptScheme))
     // "javacript:" scheme should always use same SiteInstance
     return false;
 
-  content::SiteInstance* current_instance = current_rfh->GetSiteInstance();
-  content::SiteInstance* speculative_instance =
-      speculative_rfh ? speculative_rfh->GetSiteInstance() : nullptr;
   int process_id = current_instance->GetProcess()->GetID();
-  if (NavigationWasRedirectedCrossSite(browser_context, current_instance,
-                                       speculative_instance, url,
-                                       has_response_started)) {
-    // Navigation was redirected. We can't force the current, speculative or a
-    // new unrelated site instance to be used. Delegate to the content layer.
-    return false;
-  } else if (IsRendererSandboxed(process_id)) {
-    // Renderer is sandboxed, delegate the decision to the content layer for all
-    // origins.
-    return false;
-  } else if (!RendererUsesNativeWindowOpen(process_id)) {
-    // non-sandboxed renderers without native window.open should always create
-    // a new SiteInstance
-    return true;
-  } else {
-    auto* web_contents = content::WebContents::FromRenderFrameHost(current_rfh);
+  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.
@@ -181,26 +169,6 @@ bool AtomBrowserClient::ShouldForceNewSiteInstance(
   return !IsSameWebSite(browser_context, src_url, url);
 }
 
-bool AtomBrowserClient::NavigationWasRedirectedCrossSite(
-    content::BrowserContext* browser_context,
-    content::SiteInstance* current_instance,
-    content::SiteInstance* speculative_instance,
-    const GURL& dest_url,
-    bool has_response_started) const {
-  bool navigation_was_redirected = false;
-  if (has_response_started) {
-    navigation_was_redirected = !IsSameWebSite(
-        browser_context, current_instance->GetSiteURL(), dest_url);
-  } else {
-    navigation_was_redirected =
-        speculative_instance &&
-        !IsSameWebSite(browser_context, speculative_instance->GetSiteURL(),
-                       dest_url);
-  }
-
-  return navigation_was_redirected;
-}
-
 void AtomBrowserClient::AddProcessPreferences(
     int process_id,
     AtomBrowserClient::ProcessPreferences prefs) {
@@ -213,69 +181,29 @@ void AtomBrowserClient::RemoveProcessPreferences(int process_id) {
   process_preferences_.erase(process_id);
 }
 
-bool AtomBrowserClient::IsProcessObserved(int process_id) const {
+bool AtomBrowserClient::IsProcessObserved(int process_id) {
   base::AutoLock auto_lock(process_preferences_lock_);
   return process_preferences_.find(process_id) != process_preferences_.end();
 }
 
-bool AtomBrowserClient::IsRendererSandboxed(int process_id) const {
+bool AtomBrowserClient::IsRendererSandboxed(int 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) const {
+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;
 }
 
-bool AtomBrowserClient::RendererDisablesPopups(int process_id) const {
+bool AtomBrowserClient::RendererDisablesPopups(int process_id) {
   base::AutoLock auto_lock(process_preferences_lock_);
   auto it = process_preferences_.find(process_id);
   return it != process_preferences_.end() && it->second.disable_popups;
 }
 
-std::string AtomBrowserClient::GetAffinityPreference(
-    content::RenderFrameHost* rfh) const {
-  auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
-  auto* web_preferences = WebContentsPreferences::From(web_contents);
-  std::string affinity;
-  if (web_preferences &&
-      web_preferences->GetPreference("affinity", &affinity) &&
-      !affinity.empty()) {
-    affinity = base::ToLowerASCII(affinity);
-  }
-
-  return affinity;
-}
-
-content::SiteInstance* AtomBrowserClient::GetSiteInstanceFromAffinity(
-    content::BrowserContext* browser_context,
-    const GURL& url,
-    content::RenderFrameHost* rfh) const {
-  std::string affinity = GetAffinityPreference(rfh);
-  if (!affinity.empty()) {
-    auto iter = site_per_affinities_.find(affinity);
-    GURL dest_site = content::SiteInstance::GetSiteForURL(browser_context, url);
-    if (iter != site_per_affinities_.end() &&
-        IsSameWebSite(browser_context, iter->second->GetSiteURL(), dest_site)) {
-      return iter->second;
-    }
-  }
-
-  return nullptr;
-}
-
-void AtomBrowserClient::ConsiderSiteInstanceForAffinity(
-    content::RenderFrameHost* rfh,
-    content::SiteInstance* site_instance) {
-  std::string affinity = GetAffinityPreference(rfh);
-  if (!affinity.empty()) {
-    site_per_affinities_[affinity] = site_instance;
-  }
-}
-
 void AtomBrowserClient::RenderProcessWillLaunch(
     content::RenderProcessHost* host,
     service_manager::mojom::ServiceRequest* service_request) {
@@ -335,59 +263,62 @@ void AtomBrowserClient::OverrideWebkitPrefs(content::RenderViewHost* host,
     web_preferences->OverrideWebkitPrefs(prefs);
 }
 
-content::ContentBrowserClient::SiteInstanceForNavigationType
-AtomBrowserClient::ShouldOverrideSiteInstanceForNavigation(
-    content::RenderFrameHost* current_rfh,
-    content::RenderFrameHost* speculative_rfh,
+void AtomBrowserClient::OverrideSiteInstanceForNavigation(
+    content::RenderFrameHost* rfh,
     content::BrowserContext* browser_context,
     const GURL& url,
-    bool has_response_started,
-    content::SiteInstance** affinity_site_instance) const {
+    bool has_request_started,
+    content::SiteInstance* candidate_instance,
+    content::SiteInstance** new_instance) {
   if (g_suppress_renderer_process_restart) {
     g_suppress_renderer_process_restart = false;
-    return SiteInstanceForNavigationType::ASK_CHROMIUM;
+    return;
   }
 
-  // Do we have an affinity site to manage ?
-  content::SiteInstance* site_instance_from_affinity =
-      GetSiteInstanceFromAffinity(browser_context, url, current_rfh);
-  if (site_instance_from_affinity) {
-    *affinity_site_instance = site_instance_from_affinity;
-    return SiteInstanceForNavigationType::FORCE_AFFINITY;
-  }
+  content::SiteInstance* current_instance = rfh->GetSiteInstance();
+  if (!ShouldCreateNewSiteInstance(rfh, browser_context, current_instance, url))
+    return;
 
-  if (!ShouldForceNewSiteInstance(current_rfh, speculative_rfh, browser_context,
-                                  url, has_response_started)) {
-    return SiteInstanceForNavigationType::ASK_CHROMIUM;
-  }
+  // Do we have an affinity site to manage ?
+  auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
+  auto* web_preferences = WebContentsPreferences::From(web_contents);
+  std::string affinity;
+  if (web_preferences &&
+      web_preferences->GetPreference("affinity", &affinity) &&
+      !affinity.empty()) {
+    affinity = base::ToLowerASCII(affinity);
+    auto iter = site_per_affinities.find(affinity);
+    GURL dest_site = content::SiteInstance::GetSiteForURL(browser_context, url);
+    if (iter != site_per_affinities.end() &&
+        IsSameWebSite(browser_context, iter->second->GetSiteURL(), dest_site)) {
+      *new_instance = iter->second;
+    } else {
+      site_per_affinities[affinity] = candidate_instance;
+      *new_instance = candidate_instance;
+      // Remember the original web contents for the pending renderer process.
+      auto* pending_process = candidate_instance->GetProcess();
+      pending_processes_[pending_process->GetID()] = web_contents;
+    }
+  } else {
+    // OverrideSiteInstanceForNavigation will be called more than once during a
+    // navigation (currently twice, on request and when it's about to commit in
+    // the renderer), look at RenderFrameHostManager::GetFrameHostForNavigation.
+    // In the default mode we should resuse the same site instance until the
+    // request commits otherwise it will get destroyed. Currently there is no
+    // unique lifetime tracker for a navigation request during site instance
+    // creation. We check for the state of the request, which should be one of
+    // (WAITING_FOR_RENDERER_RESPONSE, STARTED, RESPONSE_STARTED, FAILED) along
+    // with the availability of a speculative render frame host.
+    if (has_request_started) {
+      *new_instance = current_instance;
+      return;
+    }
 
-  // ShouldOverrideSiteInstanceForNavigation will be called more than once
-  // during a navigation (currently twice, on request and when it's about
-  // to commit in the renderer), look at
-  // RenderFrameHostManager::GetFrameHostForNavigation.
-  // In the default mode we should reuse the same site instance until the
-  // request commits otherwise it will get destroyed. Currently there is no
-  // unique lifetime tracker for a navigation request during site instance
-  // creation. We check for the state of the request, which should be one of
-  // (WAITING_FOR_RENDERER_RESPONSE, STARTED, RESPONSE_STARTED, FAILED) along
-  // with the availability of a speculative render frame host.
-  if (has_response_started) {
-    return SiteInstanceForNavigationType::FORCE_CURRENT;
+    *new_instance = candidate_instance;
+    // Remember the original web contents for the pending renderer process.
+    auto* pending_process = candidate_instance->GetProcess();
+    pending_processes_[pending_process->GetID()] = web_contents;
   }
-
-  return SiteInstanceForNavigationType::FORCE_CANDIDATE_OR_NEW;
-}
-
-void AtomBrowserClient::RegisterPendingSiteInstance(
-    content::RenderFrameHost* rfh,
-    content::SiteInstance* pending_site_instance) {
-  // Do we have an affinity site to manage?
-  ConsiderSiteInstanceForAffinity(rfh, pending_site_instance);
-
-  // Remember the original web contents for the pending renderer process.
-  auto* web_contents = content::WebContents::FromRenderFrameHost(rfh);
-  auto* pending_process = pending_site_instance->GetProcess();
-  pending_processes_[pending_process->GetID()] = web_contents;
 }
 
 void AtomBrowserClient::AppendExtraCommandLineSwitches(
@@ -560,10 +491,10 @@ void AtomBrowserClient::SiteInstanceDeleting(
     content::SiteInstance* site_instance) {
   // We are storing weak_ptr, is it fundamental to maintain the map up-to-date
   // when an instance is destroyed.
-  for (auto iter = site_per_affinities_.begin();
-       iter != site_per_affinities_.end(); ++iter) {
+  for (auto iter = site_per_affinities.begin();
+       iter != site_per_affinities.end(); ++iter) {
     if (iter->second == site_instance) {
-      site_per_affinities_.erase(iter);
+      site_per_affinities.erase(iter);
       break;
     }
   }
@@ -733,16 +664,11 @@ AtomBrowserClient::CreateThrottlesForNavigation(
   return throttles;
 }
 
-bool AtomBrowserClient::ShouldBypassCORB(int render_process_id) const {
+bool AtomBrowserClient::ShouldBypassCORB(int render_process_id) {
   // This is called on the network thread.
   base::AutoLock auto_lock(process_preferences_lock_);
   auto it = process_preferences_.find(render_process_id);
   return it != process_preferences_.end() && !it->second.web_security;
 }
 
-bool AtomBrowserClient::ShouldEnableStrictSiteIsolation() {
-  // Enable site isolation. It is off by default in Chromium <= 69.
-  return true;
-}
-
 }  // namespace atom

+ 16 - 36
atom/browser/atom_browser_client.h

@@ -52,9 +52,6 @@ class AtomBrowserClient : public brightray::BrowserClient,
   std::vector<std::unique_ptr<content::NavigationThrottle>>
   CreateThrottlesForNavigation(content::NavigationHandle* handle) override;
 
-  // content::ContentBrowserClient:
-  bool ShouldEnableStrictSiteIsolation() override;
-
  protected:
   // content::ContentBrowserClient:
   void RenderProcessWillLaunch(
@@ -64,16 +61,13 @@ class AtomBrowserClient : public brightray::BrowserClient,
   CreateSpeechRecognitionManagerDelegate() override;
   void OverrideWebkitPrefs(content::RenderViewHost* render_view_host,
                            content::WebPreferences* prefs) override;
-  SiteInstanceForNavigationType ShouldOverrideSiteInstanceForNavigation(
-      content::RenderFrameHost* current_rfh,
-      content::RenderFrameHost* speculative_rfh,
+  void OverrideSiteInstanceForNavigation(
+      content::RenderFrameHost* render_frame_host,
       content::BrowserContext* browser_context,
-      const GURL& url,
+      const GURL& dest_url,
       bool has_request_started,
-      content::SiteInstance** affinity_site_instance) const override;
-  void RegisterPendingSiteInstance(
-      content::RenderFrameHost* render_frame_host,
-      content::SiteInstance* pending_site_instance) override;
+      content::SiteInstance* candidate_instance,
+      content::SiteInstance** new_instance) override;
   void AppendExtraCommandLineSwitches(base::CommandLine* command_line,
                                       int child_process_id) override;
   void DidCreatePpapiPlugin(content::BrowserPpapiHost* browser_host) override;
@@ -132,7 +126,7 @@ class AtomBrowserClient : public brightray::BrowserClient,
   void WebNotificationAllowed(
       int render_process_id,
       const base::Callback<void(bool, bool)>& callback) override;
-  bool ShouldBypassCORB(int render_process_id) const override;
+  bool ShouldBypassCORB(int render_process_id) override;
 
   // content::RenderProcessHostObserver:
   void RenderProcessHostDestroyed(content::RenderProcessHost* host) override;
@@ -157,30 +151,16 @@ class AtomBrowserClient : public brightray::BrowserClient,
     bool web_security = true;
   };
 
-  bool ShouldForceNewSiteInstance(content::RenderFrameHost* current_rfh,
-                                  content::RenderFrameHost* speculative_rfh,
-                                  content::BrowserContext* browser_context,
-                                  const GURL& dest_url,
-                                  bool has_request_started) const;
-  bool NavigationWasRedirectedCrossSite(
-      content::BrowserContext* browser_context,
-      content::SiteInstance* current_instance,
-      content::SiteInstance* speculative_instance,
-      const GURL& dest_url,
-      bool has_request_started) const;
+  bool ShouldCreateNewSiteInstance(content::RenderFrameHost* render_frame_host,
+                                   content::BrowserContext* browser_context,
+                                   content::SiteInstance* current_instance,
+                                   const GURL& dest_url);
   void AddProcessPreferences(int process_id, ProcessPreferences prefs);
   void RemoveProcessPreferences(int process_id);
-  bool IsProcessObserved(int process_id) const;
-  bool IsRendererSandboxed(int process_id) const;
-  bool RendererUsesNativeWindowOpen(int process_id) const;
-  bool RendererDisablesPopups(int process_id) const;
-  std::string GetAffinityPreference(content::RenderFrameHost* rfh) const;
-  content::SiteInstance* GetSiteInstanceFromAffinity(
-      content::BrowserContext* browser_context,
-      const GURL& url,
-      content::RenderFrameHost* rfh) const;
-  void ConsiderSiteInstanceForAffinity(content::RenderFrameHost* rfh,
-                                       content::SiteInstance* site_instance);
+  bool IsProcessObserved(int process_id);
+  bool IsRendererSandboxed(int process_id);
+  bool RendererUsesNativeWindowOpen(int process_id);
+  bool RendererDisablesPopups(int process_id);
 
   // pending_render_process => web contents.
   std::map<int, content::WebContents*> pending_processes_;
@@ -188,14 +168,14 @@ class AtomBrowserClient : public brightray::BrowserClient,
   std::map<int, base::ProcessId> render_process_host_pids_;
 
   // list of site per affinity. weak_ptr to prevent instance locking
-  std::map<std::string, content::SiteInstance*> site_per_affinities_;
+  std::map<std::string, content::SiteInstance*> site_per_affinities;
 
   std::unique_ptr<AtomResourceDispatcherHostDelegate>
       resource_dispatcher_host_delegate_;
 
   Delegate* delegate_ = nullptr;
 
-  mutable base::Lock process_preferences_lock_;
+  base::Lock process_preferences_lock_;
   std::map<int, ProcessPreferences> process_preferences_;
 
   DISALLOW_COPY_AND_ASSIGN(AtomBrowserClient);

+ 0 - 2
brightray/browser/browser_main_parts.cc

@@ -192,8 +192,6 @@ void BrowserMainParts::InitializeFeatureList() {
   enable_features += std::string(",") + features::kSharedArrayBuffer.name;
   auto disable_features =
       cmd_line->GetSwitchValueASCII(switches::kDisableFeatures);
-  disable_features +=
-      std::string(",") + features::kSpareRendererForSitePerProcess.name;
 #if defined(OS_MACOSX)
   // Disable the V2 sandbox on macOS.
   // Chromium is going to use the system sandbox API of macOS for the sandbox

+ 21 - 5
lib/browser/guest-window-manager.js

@@ -90,9 +90,9 @@ const setupGuest = function (embedder, frameName, guest, options) {
   }
   const closedByUser = function () {
     embedder._sendInternal('ELECTRON_GUEST_WINDOW_MANAGER_WINDOW_CLOSED_' + guestId)
-    embedder.removeListener('current-render-view-deleted', closedByEmbedder)
+    embedder.removeListener('render-view-deleted', closedByEmbedder)
   }
-  embedder.once('current-render-view-deleted', closedByEmbedder)
+  embedder.once('render-view-deleted', closedByEmbedder)
   guest.once('closed', closedByUser)
   if (frameName) {
     frameToGuest.set(frameName, guest)
@@ -118,12 +118,28 @@ const createGuest = function (embedder, url, referrer, frameName, options, postD
   }
 
   guest = new BrowserWindow(options)
-  if (!options.webContents) {
+  if (!options.webContents || url !== 'about:blank') {
     // We should not call `loadURL` if the window was constructed from an
-    // existing webContents (window.open in a sandboxed renderer).
+    // existing webContents(window.open in a sandboxed renderer) and if the url
+    // is not 'about:blank'.
     //
     // Navigating to the url when creating the window from an existing
-    // webContents is not necessary (it will navigate there anyway).
+    // webContents would not be necessary(it will navigate there anyway), but
+    // apparently there's a bug that allows the child window to be scripted by
+    // the opener, even when the child window is from another origin.
+    //
+    // That's why the second condition(url !== "about:blank") is required: to
+    // force `OverrideSiteInstanceForNavigation` to be called and consequently
+    // spawn a new renderer if the new window is targeting a different origin.
+    //
+    // If the URL is "about:blank", then it is very likely that the opener just
+    // wants to synchronously script the popup, for example:
+    //
+    //     let popup = window.open()
+    //     popup.document.body.write('<h1>hello</h1>')
+    //
+    // The above code would not work if a navigation to "about:blank" is done
+    // here, since the window would be cleared of all changes in the next tick.
     const loadOptions = {
       httpReferrer: referrer
     }

+ 9 - 9
patches/common/chromium/cross_site_document_resource_handler.patch

@@ -22,14 +22,14 @@ index 907922701280b589bf11691342de0ec95cdec6a1..eaf8bac18f8e3a2735ce7ded60619909
  }
  
 diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
-index f713d0cfbf90665d921f56f4d828887ad1f7842c..4fe21468aee93a7cb3783220ebfe8dd100a3d1d5 100644
+index bb54b89bef5c6f32e7b4a056336c85494e2a04de..f069dfc4d2823b22eb0d90d28bc20236aeeddd04 100644
 --- a/content/public/browser/content_browser_client.cc
 +++ b/content/public/browser/content_browser_client.cc
-@@ -57,6 +57,10 @@ ContentBrowserClient::SiteInstanceForNavigationType ContentBrowserClient::Should
-   return SiteInstanceForNavigationType::ASK_CHROMIUM;
+@@ -47,6 +47,10 @@ void OverrideOnBindInterface(const service_manager::BindSourceInfo& remote_info,
+                                                          handle);
  }
  
-+bool ContentBrowserClient::ShouldBypassCORB(int render_process_id) const {
++bool ContentBrowserClient::ShouldBypassCORB(int render_process_id) {
 +  return false;
 +}
 +
@@ -37,15 +37,15 @@ index f713d0cfbf90665d921f56f4d828887ad1f7842c..4fe21468aee93a7cb3783220ebfe8dd1
      const MainFunctionParams& parameters) {
    return nullptr;
 diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
-index 4bf6b2b5f8110f539adc61858cfdc8f77f7ed08b..94454812e27d4d357eeee27cfc1e185386ea2003 100644
+index 2c22cb1cfe0dddc97c00e5f4ff89de6b18bc232f..a7b59095a887d566af9e74a646443fb49ea23bfc 100644
 --- a/content/public/browser/content_browser_client.h
 +++ b/content/public/browser/content_browser_client.h
-@@ -225,6 +225,9 @@ class CONTENT_EXPORT ContentBrowserClient {
-       content::RenderFrameHost* rfh,
-       content::SiteInstance* pending_site_instance){};
+@@ -205,6 +205,9 @@ class CONTENT_EXPORT ContentBrowserClient {
+       SiteInstance* candidate_site_instance,
+       SiteInstance** new_instance) {}
  
 +  // Electron: Allows bypassing CORB checks for a renderer process.
-+  virtual bool ShouldBypassCORB(int render_process_id) const;
++  virtual bool ShouldBypassCORB(int render_process_id);
 +
    // Allows the embedder to set any number of custom BrowserMainParts
    // implementations for the browser startup code. See comments in

+ 22 - 104
patches/common/chromium/frame_host_manager.patch

@@ -5,10 +5,10 @@ Subject: frame_host_manager.patch
 
 
 diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
-index 872e4609c94f1e052d623ae57c1279c72eb2c3f4..39d26adb60c50f88d19e824846519338083dc166 100644
+index 872e4609c94f1e052d623ae57c1279c72eb2c3f4..a59676004f2411631418bf12e2978623b9b27b53 100644
 --- a/content/browser/frame_host/render_frame_host_manager.cc
 +++ b/content/browser/frame_host/render_frame_host_manager.cc
-@@ -1960,6 +1960,17 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
+@@ -1960,6 +1960,18 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
    bool was_server_redirect = request.navigation_handle() &&
                               request.navigation_handle()->WasServerRedirect();
  
@@ -21,12 +21,13 @@ index 872e4609c94f1e052d623ae57c1279c72eb2c3f4..39d26adb60c50f88d19e824846519338
 +  scoped_refptr<SiteInstance> candidate_site_instance =
 +      speculative_render_frame_host_
 +          ? speculative_render_frame_host_->GetSiteInstance()
-+          : nullptr;
++          : content::SiteInstance::CreateForURL(browser_context,
++                                                request.common_params().url);
 +
    if (frame_tree_node_->IsMainFrame()) {
      // Renderer-initiated main frame navigations that may require a
      // SiteInstance swap are sent to the browser via the OpenURL IPC and are
-@@ -1979,6 +1990,51 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
+@@ -1979,6 +1991,19 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
                                                 request.common_params().url));
      no_renderer_swap_allowed |=
          request.from_begin_navigation() && !can_renderer_initiate_transfer;
@@ -36,49 +37,17 @@ index 872e4609c94f1e052d623ae57c1279c72eb2c3f4..39d26adb60c50f88d19e824846519338
 +         request.state() == NavigationRequest::FAILED) &&
 +        !speculative_render_frame_host_;
 +    // Gives user a chance to choose a custom site instance.
-+    SiteInstance* affinity_site_instance = nullptr;
-+    scoped_refptr<SiteInstance> overriden_site_instance;
-+    ContentBrowserClient::SiteInstanceForNavigationType siteInstanceType =
-+        GetContentClient()->browser()->ShouldOverrideSiteInstanceForNavigation(
-+            current_frame_host(), speculative_frame_host(), browser_context,
-+            request.common_params().url, has_response_started,
-+            &affinity_site_instance);
-+    switch (siteInstanceType) {
-+      case ContentBrowserClient::SiteInstanceForNavigationType::
-+          FORCE_CANDIDATE_OR_NEW:
-+        overriden_site_instance =
-+            candidate_site_instance
-+                ? candidate_site_instance
-+                : SiteInstance::CreateForURL(browser_context,
-+                                             request.common_params().url);
-+        break;
-+      case ContentBrowserClient::SiteInstanceForNavigationType::FORCE_CURRENT:
-+        overriden_site_instance = render_frame_host_->GetSiteInstance();
-+        break;
-+      case ContentBrowserClient::SiteInstanceForNavigationType::FORCE_AFFINITY:
-+        DCHECK(affinity_site_instance);
-+        overriden_site_instance =
-+            scoped_refptr<SiteInstance>(affinity_site_instance);
-+        break;
-+      case ContentBrowserClient::SiteInstanceForNavigationType::ASK_CHROMIUM:
-+        DCHECK(!affinity_site_instance);
-+        break;
-+      default:
-+        break;
-+    }
-+    if (overriden_site_instance) {
-+      if (siteInstanceType ==
-+          ContentBrowserClient::SiteInstanceForNavigationType::
-+              FORCE_CANDIDATE_OR_NEW) {
-+        GetContentClient()->browser()->RegisterPendingSiteInstance(
-+            render_frame_host_.get(), overriden_site_instance.get());
-+      }
-+      return overriden_site_instance;
-+    }
++    SiteInstance* client_custom_instance = nullptr;
++    GetContentClient()->browser()->OverrideSiteInstanceForNavigation(
++        render_frame_host_.get(), browser_context, request.common_params().url,
++        has_response_started, candidate_site_instance.get(),
++        &client_custom_instance);
++    if (client_custom_instance)
++      return scoped_refptr<SiteInstance>(client_custom_instance);
    } else {
      // Subframe navigations will use the current renderer, unless specifically
      // allowed to swap processes.
-@@ -1990,23 +2046,17 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
+@@ -1990,18 +2015,9 @@ RenderFrameHostManager::GetSiteInstanceForNavigationRequest(
    if (no_renderer_swap_allowed)
      return scoped_refptr<SiteInstance>(current_site_instance);
  
@@ -98,73 +67,22 @@ index 872e4609c94f1e052d623ae57c1279c72eb2c3f4..39d26adb60c50f88d19e824846519338
        request.common_params().transition,
        request.state() == NavigationRequest::FAILED,
        request.restore_type() != RestoreType::NONE, request.is_view_source(),
-       was_server_redirect);
- 
-+  GetContentClient()->browser()->RegisterPendingSiteInstance(
-+      render_frame_host_.get(), dest_site_instance.get());
-+
-   return dest_site_instance;
- }
- 
-diff --git a/content/public/browser/content_browser_client.cc b/content/public/browser/content_browser_client.cc
-index bb54b89bef5c6f32e7b4a056336c85494e2a04de..f713d0cfbf90665d921f56f4d828887ad1f7842c 100644
---- a/content/public/browser/content_browser_client.cc
-+++ b/content/public/browser/content_browser_client.cc
-@@ -47,6 +47,16 @@ void OverrideOnBindInterface(const service_manager::BindSourceInfo& remote_info,
-                                                          handle);
- }
- 
-+ContentBrowserClient::SiteInstanceForNavigationType ContentBrowserClient::ShouldOverrideSiteInstanceForNavigation(
-+    content::RenderFrameHost* current_rfh,
-+    content::RenderFrameHost* speculative_rfh,
-+    content::BrowserContext* browser_context,
-+    const GURL& url,
-+    bool has_request_started,
-+    content::SiteInstance** affinity_site_instance) const {
-+  return SiteInstanceForNavigationType::ASK_CHROMIUM;
-+}
-+
- BrowserMainParts* ContentBrowserClient::CreateBrowserMainParts(
-     const MainFunctionParams& parameters) {
-   return nullptr;
 diff --git a/content/public/browser/content_browser_client.h b/content/public/browser/content_browser_client.h
-index 3be31602689cb93b965729cc4e35cf6d23a8ec2f..4bf6b2b5f8110f539adc61858cfdc8f77f7ed08b 100644
+index 3be31602689cb93b965729cc4e35cf6d23a8ec2f..2c22cb1cfe0dddc97c00e5f4ff89de6b18bc232f 100644
 --- a/content/public/browser/content_browser_client.h
 +++ b/content/public/browser/content_browser_client.h
-@@ -194,8 +194,37 @@ CONTENT_EXPORT void OverrideOnBindInterface(
- // the observer interfaces.)
- class CONTENT_EXPORT ContentBrowserClient {
+@@ -196,6 +196,15 @@ class CONTENT_EXPORT ContentBrowserClient {
   public:
-+  // Identifies the type of site instance to use for a navigation.
-+  enum SiteInstanceForNavigationType {
-+    // Use either the candidate site instance or, if it doesn't exist
-+    // a new, unrelated site instance for the navigation.
-+    FORCE_CANDIDATE_OR_NEW = 0,
-+
-+    // Use the current site instance for the navigation.
-+    FORCE_CURRENT,
-+
-+    // Use the provided affinity site instance for the navigation.
-+    FORCE_AFFINITY,
-+
-+    // Delegate the site instance creation to Chromium.
-+    ASK_CHROMIUM
-+  };
    virtual ~ContentBrowserClient() {}
  
 +  // Electron: Allows overriding the SiteInstance when navigating.
-+  virtual SiteInstanceForNavigationType ShouldOverrideSiteInstanceForNavigation(
-+    content::RenderFrameHost* current_rfh,
-+    content::RenderFrameHost* speculative_rfh,
-+    content::BrowserContext* browser_context,
-+    const GURL& url,
-+    bool has_request_started,
-+    content::SiteInstance** affinity_site_instance) const;
-+
-+  // Electron: Registers a pending site instance during a navigation.
-+  virtual void RegisterPendingSiteInstance(
-+      content::RenderFrameHost* rfh,
-+      content::SiteInstance* pending_site_instance){};
++  virtual void OverrideSiteInstanceForNavigation(
++      RenderFrameHost* render_frame_host,
++      BrowserContext* browser_context,
++      const GURL& dest_url,
++      bool has_response_started,
++      SiteInstance* candidate_site_instance,
++      SiteInstance** new_instance) {}
 +
    // Allows the embedder to set any number of custom BrowserMainParts
    // implementations for the browser startup code. See comments in

+ 49 - 55
spec/api-browser-window-affinity-spec.js

@@ -26,73 +26,67 @@ describe('BrowserWindow with affinity module', () => {
     })
   }
 
-  function testAffinityProcessIds (name, webPreferences = {}) {
-    describe(name, () => {
-      let mAffinityWindow
-      before(done => {
-        createWindowWithWebPrefs({ affinity: myAffinityName, ...webPreferences })
-          .then((w) => {
-            mAffinityWindow = w
-            done()
-          })
-      })
-
-      after(done => {
-        closeWindow(mAffinityWindow, { assertSingleWindow: false }).then(() => {
-          mAffinityWindow = null
+  describe(`BrowserWindow with an affinity '${myAffinityName}'`, () => {
+    let mAffinityWindow
+    before(done => {
+      createWindowWithWebPrefs({ affinity: myAffinityName })
+        .then((w) => {
+          mAffinityWindow = w
           done()
         })
-      })
-
-      it('should have a different process id than a default window', done => {
-        createWindowWithWebPrefs({ ...webPreferences })
-          .then(w => {
-            const affinityID = mAffinityWindow.webContents.getOSProcessId()
-            const wcID = w.webContents.getOSProcessId()
+    })
 
-            expect(affinityID).to.not.equal(wcID, 'Should have different OS process IDs')
-            closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
-          })
+    after(done => {
+      closeWindow(mAffinityWindow, { assertSingleWindow: false }).then(() => {
+        mAffinityWindow = null
+        done()
       })
+    })
 
-      it(`should have a different process id than a window with a different affinity '${anotherAffinityName}'`, done => {
-        createWindowWithWebPrefs({ affinity: anotherAffinityName, ...webPreferences })
-          .then(w => {
-            const affinityID = mAffinityWindow.webContents.getOSProcessId()
-            const wcID = w.webContents.getOSProcessId()
+    it('should have a different process id than a default window', done => {
+      createWindowWithWebPrefs({})
+        .then(w => {
+          const affinityID = mAffinityWindow.webContents.getOSProcessId()
+          const wcID = w.webContents.getOSProcessId()
 
-            expect(affinityID).to.not.equal(wcID, 'Should have different OS process IDs')
-            closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
-          })
-      })
+          expect(affinityID).to.not.equal(wcID, 'Should have different OS process IDs')
+          closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
+        })
+    })
 
-      it(`should have the same OS process id than a window with the same affinity '${myAffinityName}'`, done => {
-        createWindowWithWebPrefs({ affinity: myAffinityName, ...webPreferences })
-          .then(w => {
-            const affinityID = mAffinityWindow.webContents.getOSProcessId()
-            const wcID = w.webContents.getOSProcessId()
+    it(`should have a different process id than a window with a different affinity '${anotherAffinityName}'`, done => {
+      createWindowWithWebPrefs({ affinity: anotherAffinityName })
+        .then(w => {
+          const affinityID = mAffinityWindow.webContents.getOSProcessId()
+          const wcID = w.webContents.getOSProcessId()
 
-            expect(affinityID).to.equal(wcID, 'Should have the same OS process ID')
-            closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
-          })
-      })
+          expect(affinityID).to.not.equal(wcID, 'Should have different OS process IDs')
+          closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
+        })
+    })
 
-      it(`should have the same OS process id than a window with an equivalent affinity '${myAffinityNameUpper}' (case insensitive)`, done => {
-        createWindowWithWebPrefs({ affinity: myAffinityNameUpper, ...webPreferences })
-          .then(w => {
-            const affinityID = mAffinityWindow.webContents.getOSProcessId()
-            const wcID = w.webContents.getOSProcessId()
+    it(`should have the same OS process id than a window with the same affinity '${myAffinityName}'`, done => {
+      createWindowWithWebPrefs({ affinity: myAffinityName })
+        .then(w => {
+          const affinityID = mAffinityWindow.webContents.getOSProcessId()
+          const wcID = w.webContents.getOSProcessId()
 
-            expect(affinityID).to.equal(wcID, 'Should have the same OS process ID')
-            closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
-          })
-      })
+          expect(affinityID).to.equal(wcID, 'Should have the same OS process ID')
+          closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
+        })
     })
-  }
 
-  testAffinityProcessIds(`BrowserWindow with an affinity '${myAffinityName}'`)
-  testAffinityProcessIds(`BrowserWindow with an affinity '${myAffinityName}' and sandbox enabled`, { sandbox: true })
-  testAffinityProcessIds(`BrowserWindow with an affinity '${myAffinityName}' and nativeWindowOpen enabled`, { nativeWindowOpen: true })
+    it(`should have the same OS process id than a window with an equivalent affinity '${myAffinityNameUpper}' (case insensitive)`, done => {
+      createWindowWithWebPrefs({ affinity: myAffinityNameUpper })
+        .then(w => {
+          const affinityID = mAffinityWindow.webContents.getOSProcessId()
+          const wcID = w.webContents.getOSProcessId()
+
+          expect(affinityID).to.equal(wcID, 'Should have the same OS process ID')
+          closeWindow(w, { assertSingleWindow: false }).then(() => { done() })
+        })
+    })
+  })
 
   describe(`BrowserWindow with an affinity : nodeIntegration=false`, () => {
     const preload = path.join(fixtures, 'module', 'send-later.js')

+ 35 - 32
spec/api-browser-window-spec.js

@@ -90,8 +90,6 @@ describe('BrowserWindow module', () => {
           res.end()
         } else if (req.url === '/navigate-302') {
           res.end(`<html><body><script>window.location='${server.url}/302'</script></body></html>`)
-        } else if (req.url === '/cross-site') {
-          res.end(`<html><body><h1>${req.url}</h1></body></html>`)
         } else {
           res.end()
         }
@@ -1473,6 +1471,29 @@ describe('BrowserWindow module', () => {
 
       const preload = path.join(fixtures, 'module', 'preload-sandbox.js')
 
+      // http protocol to simulate accessing another domain. This is required
+      // because the code paths for cross domain popups is different.
+      function crossDomainHandler (request, callback) {
+        // Disabled due to false positive in StandardJS
+        // eslint-disable-next-line standard/no-callback-literal
+        callback({
+          mimeType: 'text/html',
+          data: `<html><body><h1>${request.url}</h1></body></html>`
+        })
+      }
+
+      before((done) => {
+        protocol.interceptStringProtocol('http', crossDomainHandler, () => {
+          done()
+        })
+      })
+
+      after((done) => {
+        protocol.uninterceptProtocol('http', () => {
+          done()
+        })
+      })
+
       it('exposes ipcRenderer to preload script', (done) => {
         ipcMain.once('answer', function (event, test) {
           assert.strictEqual(test, 'preload')
@@ -1567,53 +1588,35 @@ describe('BrowserWindow module', () => {
         })
 
         ipcRenderer.send('set-web-preferences-on-next-new-window', w.webContents.id, 'preload', preload)
-        const openerWindowOpen = emittedOnce(ipcMain, 'opener-loaded')
-        w.loadFile(
-          path.join(fixtures, 'api', 'sandbox.html'),
-          { search: 'window-open-external' }
-        )
-
-        // Wait for a message from the main window saying that it's ready.
-        await openerWindowOpen
-
-        // Ask the opener to open a popup with window.opener.
-        const expectedPopupUrl = `${server.url}/cross-site` // Set in "sandbox.html".
-
         const browserWindowCreated = emittedOnce(app, 'browser-window-created')
-        w.webContents.send('open-the-popup', expectedPopupUrl)
+        const childLoaded = emittedOnce(ipcMain, 'child-loaded')
+        w.loadFile(path.join(fixtures, 'api', 'sandbox.html'), { search: 'window-open-external' })
+        const expectedPopupUrl = 'http://www.google.com/#q=electron' // Set in the "sandbox.html".
 
         // The page is going to open a popup that it won't be able to close.
         // We have to close it from here later.
         // XXX(alexeykuzmin): It will leak if the test fails too soon.
         const [, popupWindow] = await browserWindowCreated
 
-        // Ask the popup window for details.
-        const detailsAnswer = emittedOnce(ipcMain, 'child-loaded')
-        popupWindow.webContents.send('provide-details')
-        const [, openerIsNull, , locationHref] = await detailsAnswer
-        expect(openerIsNull).to.be.false('window.opener is null')
+        // Wait for a message from the popup's preload script.
+        const [, openerIsNull, html, locationHref] = await childLoaded
+        expect(openerIsNull).to.be.true('window.opener is not null')
+        expect(html).to.equal(`<h1>${expectedPopupUrl}</h1>`,
+          'looks like a http: request has not been intercepted locally')
         expect(locationHref).to.equal(expectedPopupUrl)
 
         // Ask the page to access the popup.
-        const touchPopupResult = emittedOnce(ipcMain, 'answer')
+        const answer = emittedOnce(ipcMain, 'answer')
         w.webContents.send('touch-the-popup')
-        const [, popupAccessMessage] = await touchPopupResult
-
-        // Ask the popup to access the opener.
-        const touchOpenerResult = emittedOnce(ipcMain, 'answer')
-        popupWindow.webContents.send('touch-the-opener')
-        const [, openerAccessMessage] = await touchOpenerResult
+        const [, exceptionMessage] = await answer
 
         // We don't need the popup anymore, and its parent page can't close it,
         // so let's close it from here before we run any checks.
         await closeWindow(popupWindow, { assertSingleWindow: false })
 
-        expect(popupAccessMessage).to.be.a('string',
+        expect(exceptionMessage).to.be.a('string',
           `child's .document is accessible from its parent window`)
-        expect(popupAccessMessage).to.match(/^Blocked a frame with origin/)
-        expect(openerAccessMessage).to.be.a('string',
-          `opener .document is accessible from a popup window`)
-        expect(openerAccessMessage).to.match(/^Blocked a frame with origin/)
+        expect(exceptionMessage).to.match(/^Blocked a frame with origin/)
       })
 
       it('should inherit the sandbox setting in opened windows', (done) => {

+ 0 - 79
spec/api-web-contents-spec.js

@@ -693,85 +693,6 @@ describe('webContents module', () => {
     })
   })
 
-  describe('render view deleted events', () => {
-    let server = null
-
-    before((done) => {
-      server = http.createServer((req, res) => {
-        const respond = () => {
-          if (req.url === '/redirect-cross-site') {
-            res.setHeader('Location', `${server.cross_site_url}/redirected`)
-            res.statusCode = 302
-            res.end()
-          } else if (req.url === '/redirected') {
-            res.end('<html><script>window.localStorage</script></html>')
-          } else {
-            res.end()
-          }
-        }
-        setTimeout(respond, 0)
-      })
-      server.listen(0, '127.0.0.1', () => {
-        server.url = `http://127.0.0.1:${server.address().port}`
-        server.cross_site_url = `http://localhost:${server.address().port}`
-        done()
-      })
-    })
-
-    after(() => {
-      server.close()
-      server = null
-    })
-
-    it('does not emit current-render-view-deleted when speculative RVHs are deleted', (done) => {
-      let currentRenderViewDeletedEmitted = false
-      w.webContents.once('destroyed', () => {
-        assert.strictEqual(currentRenderViewDeletedEmitted, false, 'current-render-view-deleted was emitted')
-        done()
-      })
-      const renderViewDeletedHandler = () => {
-        currentRenderViewDeletedEmitted = true
-      }
-      w.webContents.on('current-render-view-deleted', renderViewDeletedHandler)
-      w.webContents.on('did-finish-load', (e) => {
-        w.webContents.removeListener('current-render-view-deleted', renderViewDeletedHandler)
-        w.close()
-      })
-      w.loadURL(`${server.url}/redirect-cross-site`)
-    })
-
-    it('emits current-render-view-deleted if the current RVHs are deleted', (done) => {
-      let currentRenderViewDeletedEmitted = false
-      w.webContents.once('destroyed', () => {
-        assert.strictEqual(currentRenderViewDeletedEmitted, true, 'current-render-view-deleted wasn\'t emitted')
-        done()
-      })
-      w.webContents.on('current-render-view-deleted', () => {
-        currentRenderViewDeletedEmitted = true
-      })
-      w.webContents.on('did-finish-load', (e) => {
-        w.close()
-      })
-      w.loadURL(`${server.url}/redirect-cross-site`)
-    })
-
-    it('emits render-view-deleted if any RVHs are deleted', (done) => {
-      let rvhDeletedCount = 0
-      w.webContents.once('destroyed', () => {
-        const expectedRenderViewDeletedEventCount = 3 // 1 speculative upon redirection + 2 upon window close.
-        assert.strictEqual(rvhDeletedCount, expectedRenderViewDeletedEventCount, 'render-view-deleted wasn\'t emitted the expected nr. of times')
-        done()
-      })
-      w.webContents.on('render-view-deleted', () => {
-        rvhDeletedCount++
-      })
-      w.webContents.on('did-finish-load', (e) => {
-        w.close()
-      })
-      w.loadURL(`${server.url}/redirect-cross-site`)
-    })
-  })
-
   describe('setIgnoreMenuShortcuts(ignore)', () => {
     it('does not throw', () => {
       assert.strictEqual(w.webContents.setIgnoreMenuShortcuts(true), undefined)

+ 3 - 64
spec/chromium-spec.js

@@ -1015,19 +1015,11 @@ describe('chromium feature', () => {
       })
 
       it('cannot access localStorage', (done) => {
-        contents.on('crashed', (event, killed) => {
-          // Site isolation ON: process is killed for trying to access resources without permission.
-          if (process.platform !== 'win32') {
-            // Chromium on Windows does not set this flag correctly.
-            assert.strictEqual(killed, true, 'Process should\'ve been killed')
-          }
-          done()
-        })
-        ipcMain.once('local-storage-response', (event, message) => {
-          // Site isolation OFF: access is refused.
+        ipcMain.once('local-storage-response', (event, error) => {
           assert.strictEqual(
-            message,
+            error,
             'Failed to read the \'localStorage\' property from \'Window\': Access is denied for this document.')
+          done()
         })
         contents.loadURL(protocolName + '://host/localStorage')
       })
@@ -1068,59 +1060,6 @@ describe('chromium feature', () => {
         contents.loadURL(`${protocolName}://host/cookie`)
       })
     })
-
-    describe('can be accessed', () => {
-      let server = null
-      before((done) => {
-        server = http.createServer((req, res) => {
-          const respond = () => {
-            if (req.url === '/redirect-cross-site') {
-              res.setHeader('Location', `${server.cross_site_url}/redirected`)
-              res.statusCode = 302
-              res.end()
-            } else if (req.url === '/redirected') {
-              res.end('<html><script>window.localStorage</script></html>')
-            } else {
-              res.end()
-            }
-          }
-          setTimeout(respond, 0)
-        })
-        server.listen(0, '127.0.0.1', () => {
-          server.url = `http://127.0.0.1:${server.address().port}`
-          server.cross_site_url = `http://localhost:${server.address().port}`
-          done()
-        })
-      })
-
-      after(() => {
-        server.close()
-        server = null
-      })
-
-      const testLocalStorageAfterXSiteRedirect = (testTitle, extraPreferences = {}) => {
-        it(testTitle, (done) => {
-          const webPreferences = { show: false, ...extraPreferences }
-          w = new BrowserWindow(webPreferences)
-          let redirected = false
-          w.webContents.on('crashed', () => {
-            assert.fail('renderer crashed / was killed')
-          })
-          w.webContents.on('did-redirect-navigation', (event, url) => {
-            assert.strictEqual(url, `${server.cross_site_url}/redirected`)
-            redirected = true
-          })
-          w.webContents.on('did-finish-load', () => {
-            assert.strictEqual(redirected, true, 'didnt redirect')
-            done()
-          })
-          w.loadURL(`${server.url}/redirect-cross-site`)
-        })
-      }
-
-      testLocalStorageAfterXSiteRedirect('after a cross-site redirect')
-      testLocalStorageAfterXSiteRedirect('after a cross-site redirect in sandbox mode', { sandbox: true })
-    })
   })
 
   describe('websockets', () => {

+ 1 - 4
spec/fixtures/api/sandbox.html

@@ -81,9 +81,6 @@
       },
       'window-open-external': () => {
         addEventListener('load', () => {
-          ipcRenderer.once('open-the-popup', (event, url) => {
-            popup = open(url, '', 'top=65,left=55,width=505,height=605')
-          })
           ipcRenderer.once('touch-the-popup', () => {
             let errorMessage = null
             try {
@@ -93,7 +90,7 @@
             }
             ipcRenderer.send('answer', errorMessage)
           })
-          ipcRenderer.send('opener-loaded')
+          popup = open('http://www.google.com/#q=electron', '', 'top=65,left=55,width=505,height=605')
         })
       },
       'verify-ipc-sender': () => {

+ 1 - 10
spec/fixtures/module/preload-sandbox.js

@@ -23,16 +23,7 @@
     }
   } else if (location.href !== 'about:blank') {
     addEventListener('DOMContentLoaded', () => {
-      ipcRenderer.on('touch-the-opener', () => {
-        let errorMessage = null
-        try {
-          const openerDoc = opener.document // eslint-disable-line no-unused-vars
-        } catch (error) {
-          errorMessage = error.message
-        }
-        ipcRenderer.send('answer', errorMessage)
-      })
       ipcRenderer.send('child-loaded', window.opener == null, document.body.innerHTML, location.href)
-    })
+    }, false)
   }
 })()

+ 2 - 5
spec/fixtures/pages/storage/local_storage.html

@@ -1,11 +1,8 @@
 <script>
-  const {ipcRenderer} = require('electron')
-
-  let message = 'Ok'
   try {
     window.localStorage
   } catch (e) {
-    message = e.message
+    const {ipcRenderer} = require('electron')
+    ipcRenderer.send('local-storage-response', e.message)
   }
-  ipcRenderer.send('local-storage-response', message)
 </script>