Move WebUI ownership from the RenderFrameHostManager to the RenderFrameHost.

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 1ffbbaa4861f5795c19410c982d11b26da767cdd..6572650f03d7b999703ea3ec988fdc02d0d6a80c 100644
--- a/content/browser/frame_host/render_frame_host_manager.cc
+++ b/content/browser/frame_host/render_frame_host_manager.cc
@@ -30,7 +30,6 @@
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
-#include "content/browser/webui/web_ui_impl.h"
 #include "content/common/frame_messages.h"
 #include "content/common/site_isolation_policy.h"
 #include "content/common/view_messages.h"
@@ -39,7 +38,6 @@
 #include "content/public/browser/render_widget_host_iterator.h"
 #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/browser_plugin_guest_mode.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/referrer.h"
@@ -220,7 +218,6 @@ RenderFrameHostManager::RenderFrameHostManager(
       render_widget_delegate_(render_widget_delegate),
       proxy_hosts_(new RenderFrameProxyHostMap(this)),
       interstitial_page_(nullptr),
-      should_reuse_web_ui_(false),
       weak_factory_(this) {
   DCHECK(frame_tree_node_);
 }
@@ -245,10 +242,6 @@ RenderFrameHostManager::~RenderFrameHostManager() {
   // the current RenderFrameHost and uses it during its destructor.
   ResetProxyHosts();
 
-  // Release the WebUI prior to resetting the current RenderFrameHost, as the
-  // WebUI accesses the RenderFrameHost during cleanup.
-  web_ui_.reset();
-
   // We should always have a current RenderFrameHost except in some tests.
   SetRenderFrameHost(scoped_ptr<RenderFrameHostImpl>());
 }
@@ -265,9 +258,9 @@ void RenderFrameHostManager::Init(BrowserContext* browser_context,
     site_instance = SiteInstance::Create(browser_context);
 
   int flags = delegate_->IsHidden() ? CREATE_RF_HIDDEN : 0;
-  SetRenderFrameHost(CreateRenderFrameHost(site_instance, view_routing_id,
-                                           frame_routing_id, widget_routing_id,
-                                           flags));
+  SetRenderFrameHost(CreateRenderFrameHost(
+      site_instance, view_routing_id, frame_routing_id, widget_routing_id,
+      flags, GURL(), NavigationEntryImpl::kInvalidBindings));
 
   // Notify the delegate of the creation of the current RenderFrameHost.
   // Do this only for subframes, as the main frame case is taken care of by
@@ -351,27 +344,6 @@ void RenderFrameHostManager::RemoveOuterDelegateFrame() {
       outer_delegate_frame_tree_node);
 }
 
-void RenderFrameHostManager::SetPendingWebUI(const GURL& url, int bindings) {
-  pending_web_ui_ = CreateWebUI(url, bindings);
-  pending_and_current_web_ui_.reset();
-}
-
-scoped_ptr<WebUIImpl> RenderFrameHostManager::CreateWebUI(const GURL& url,
-                                                          int bindings) {
-  scoped_ptr<WebUIImpl> new_web_ui(delegate_->CreateWebUIForRenderManager(url));
-
-  // If we have assigned (zero or more) bindings to this NavigationEntry in the
-  // past, make sure we're not granting it different bindings than it had
-  // before.  If so, note it and don't give it any bindings, to avoid a
-  // potential privilege escalation.
-  if (new_web_ui && bindings != NavigationEntryImpl::kInvalidBindings &&
-      new_web_ui->GetBindings() != bindings) {
-    RecordAction(base::UserMetricsAction("ProcessSwapBindingsMismatch_RVHM"));
-    return nullptr;
-  }
-  return new_web_ui.Pass();
-}
-
 RenderFrameHostImpl* RenderFrameHostManager::Navigate(
     const GURL& dest_url,
     const FrameNavigationEntry& frame_entry,
@@ -418,7 +390,7 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
     CreateOpenerProxies(dest_render_frame_host->GetSiteInstance(),
                         frame_tree_node_);
     if (!InitRenderView(dest_render_frame_host->render_view_host(),
-                        MSG_ROUTING_NONE))
+                        MSG_ROUTING_NONE, pending_web_ui()))
       return nullptr;
 
     // Now that we've created a new renderer, be sure to hide it if it isn't
