Don't swap processes for chrome:// subframes.

content/browser/frame_host/navigation_handle_impl.cc

Index: content/browser/frame_host/navigation_handle_impl.cc
diff --git a/content/browser/frame_host/navigation_handle_impl.cc b/content/browser/frame_host/navigation_handle_impl.cc
index 30536bd306c76c4da490f9ea142210612c907d0c..5258833bd734acd9bb8176eebcf3bf6f1b57ab40 100644
--- a/content/browser/frame_host/navigation_handle_impl.cc
+++ b/content/browser/frame_host/navigation_handle_impl.cc
@@ -736,34 +736,17 @@ bool NavigationHandleImpl::MaybeTransferAndProceedInternal() {
   }
 
   // The content embedder can decide that a transfer to a different process is
-  // required for this URL.
-  bool should_transfer =
-      GetContentClient()->browser()->ShouldSwapProcessesForRedirect(
+  // required for this URL. Start the transfer if needed.
+  if (GetContentClient()->browser()->ShouldSwapProcessesForRedirect(
           frame_tree_node_->navigator()->GetController()->GetBrowserContext(),
-          original_url_, url_);
-
-  RenderFrameHostManager* manager =
-      render_frame_host_->frame_tree_node()->render_manager();
-
-  // In the site-per-process model, the RenderFrameHostManager may also decide
-  // (independently from the content embedder's ShouldSwapProcessesForRedirect
-  // above) that a process transfer is needed. Process transfers are skipped for
-  // WebUI processes for now, since e.g. chrome://settings has multiple
-  // "cross-site" chrome:// frames, and that doesn't yet work cross-process.
-  if (SiteIsolationPolicy::AreCrossProcessFramesPossible() &&
-      !ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(

[Charlie Reis, 2017/01/21 00:46:44]

Removing this whole block looks wrong to me.  I expected to only remove this one
line, such that all navigations get sent through
IsRendererTransferNeededForNavigation.

Hopefully we'll see test failures?  If not, we need to look closely at whether
IsRendererTransferNeededForNavigation is still needed.

[nasko, 2017/01/23 16:51:49]

Yes, you are correct. I thought I ran successfully locally tests with this whole
block removed, but apparently it wasn't exactly that.

-          render_frame_host_->GetProcess()->GetID())) {
-    should_transfer |= manager->IsRendererTransferNeededForNavigation(
-        render_frame_host_, url_);
-  }
-
-  // Start the transfer if needed.
-  if (should_transfer) {
+          original_url_, url_)) {
     // This may destroy the NavigationHandle if the transfer fails.
     base::WeakPtr<NavigationHandleImpl> weak_self = weak_factory_.GetWeakPtr();
-    manager->OnCrossSiteResponse(render_frame_host_, request_id_,
-                                 redirect_chain_, sanitized_referrer_,
-                                 transition_, should_replace_current_entry_);
+    render_frame_host_->frame_tree_node()
+        ->render_manager()
+        ->OnCrossSiteResponse(render_frame_host_, request_id_, redirect_chain_,
+                              sanitized_referrer_, transition_,
+                              should_replace_current_entry_);
     if (!weak_self)
       return false;
   }