Make sure new render is used when navigating away from about:blank in WebUI renderer.

content/browser/frame_host/render_frame_host_manager.cc

Index: content/browser/frame_host/render_frame_host_manager.cc
diff --git a/content/browser/frame_host/render_frame_host_manager.cc b/content/browser/frame_host/render_frame_host_manager.cc
index e6e93d96f64bbbbcb7c1d75d017efb528f705af2..05a838f077426a846823f61e81cdc5e1dde08041 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -36,6 +36,7 @@
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/user_metrics.h"
 #include "content/public/browser/web_ui_controller.h"
+#include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/referrer.h"
 #include "content/public/common/url_constants.h"
@@ -747,7 +748,8 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForNavigation(
   SiteInstance* new_instance = current_instance;
 
   // We do not currently swap processes for navigations in webview tag guests.
-  bool is_guest_scheme = current_instance->GetSiteURL().SchemeIs(kGuestScheme);
+  if(current_instance->GetSiteURL().SchemeIs(kGuestScheme))

[Charlie Reis, 2014/10/20 19:37:53]

nit: Space after if.
(git cl format will help catch style errors like this.)

This change looks good-- I like returning early here rather than weaving
is_guest_scheme into everything.

[Krzysztof Olczyk, 2014/10/21 08:27:05]

Done.

+    return current_instance;
 
   // Determine if we need a new BrowsingInstance for this entry.  If true, this
   // implies that it will get a new SiteInstance (and likely process), and that
@@ -765,14 +767,19 @@ SiteInstance* RenderFrameHostManager::GetSiteInstanceForNavigation(
       render_frame_host_->GetSiteInstance()->GetSiteURL();
   bool current_is_view_source_mode = current_entry ?
       current_entry->IsViewSourceMode() : dest_is_view_source_mode;
-  bool force_swap = !is_guest_scheme &&
-      ShouldSwapBrowsingInstancesForNavigation(
-          current_effective_url,
-          current_is_view_source_mode,
-          dest_instance,
-          SiteInstanceImpl::GetEffectiveURL(browser_context, dest_url),
-          dest_is_view_source_mode);
-  if (!is_guest_scheme && (ShouldTransitionCrossSite() || force_swap)) {
+  int current_enabled_bindings =
+      render_frame_host_->render_view_host()->GetEnabledBindings();
+  bool navigating_away_from_blank_page_in_webui =
+      current_effective_url.spec() == url::kAboutBlankURL &&

[Charlie Reis, 2014/10/20 19:37:53]

This seems much too specific to me.  The underlying flaw here is that we aren't
checking the process bindings when deciding whether to force a swap.  The same
bug could come up with other URLs like data:, etc.

I suppose we don't usually notice this because GetSiteInstanceForURL normally
catches this case (via HasWrongProcessForURL).  However, I think
ShouldSwapBrowsingInstancesForNavigation should check both the URL and the
bindings when considering WebUI.  

I imagine we should fix this using the {Has/Use}WebUIBindings calls that also
are in RenderProcessHostImpl::IsSuitableHost.  I don't think we want to just
call HasWrongProcessForURL because that might force a swap for hosted apps,
which need to stay in the same BrowsingInstance.

[Krzysztof Olczyk, 2014/10/21 08:27:05]

Indeed, it looks like it makes more sense to put such a check there.
I modified ShouldSwapBrowsingInstancesForNavigation with HasWebUIBindings check.

+      (current_enabled_bindings & BINDINGS_POLICY_WEB_UI);
+  bool force_swap = ShouldSwapBrowsingInstancesForNavigation(
+      current_effective_url,
+      current_is_view_source_mode,
+      dest_instance,
+      SiteInstanceImpl::GetEffectiveURL(browser_context, dest_url),
+      dest_is_view_source_mode);
+  if (ShouldTransitionCrossSite() || force_swap ||
+      navigating_away_from_blank_page_in_webui) {

[Charlie Reis, 2014/10/20 19:37:53]

I'd rather not make this more complicated in this location. 
ShouldSwapBrowsingInstancesForNavigation is the right place to be adding checks
that force a swap, rather than appending it as another condition here.  (And I
do think we should force a BrowsingInstance swap if the bindings situation
requires it.)

[Krzysztof Olczyk, 2014/10/21 08:27:05]

Done.

     new_instance = GetSiteInstanceForURL(
         dest_url,
         dest_instance,