@@ -670,13 +642,12 @@ void RenderFrameHostManager::CommitPendingIfNecessary(
     RenderFrameHostImpl* render_frame_host,
     bool was_caused_by_user_gesture) {
   if (!pending_render_frame_host_ && !speculative_render_frame_host_) {
-    DCHECK_IMPLIES(should_reuse_web_ui_, web_ui_);
 
     // We should only hear this from our current renderer.
     DCHECK_EQ(render_frame_host_, render_frame_host);
 
-    // Even when there is no pending RVH, there may be a pending Web UI.
-    if (pending_web_ui() || speculative_web_ui_)
+    // If current RenderFrameHost has a pending Web UI, commit it.
+    if (render_frame_host_->pending_web_ui())
       CommitPending();
     return;
   }
@@ -1009,21 +980,12 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
     CleanUpNavigation();
     navigation_rfh = render_frame_host_.get();
 
-    // As SiteInstances are the same, check if the WebUI should be reused.
-    const NavigationEntry* current_navigation_entry =
-        delegate_->GetLastCommittedNavigationEntryForRenderManager();
-    should_reuse_web_ui_ = ShouldReuseWebUI(current_navigation_entry,
-                                            request.common_params().url);
-    if (!should_reuse_web_ui_) {
-      speculative_web_ui_ = CreateWebUI(request.common_params().url,
-                                        request.bindings());
-      // Make sure the current RenderViewHost has the right bindings.
-      if (speculative_web_ui() &&
-          !render_frame_host_->GetProcess()->IsForGuestsOnly()) {
-        render_frame_host_->render_view_host()->AllowBindings(
-            speculative_web_ui()->GetBindings());
-      }
-    }
+    // As SiteInstances are the same, make the RFH update its possible pending
+    // WebUI.
+    render_frame_host_->UpdatePendingWebUI(request.common_params().url,
+                                           request.bindings());
+    DCHECK(!speculative_render_frame_host_);
+    DCHECK(speculative_web_ui() == render_frame_host_->pending_web_ui());
   } else {
     // If the SiteInstance for the final URL doesn't match the one from the
     // speculatively created RenderFrameHost, create a new RenderFrameHost using
@@ -1036,8 +998,18 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
           request.common_params().url, current_site_instance,
           dest_site_instance.get(), request.bindings());
       DCHECK(success);
+    } else {
+      // When reusing an existing speculative RenderFrameHost its active WebUI
+      // must be updated to make sure it matches the current URL. As it is not a
+      // new RenderFrameHost instance it must be done by updating the pending
+      // WebUI and immediately making it active.
+      speculative_render_frame_host_->UpdatePendingWebUI(
+          request.common_params().url, request.bindings());
+      speculative_render_frame_host_->CommitPendingWebUI();
     }
     DCHECK(speculative_render_frame_host_);
+    DCHECK_EQ(speculative_web_ui(), speculative_render_frame_host_->web_ui());
+
     navigation_rfh = speculative_render_frame_host_.get();
 
     // Check if our current RFH is live.
