Change BrowsingInstance when changing WebUI type.

content/browser/frame_host/render_frame_host_manager.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/frame_host/render_frame_host_manager.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/command_line.h"
@@ skipping 1224 matching lines @@
   }
 
   // Check for reasons to swap processes even if we are in a process model that
   // doesn't usually swap (e.g., process-per-tab).  Any time we return true,
   // the new_entry will be rendered in a new SiteInstance AND BrowsingInstance.
   BrowserContext* browser_context =
       delegate_->GetControllerForRenderManager().GetBrowserContext();
 
   // Don't force a new BrowsingInstance for debug URLs that are handled in the
   // renderer process, like javascript: or chrome://crash.
   if (IsRendererDebugURL(new_effective_url))

[Charlie Reis, 2015/10/23 23:09:50]

What if we also returned false for about:blank?  That seems safe to load in any
BI, and it eliminates the concern below.

[nasko, 2015/10/23 23:46:53]

Agree!

     return false;
 
   // For security, we should transition between processes when one is a Web UI
   // page and one isn't.

[Charlie Reis, 2015/10/23 23:09:50]

nit: , or if the Web UI types differ.

[nasko, 2015/10/23 23:46:53]

Done.

   if (ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
           render_frame_host_->GetProcess()->GetID()) ||
       WebUIControllerFactoryRegistry::GetInstance()->UseWebUIForURL(
           browser_context, current_effective_url)) {
     // If so, force a swap if destination is not an acceptable URL for Web UI.
     // Here, data URLs are never allowed.
     if (!WebUIControllerFactoryRegistry::GetInstance()->IsURLAcceptableForWebUI(

[Charlie Reis, 2015/10/23 23:09:50]

I'm concerned about the about:blank case.  Before, we said it was allowed to
stay (per https://crbug.com/42547#c13, which found that about:blank could be
loaded in WebUI processes).

Now we'll probably find that the WebUIType of about:blank is kWebUINone, so
we'll say it requires a swap.

Do we want to preserve that behavior?  I'm not sure if WebUI pages actually
navigate to it anymore in the main frame, but it could definitely come up in
subframes.

If so, let's add a check to the ForceSwapAfterWebUIBindings test to verify that
the about:blank page stays in process.

[nasko, 2015/10/23 23:46:53]

Acknowledged.

             browser_context, new_effective_url)) {
       return true;
     }
+
+    // Force swap if the current WebUI type differs from the one for the
+    // destination.
+    if (WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
+            browser_context, current_effective_url) !=
+        WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
+            browser_context, new_effective_url)) {
+      return true;
+    }
   } else {
     // Force a swap if it's a Web UI URL.
     if (WebUIControllerFactoryRegistry::GetInstance()->UseWebUIForURL(
             browser_context, new_effective_url)) {
       return true;
     }
   }
 
   // Check with the content client as well.  Important to pass
   // current_effective_url here, which uses the SiteInstance's site if there is
@@ skipping 1378 matching lines @@
 int RenderFrameHostManager::GetOpenerRoutingID(SiteInstance* instance) {
   if (!frame_tree_node_->opener())
     return MSG_ROUTING_NONE;
 
   return frame_tree_node_->opener()
       ->render_manager()
       ->GetRoutingIdForSiteInstance(instance);
 }
 
 }  // namespace content