@@ -1059,7 +1031,8 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
   if (!navigation_rfh->IsRenderFrameLive()) {
     // Recreate the opener chain.
     CreateOpenerProxies(navigation_rfh->GetSiteInstance(), frame_tree_node_);
-    if (!InitRenderView(navigation_rfh->render_view_host(), MSG_ROUTING_NONE)) {
+    if (!InitRenderView(navigation_rfh->render_view_host(), MSG_ROUTING_NONE,
+                        speculative_web_ui())) {
       return nullptr;
     }
 
@@ -1082,8 +1055,7 @@ RenderFrameHostImpl* RenderFrameHostManager::GetFrameHostForNavigation(
 void RenderFrameHostManager::CleanUpNavigation() {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
-  speculative_web_ui_.reset();
-  should_reuse_web_ui_ = false;
+  render_frame_host_->DiscardPendingWebUI();

[nasko, 2015/10/01 20:05:16]

Why are we touching the pending WebUI of the current RFH in PlzNavigate mode? Do
we avoid creating a speculative RFH in case the active RFH already hosts WebUI
page?

[carlosk, 2015/10/05 16:59:58]

For *both* navigation implementations, when the current RFH/SI is reused but
it's WebUI has to change, a pending one will be created in it. There will be no
pending/speculative RFH in this case.

That section of the design doc I mentioned in the previous comment should make
this clearer.

   if (speculative_render_frame_host_)
     DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());
 }
@@ -1267,18 +1239,6 @@ bool RenderFrameHostManager::ShouldSwapBrowsingInstancesForNavigation(
   return false;
 }
 
-bool RenderFrameHostManager::ShouldReuseWebUI(
-    const NavigationEntry* current_entry,
-    const GURL& new_url) const {
-  NavigationControllerImpl& controller =
-      delegate_->GetControllerForRenderManager();
-  return current_entry && web_ui_ &&
-      (WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
-          controller.GetBrowserContext(), current_entry->GetURL()) ==
-       WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
-          controller.GetBrowserContext(), new_url));
-}
-
 SiteInstance* RenderFrameHostManager::GetSiteInstanceForNavigation(
     const GURL& dest_url,
     SiteInstance* source_instance,
@@ -1562,7 +1522,9 @@ const GURL& RenderFrameHostManager::GetCurrentURLForSiteInstance(
 
 void RenderFrameHostManager::CreatePendingRenderFrameHost(
     SiteInstance* old_instance,
-    SiteInstance* new_instance) {
+    SiteInstance* new_instance,
+    const GURL& url,
+    int bindings) {
   int create_render_frame_flags = 0;
   if (delegate_->IsHidden())
     create_render_frame_flags |= CREATE_RF_HIDDEN;
@@ -1580,8 +1542,8 @@ void RenderFrameHostManager::CreatePendingRenderFrameHost(
   CreateProxiesForNewRenderFrameHost(old_instance, new_instance);
 
   // Create a non-swapped-out RFH with the given opener.
-  pending_render_frame_host_ = CreateRenderFrame(
-      new_instance, pending_web_ui(), create_render_frame_flags, nullptr);
+  pending_render_frame_host_ = CreateRenderFrameInternal(
+      new_instance, url, bindings, create_render_frame_flags, nullptr);
 }
 
 void RenderFrameHostManager::CreateProxiesForNewRenderFrameHost(
@@ -1637,7 +1599,9 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
     int32 view_routing_id,
     int32 frame_routing_id,
     int32 widget_routing_id,
-    int flags) {
+    int flags,
+    const GURL& dest_url,
+    int past_bindings) {
   if (frame_routing_id == MSG_ROUTING_NONE)
     frame_routing_id = site_instance->GetProcess()->GetNextRoutingID();
 
@@ -1658,7 +1622,7 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrameHost(
   return RenderFrameHostFactory::Create(
       site_instance, render_view_host, render_frame_delegate_,
       render_widget_delegate_, frame_tree, frame_tree_node_, frame_routing_id,
-      widget_routing_id, flags);
+      widget_routing_id, flags, dest_url, past_bindings);
 }
 
 // PlzNavigate
@@ -1669,12 +1633,6 @@ bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
     int bindings) {
   CHECK(new_instance);
   CHECK_NE(old_instance, new_instance);
-  CHECK(!should_reuse_web_ui_);
-
-  // Note: |speculative_web_ui_| must be initialized before starting the
-  // |speculative_render_frame_host_| creation steps otherwise the WebUI
-  // won't be properly initialized.
-  speculative_web_ui_ = CreateWebUI(url, bindings);
 
   // The process for the new SiteInstance may (if we're sharing a process with
   // another host that already initialized it) or may not (we have our own
@@ -1688,22 +1646,27 @@ bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
   int create_render_frame_flags = 0;
   if (delegate_->IsHidden())
     create_render_frame_flags |= CREATE_RF_HIDDEN;
-  speculative_render_frame_host_ =
-      CreateRenderFrame(new_instance, speculative_web_ui_.get(),
-                        create_render_frame_flags, nullptr);
+  speculative_render_frame_host_ = CreateRenderFrameInternal(
+      new_instance, url, bindings, create_render_frame_flags, nullptr);
 
-  if (!speculative_render_frame_host_) {
-    speculative_web_ui_.reset();
-    return false;
-  }
   return true;
 }
 
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     SiteInstance* instance,
-    WebUIImpl* web_ui,
     int flags,
     int* view_routing_id_ptr) {
+  return CreateRenderFrameInternal(instance, GURL(),
+                                   NavigationEntryImpl::kInvalidBindings, flags,
+                                   view_routing_id_ptr);
+}
+
+scoped_ptr<RenderFrameHostImpl>
+RenderFrameHostManager::CreateRenderFrameInternal(SiteInstance* instance,
+                                                  const GURL& url,
+                                                  int bindings,

[nasko, 2015/10/01 20:05:16]

I wonder if we can avoid creating this extra method and passing the URL and
bindings. Can't we explicitly call the WebUI methods on the RFH once it is
created?

[carlosk, 2015/10/05 16:59:58]

That's the solution I had before. PTAL at the discussion about it
(https://codereview.chromium.org/1352813006/diff/1/content/browser/frame_host/...)
and let me know WDYT.

[nasko, 2015/10/07 23:49:32]

The argument that we always have to call the extra method is valid. I'm not
excited about the internal method either. It might be useful to ping creis@
about this and see his plans on session history and how the URL/bindings will be
stored. Ideally, we should have one method and not needing the indirection to
Internal, but I haven't looked at all the callers.

[carlosk, 2015/10/09 17:29:10]

OK, I'll ping cresi@.

Passing the extra parameters is the cost for eliminating InitializeWebUI which
was called in all RFH constructions. It seemed the right thing to do (but this
might change depending on dbeam@'s answer on the RenderViewCreated|Reused
logic).

But the new internal method was added to simplify the call for current external
callers who would never try to create a WebUI anyway. Another option is to
revert to a single public method and make callers pass default values for URL
and bindings.

[carlosk, 2015/10/13 14:40:26]

After a VC with Nasko I reverted to having the RFHI::InitializeWebUI and
eliminated the two extra creation parameters.

I am still keeping the CreateRenderFrameInternal because the public version
doesn't need to pass those parameters (none of the current public use cases
needs to create a WebUI).

[Charlie Reis, 2015/10/13 17:03:27]

I chatted with Nasko to get the context, and it sounds like you two have a
direction forward.  We agreed I should hold off on the review for now, to avoid
having too many cooks in the kitchen.  :)

+                                                  int flags,
+                                                  int* view_routing_id_ptr) {
   bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
   bool swapped_out_forbidden =
       SiteIsolationPolicy::IsSwappedOutStateForbidden();
@@ -1738,6 +1701,8 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     // Prevent the process from exiting while we're trying to use it.
     if (!swapped_out) {
       new_render_frame_host = proxy->PassFrameHostOwnership();
+      new_render_frame_host->UpdatePendingWebUI(url, bindings);
+      new_render_frame_host->CommitPendingWebUI();
       new_render_frame_host->GetProcess()->AddPendingView();
 
       proxy_hosts_->Remove(instance->GetId());
@@ -1761,8 +1726,9 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
       widget_routing_id = instance->GetProcess()->GetNextRoutingID();
     }
 
-    new_render_frame_host = CreateRenderFrameHost(
-        instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE, widget_routing_id, flags);
+    new_render_frame_host =
+        CreateRenderFrameHost(instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE,
+                              widget_routing_id, flags, url, bindings);
     RenderViewHostImpl* render_view_host =
         new_render_frame_host->render_view_host();
     int proxy_routing_id = MSG_ROUTING_NONE;
@@ -1781,7 +1747,9 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     }
 
     if (frame_tree_node_->IsMainFrame()) {
-      success = InitRenderView(render_view_host, proxy_routing_id);
+      success = InitRenderView(
+          render_view_host, proxy_routing_id,
+          swapped_out ? nullptr : new_render_frame_host->web_ui());
 
       // If we are reusing the RenderViewHost and it doesn't already have a
       // RenderWidgetHostView, we need to create one if this is the main frame.
@@ -1820,21 +1788,6 @@ scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     }
   }
 
-  // When a new RenderView is created by the renderer process, the new
-  // WebContents gets a RenderViewHost in the SiteInstance of its opener
-  // WebContents. If not used in the first navigation, this RVH is swapped out
-  // and is not granted bindings, so we may need to grant them when swapping it
-  // in.
-  if (web_ui && !new_render_frame_host->GetProcess()->IsForGuestsOnly()) {
-    int required_bindings = web_ui->GetBindings();
-    RenderViewHost* render_view_host =
-        new_render_frame_host->render_view_host();
-    if ((render_view_host->GetEnabledBindings() & required_bindings) !=
-            required_bindings) {
-      render_view_host->AllowBindings(required_bindings);
-    }
-  }
-
   // Returns the new RFH if it isn't swapped out.
   if (success && !swapped_out) {
     DCHECK(new_render_frame_host->GetSiteInstance() == instance);
@@ -1875,7 +1828,7 @@ int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
 
   if (SiteIsolationPolicy::IsSwappedOutStateForbidden() &&
       frame_tree_node_->IsMainFrame()) {
-    InitRenderView(render_view_host, proxy->GetRoutingID());
+    InitRenderView(render_view_host, proxy->GetRoutingID(), nullptr);
     proxy->set_render_frame_proxy_created(true);
   } else {
     proxy->InitRenderFrameProxy();
@@ -1910,7 +1863,7 @@ void RenderFrameHostManager::EnsureRenderViewInitialized(
   if (!proxy)
     return;
 
-  InitRenderView(render_view_host, proxy->GetRoutingID());
+  InitRenderView(render_view_host, proxy->GetRoutingID(), nullptr);
   proxy->set_render_frame_proxy_created(true);
 }
 
@@ -1945,7 +1898,8 @@ void RenderFrameHostManager::SetRWHViewForInnerContents(
 
 bool RenderFrameHostManager::InitRenderView(
     RenderViewHostImpl* render_view_host,
-    int proxy_routing_id) {
+    int proxy_routing_id,
+    WebUIImpl* dest_web_ui) {

[nasko, 2015/10/01 20:05:16]

Why pass this as a parameter? Can't we just get it from the RFH?

[carlosk, 2015/10/05 16:59:58]

No, because the RFH is not yet set as current/pending/speculative at this point
for some code paths and so we can't get the WebUI instance we need to act on
here. For instance when this is called from CreateRenderFrameInternal; see the
modifications there.

[nasko, 2015/10/07 23:49:32]

I think we can do better, but let's revisit this in a later CL as things get
simplified.

[carlosk, 2015/10/09 17:29:10]

Acknowledged. Let's talk and figure out what can be done.

   // Ensure the renderer process is initialized before creating the
   // RenderView.
   if (!render_view_host->GetProcess()->Init())
@@ -1955,26 +1909,13 @@ bool RenderFrameHostManager::InitRenderView(
   if (render_view_host->IsRenderViewLive())
     return true;
 
-  // If the ongoing navigation is to a WebUI and the RenderView is not in a
-  // guest process, tell the RenderViewHost about any bindings it will need
-  // enabled.
-  WebUIImpl* dest_web_ui = nullptr;
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableBrowserSideNavigation)) {
-    dest_web_ui =
-        should_reuse_web_ui_ ? web_ui_.get() : speculative_web_ui_.get();
-  } else {
-    dest_web_ui = pending_web_ui();
-  }
-  if (dest_web_ui && !render_view_host->GetProcess()->IsForGuestsOnly()) {
-    render_view_host->AllowBindings(dest_web_ui->GetBindings());
-  } else {
-    // Ensure that we don't create an unprivileged RenderView in a WebUI-enabled
-    // process unless it's swapped out.
-    if (render_view_host->is_active()) {
-      CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
-                render_view_host->GetProcess()->GetID()));
-    }
+  // If the ongoing navigation is not to a WebUI or the RenderView is in a
+  // guest process, ensure that we don't create an unprivileged RenderView in a
+  // WebUI-enabled process unless it's swapped out.
+  if ((!dest_web_ui || render_view_host->GetProcess()->IsForGuestsOnly()) &&
+      render_view_host->is_active()) {
+    CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
+        render_view_host->GetProcess()->GetID()));
   }
 
   int opener_frame_routing_id =
@@ -2062,42 +2003,17 @@ int RenderFrameHostManager::GetRoutingIdForSiteInstance(
 void RenderFrameHostManager::CommitPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CommitPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
-  bool browser_side_navigation =
-      base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableBrowserSideNavigation);
-
   // First check whether we're going to want to focus the location bar after
   // this commit.  We do this now because the navigation hasn't formally
-  // committed yet, so if we've already cleared |pending_web_ui_| the call chain
+  // committed yet, so if we've already cleared the pending WebUI the call chain
   // this triggers won't be able to figure out what's going on.
   bool will_focus_location_bar = delegate_->FocusLocationBarByDefault();
 
-  // Next commit the Web UI, if any. Either replace |web_ui_| with
-  // |pending_web_ui_|, or clear |web_ui_| if there is no pending WebUI, or
-  // leave |web_ui_| as is if reusing it.
-  DCHECK(!(pending_web_ui_ && pending_and_current_web_ui_));
-  if (pending_web_ui_ || speculative_web_ui_) {
-    DCHECK(!should_reuse_web_ui_);
-    web_ui_.reset(browser_side_navigation ? speculative_web_ui_.release()
-                                          : pending_web_ui_.release());
-  } else if (pending_and_current_web_ui_ || should_reuse_web_ui_) {
-    if (browser_side_navigation) {
-      DCHECK(web_ui_);
-      should_reuse_web_ui_ = false;
-    } else {
-      DCHECK_EQ(pending_and_current_web_ui_.get(), web_ui_.get());
-      pending_and_current_web_ui_.reset();
-    }
-  } else {
-    web_ui_.reset();
-  }
-  DCHECK(!speculative_web_ui_);
-  DCHECK(!should_reuse_web_ui_);
-
-  // It's possible for the pending_render_frame_host_ to be nullptr when we
-  // aren't crossing process boundaries. If so, we just needed to handle the Web
-  // UI committing above and we're done.
-  if (!pending_render_frame_host_ && !speculative_render_frame_host_) {
+  // If the current RenderFrameHost has a pending Web UI then just commit it and
+  // return (there should be nothing else to commit).
+  if (render_frame_host_->pending_web_ui()) {
+    DCHECK(!pending_render_frame_host_ && !speculative_render_frame_host_);
+    render_frame_host_->CommitPendingWebUI();
     if (will_focus_location_bar)
       delegate_->SetFocusToLocationBar(false);
     return;
@@ -2114,7 +2030,8 @@ void RenderFrameHostManager::CommitPending() {
   // Swap in the pending or speculative frame and make it active. Also ensure
   // the FrameTree stays in sync.
   scoped_ptr<RenderFrameHostImpl> old_render_frame_host;
-  if (!browser_side_navigation) {
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
     DCHECK(!speculative_render_frame_host_);
     old_render_frame_host =
         SetRenderFrameHost(pending_render_frame_host_.Pass());
@@ -2276,9 +2193,6 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
       dest_url, source_instance, dest_instance, nullptr, transition,
       dest_is_restore, dest_is_view_source_mode);
 
-  const NavigationEntry* current_entry =
-      delegate_->GetLastCommittedNavigationEntryForRenderManager();
-
   DCHECK(!pending_render_frame_host_);
 
   if (new_instance.get() != current_instance) {
@@ -2291,14 +2205,8 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
 
     // New SiteInstance: create a pending RFH to navigate.
 
-    // This will possibly create (set to nullptr) a Web UI object for the
-    // pending page. We'll use this later to give the page special access. This
-    // must happen before the new renderer is created below so it will get
-    // bindings. It must also happen after the above conditional call to
-    // CancelPending(), otherwise CancelPending may clear the pending_web_ui_
-    // and the page will not have its bindings set appropriately.
-    SetPendingWebUI(dest_url, bindings);
-    CreatePendingRenderFrameHost(current_instance, new_instance.get());
+    CreatePendingRenderFrameHost(current_instance, new_instance.get(), dest_url,
+                                 bindings);
     if (!pending_render_frame_host_)
       return nullptr;
 
@@ -2358,23 +2266,7 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
   // delete the proxy.
   proxy_hosts_->Remove(new_instance.get()->GetId());
 
-  if (ShouldReuseWebUI(current_entry, dest_url)) {
-    pending_web_ui_.reset();
-    pending_and_current_web_ui_ = web_ui_->AsWeakPtr();
-  } else {
-    SetPendingWebUI(dest_url, bindings);
-    // Make sure the new RenderViewHost has the right bindings.
-    if (pending_web_ui() &&
-        !render_frame_host_->GetProcess()->IsForGuestsOnly()) {
-      render_frame_host_->render_view_host()->AllowBindings(
-          pending_web_ui()->GetBindings());
-    }
-  }
-
-  if (pending_web_ui() && render_frame_host_->IsRenderFrameLive()) {
-    pending_web_ui()->RenderViewReused(render_frame_host_->render_view_host(),
-                                       frame_tree_node_->IsMainFrame());
-  }
+  render_frame_host_->UpdatePendingWebUI(dest_url, bindings);
 
   // The renderer can exit view source mode when any error or cancellation
   // happen. We must overwrite to recover the mode.
@@ -2391,6 +2283,7 @@ void RenderFrameHostManager::CancelPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CancelPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
   DiscardUnusedFrame(UnsetPendingRenderFrameHost());
+  render_frame_host_->DiscardPendingWebUI();
 }
 
 scoped_ptr<RenderFrameHostImpl>
@@ -2405,9 +2298,6 @@ RenderFrameHostManager::UnsetPendingRenderFrameHost() {
   // We no longer need to prevent the process from exiting.
   pending_render_frame_host->GetProcess()->RemovePendingView();
 
-  pending_web_ui_.reset();
-  pending_and_current_web_ui_.reset();
-
   return pending_render_frame_host.Pass();
 }
 
@@ -2582,8 +2472,7 @@ void RenderFrameHostManager::CreateOpenerProxiesForFrameTree(
         frame_tree->root()->render_manager()->CreateRenderFrameProxy(instance);
       } else {
         frame_tree->root()->render_manager()->CreateRenderFrame(
-            instance, nullptr, CREATE_RF_SWAPPED_OUT | CREATE_RF_HIDDEN,
-            nullptr);
+            instance, CREATE_RF_SWAPPED_OUT | CREATE_RF_HIDDEN, nullptr);
       }
     }
   }