Move WebUI ownership from the RenderFrameHostManager to the RenderFrameHost.

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 13 matching lines @@
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/gpu/gpu_surface_tracker.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #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/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"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/render_process_host_observer.h"
 #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"
@@ skipping 169 matching lines @@
     RenderViewHostDelegate* render_view_delegate,
     RenderWidgetHostDelegate* render_widget_delegate,
     Delegate* delegate)
     : frame_tree_node_(frame_tree_node),
       delegate_(delegate),
       render_frame_delegate_(render_frame_delegate),
       render_view_delegate_(render_view_delegate),
       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_);
 }
 
 RenderFrameHostManager::~RenderFrameHostManager() {
   if (pending_render_frame_host_) {
     scoped_ptr<RenderFrameHostImpl> relic = UnsetPendingRenderFrameHost();
     ShutdownProxiesIfLastActiveFrameInSiteInstance(relic.get());
   }
 
   if (speculative_render_frame_host_) {
     scoped_ptr<RenderFrameHostImpl> relic = UnsetSpeculativeRenderFrameHost();
     ShutdownProxiesIfLastActiveFrameInSiteInstance(relic.get());
   }
 
   ShutdownProxiesIfLastActiveFrameInSiteInstance(render_frame_host_.get());
 
   // Delete any RenderFrameProxyHosts and swapped out RenderFrameHosts.
   // It is important to delete those prior to deleting the current
   // RenderFrameHost, since the CrossProcessFrameConnector (owned by
   // RenderFrameProxyHost) points to the RenderWidgetHostView associated with
   // 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>());
 }
 
 void RenderFrameHostManager::Init(BrowserContext* browser_context,
                                   SiteInstance* site_instance,
                                   int32 view_routing_id,
                                   int32 frame_routing_id,
                                   int32 widget_routing_id,
                                   int32 surface_id) {
@@ skipping 83 matching lines @@
 
 void RenderFrameHostManager::RemoveOuterDelegateFrame() {
   FrameTreeNode* outer_delegate_frame_tree_node =
       FrameTreeNode::GloballyFindByID(
           delegate_->GetOuterDelegateFrameTreeNodeID());
   DCHECK(outer_delegate_frame_tree_node->parent());
   outer_delegate_frame_tree_node->frame_tree()->RemoveFrame(
       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,
     const NavigationEntryImpl& entry) {
   TRACE_EVENT1("navigation", "RenderFrameHostManager:Navigate",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
   // Create a pending RenderFrameHost to use for the navigation.
   RenderFrameHostImpl* dest_render_frame_host = UpdateStateForNavigate(
       dest_url,
       // TODO(creis): Move source_site_instance to FNE.
@@ skipping 278 matching lines @@
 
   // Make sure any dynamic changes to this frame's sandbox flags that were made
   // prior to navigation take effect.
   CommitPendingSandboxFlags();
 }
 
 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;
   }
 
   if (render_frame_host == pending_render_frame_host_ ||
       render_frame_host == speculative_render_frame_host_) {
     // The pending cross-process navigation completed, so show the renderer.
     CommitPending();
   } else if (render_frame_host == render_frame_host_) {
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
@@ skipping 307 matching lines @@
   if (current_site_instance == dest_site_instance.get() ||
       (!request.browser_initiated() && is_main_frame) ||
       (!is_main_frame && !dest_site_instance->RequiresDedicatedProcess() &&
        !current_site_instance->RequiresDedicatedProcess())) {
     // Reuse the current RFH if its SiteInstance matches the the navigation's
     // or if this is a subframe navigation. We only swap RFHs for subframes when
     // --site-per-process is enabled.
     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_web_ui() == render_frame_host_->pending_web_ui());

[clamy, 2015/09/17 17:04:37]

This should be DCHECK(!speculative_render_frame_host_) and then use
render_frame_host_->pending_web_ui() instead of speculative_web_ui() to make it
easier to understand what's happening there.

[carlosk, 2015/09/30 19:37:27]

I added the DCHECK you suggested.

But what I am testing here is specifically that speculative_web_ui() returns the
correct value so it must stay.

+    if (speculative_web_ui() && render_frame_host_->IsRenderFrameLive()) {
+      speculative_web_ui()->GetController()->RenderViewReused(
+          render_frame_host_->render_view_host());
+    }
+
+    // If a speculative WebUI was set, 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());
     }
   } else {
     // If the SiteInstance for the final URL doesn't match the one from the
     // speculatively created RenderFrameHost, create a new RenderFrameHost using
     // this new SiteInstance.
     if (!speculative_render_frame_host_ ||
         speculative_render_frame_host_->GetSiteInstance() !=
             dest_site_instance.get()) {
       CleanUpNavigation();
       bool success = CreateSpeculativeRenderFrameHost(
@@ skipping 39 matching lines @@
     }
   }
 
   return navigation_rfh;
 }
 
 // PlzNavigate
 void RenderFrameHostManager::CleanUpNavigation() {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
-  speculative_web_ui_.reset();
-  should_reuse_web_ui_ = false;
+  render_frame_host_->DiscardPendingWebUI();

[clamy, 2015/09/17 17:04:38]

What if you committed a created WebUI at the start of the navigation?

[carlosk, 2015/09/30 19:37:27]

I assume you are referring to the early committing of the pending RFH when the
current renderer is not live. If that's the case there would not be a pending
WebUI as for cross-site navigations there's never a pending WebUI set on any RFH
(a new RFH always come with a new *active* WebUI when one is needed). 

And this follows the same behavior of today: we don't "revert" early committed
RFH/WebUI.

   if (speculative_render_frame_host_)
     DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());
 }
 
 // PlzNavigate
 scoped_ptr<RenderFrameHostImpl>
 RenderFrameHostManager::UnsetSpeculativeRenderFrameHost() {
   CHECK(base::CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableBrowserSideNavigation));
   speculative_render_frame_host_->GetProcess()->RemovePendingView();
@@ skipping 163 matching lines @@
   // We can't switch a RenderView between view source and non-view source mode
   // without screwing up the session history sometimes (when navigating between
   // "view-source:http://foo.com/" and "http://foo.com/", Blink doesn't treat
   // it as a new navigation). So require a BrowsingInstance switch.
   if (current_is_view_source_mode != new_is_view_source_mode)
     return true;
 
   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,
     SiteInstance* dest_instance,
     SiteInstance* candidate_instance,
     ui::PageTransition transition,
     bool dest_is_restore,
     bool dest_is_view_source_mode) {
   SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
 
@@ skipping 263 matching lines @@
   // to open a new tab to an interstitial-inducing URL, and then navigates
   // the page to a different same-site URL.  (This seems very unlikely in
   // practice.)
   if (current_entry)
     return current_entry->GetURL();
   return current_instance->GetSiteURL();
 }
 
 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;
 
   if (pending_render_frame_host_)
     CancelPending();
 
   // 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
   // process or the existing process crashed) have been initialized. Calling
   // Init multiple times will be ignored, so this is safe.
   if (!new_instance->GetProcess()->Init())
     return;
 
   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(
     SiteInstance* old_instance,
     SiteInstance* new_instance) {
   // Only create opener proxies if they are in the same BrowsingInstance.
   if (new_instance->IsRelatedSiteInstance(old_instance)) {
     CreateOpenerProxies(new_instance, frame_tree_node_);
   } else if (SiteIsolationPolicy::AreCrossProcessFramesPossible()) {
     // Ensure that the frame tree has RenderFrameProxyHosts for the
@@ skipping 68 matching lines @@
 }
 
 // PlzNavigate
 bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
     const GURL& url,
     SiteInstance* old_instance,
     SiteInstance* new_instance,
     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
   // process or the existing process crashed) have been initialized. Calling
   // Init multiple times will be ignored, so this is safe.
   if (!new_instance->GetProcess()->Init())
     return false;
 
   CreateProxiesForNewRenderFrameHost(old_instance, new_instance);
 
   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,
+                                                  int flags,
+                                                  int* view_routing_id_ptr) {
   bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
   bool swapped_out_forbidden =
       SiteIsolationPolicy::IsSwappedOutStateForbidden();
 
   CHECK(instance);
   CHECK_IMPLIES(swapped_out_forbidden, !swapped_out);
   CHECK_IMPLIES(!SiteIsolationPolicy::AreCrossProcessFramesPossible(),
                 frame_tree_node_->IsMainFrame());
 
   // Swapped out views should always be hidden.
@@ skipping 14 matching lines @@
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(instance);
   if (proxy && proxy->render_frame_host()) {
     RenderViewHost* render_view_host = proxy->GetRenderViewHost();
     CHECK(!swapped_out_forbidden);
     if (view_routing_id_ptr)
       *view_routing_id_ptr = proxy->GetRenderViewHost()->GetRoutingID();
     // Delete the existing RenderFrameProxyHost, but reuse the RenderFrameHost.
     // 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->InitializeWebUI(url, bindings);
       new_render_frame_host->GetProcess()->AddPendingView();
 
       proxy_hosts_->Remove(instance->GetId());
       // NB |proxy| is deleted at this point.
     }
     // 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.
     if (!render_view_host->GetView() && frame_tree_node_->IsMainFrame())
       delegate_->CreateRenderWidgetHostViewForRenderManager(render_view_host);
   } else {
     // Create a new RenderFrameHost if we don't find an existing one.
 
     int32 widget_routing_id = MSG_ROUTING_NONE;
     int32 surface_id = 0;
     // A RenderFrame in a different process from its parent RenderFrame
     // requires a RenderWidget for input/layout/painting.
     if (frame_tree_node_->parent() &&
         frame_tree_node_->parent()->current_frame_host()->GetSiteInstance() !=
             instance) {
       CHECK(SiteIsolationPolicy::AreCrossProcessFramesPossible());
       widget_routing_id = instance->GetProcess()->GetNextRoutingID();
       surface_id = GpuSurfaceTracker::Get()->AddSurfaceForRenderer(
           instance->GetProcess()->GetID(), widget_routing_id);
     }
 
     new_render_frame_host =
         CreateRenderFrameHost(instance, MSG_ROUTING_NONE, MSG_ROUTING_NONE,
                               widget_routing_id, surface_id, flags);
+    new_render_frame_host->InitializeWebUI(url, bindings);

[clamy, 2015/09/17 17:04:37]

Why do we need to do that instead of using a RFH creation method that takes
those as parameters?

[carlosk, 2015/09/30 19:37:27]

I did this mainly to avoid having to pump down (even deeper) the URL and
past_bindings needed to create a WebUI into the RFH creation call chain creating
even more changes in RFHM and adding other files. It would also increase the
already large number of parameters in the RFH constructor.

But as this method must always be called along with the RFH constructor I agree
it would be better that way. I will make this change for the following patch set
and see how it goes.

     RenderViewHostImpl* render_view_host =
         new_render_frame_host->render_view_host();
     int proxy_routing_id = MSG_ROUTING_NONE;
 
     // Prevent the process from exiting while we're trying to navigate in it.
     // Otherwise, if the new RFH is swapped out already, store it.
     if (!swapped_out) {
       new_render_frame_host->GetProcess()->AddPendingView();
     } else {
       proxy = new RenderFrameProxyHost(
@@ skipping 42 matching lines @@
       if (view_routing_id_ptr)
         *view_routing_id_ptr = render_view_host->GetRoutingID();
     }
   }
 
   // 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();
+  // DCHECK_IMPLIES(new_render_frame_host->web_ui(),
+  //                new_render_frame_host->web_ui()->GetBindings() == bindings);
+  if (new_render_frame_host && new_render_frame_host->web_ui() &&
+      !new_render_frame_host->GetProcess()->IsForGuestsOnly()) {
     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);
+    if ((render_view_host->GetEnabledBindings() & bindings) != bindings) {
+      render_view_host->AllowBindings(bindings);
     }
   }
 
   // Returns the new RFH if it isn't swapped out.
   if (success && !swapped_out) {
     DCHECK(new_render_frame_host->GetSiteInstance() == instance);
     return new_render_frame_host.Pass();
   }
   return nullptr;
 }
@@ skipping 109 matching lines @@
   // We may have initialized this RenderViewHost for another RenderFrameHost.
   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();
+    dest_web_ui = speculative_web_ui();
   } 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(
@@ skipping 79 matching lines @@
   RenderFrameProxyHost* proxy = GetRenderFrameProxyHost(site_instance);
   if (proxy)
     return proxy->GetRoutingID();
 
   return MSG_ROUTING_NONE;
 }
 
 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 commit it.
+  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;
   }
 
   // Remember if the page was focused so we can focus the new renderer in
   // that case.
   bool focus_render_view = !will_focus_location_bar &&
                            render_frame_host_->GetView() &&
                            render_frame_host_->GetView()->HasFocus();
 
   bool is_main_frame = frame_tree_node_->IsMainFrame();
 
   // 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());
   } else {
     // PlzNavigate
     DCHECK(speculative_render_frame_host_);
     old_render_frame_host =
         SetRenderFrameHost(speculative_render_frame_host_.Pass());
   }
 
@@ skipping 141 matching lines @@
   // If we are currently navigating cross-process, we want to get back to normal
   // and then navigate as usual.
   if (pending_render_frame_host_)
     CancelPending();
 
   SiteInstance* current_instance = render_frame_host_->GetSiteInstance();
   scoped_refptr<SiteInstance> new_instance = GetSiteInstanceForNavigation(
       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) {
     TRACE_EVENT_INSTANT2(
         "navigation",
         "RenderFrameHostManager::UpdateStateForNavigate:New SiteInstance",
         TRACE_EVENT_SCOPE_THREAD,
         "current_instance id", current_instance->GetId(),
         "new_instance id", new_instance->GetId());
 
     // 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;
 
     // Check if our current RFH is live before we set up a transition.
     if (!render_frame_host_->IsRenderFrameLive()) {
       // The current RFH is not live.  There's no reason to sit around with a
       // sad tab or a newly created RFH while we wait for the pending RFH to
       // navigate.  Just switch to the pending RFH now and go back to normal.
       // (Note that we don't care about on{before}unload handlers if the current
       // RFH isn't live.)
@@ skipping 39 matching lines @@
 
   // Otherwise the same SiteInstance can be used.  Navigate render_frame_host_.
 
   // It's possible to swap out the current RFH and then decide to navigate in it
   // anyway (e.g., a cross-process navigation that redirects back to the
   // original site).  In that case, we have a proxy for the current RFH but
   // haven't deleted it yet.  The new navigation will swap it back in, so we can
   // 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());
-    }
+  render_frame_host_->UpdatePendingWebUI(dest_url, bindings);
+
+  // TODO(carlosk): try moving this into RenderFrameHostImpl::CreateWebUI
+  // Make sure the new RenderViewHost has the right bindings.
+  if (render_frame_host_->pending_web_ui() &&
+      !render_frame_host_->GetProcess()->IsForGuestsOnly()) {
+    render_frame_host_->render_view_host()->AllowBindings(
+        render_frame_host_->pending_web_ui()->GetBindings());
   }
 
-  if (pending_web_ui() && render_frame_host_->IsRenderFrameLive()) {
-    pending_web_ui()->GetController()->RenderViewReused(
+  // TODO(carlosk): try moving this into RenderFrameHostImpl::UpdatePendingWebUI
+  if (render_frame_host_->pending_web_ui() &&
+      render_frame_host_->IsRenderFrameLive()) {
+    render_frame_host_->pending_web_ui()->GetController()->RenderViewReused(
         render_frame_host_->render_view_host());
   }
 
   // The renderer can exit view source mode when any error or cancellation
   // happen. We must overwrite to recover the mode.
   if (dest_is_view_source_mode) {
     render_frame_host_->render_view_host()->Send(
         new ViewMsg_EnableViewSourceMode(
             render_frame_host_->render_view_host()->GetRoutingID()));
   }
@@ skipping 12 matching lines @@
   scoped_ptr<RenderFrameHostImpl> pending_render_frame_host =
       pending_render_frame_host_.Pass();
 
   RenderFrameDevToolsAgentHost::OnCancelPendingNavigation(
       pending_render_frame_host.get(),
       render_frame_host_.get());
 
   // 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();
 }
 
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::SetRenderFrameHost(
     scoped_ptr<RenderFrameHostImpl> render_frame_host) {
   // Swap the two.
   scoped_ptr<RenderFrameHostImpl> old_render_frame_host =
       render_frame_host_.Pass();
   render_frame_host_ = render_frame_host.Pass();
 
@@ skipping 154 matching lines @@
     if (rvh && !rvh->IsRenderViewLive()) {
       EnsureRenderViewInitialized(rvh, instance);
     } else {
       // Create a swapped out RenderView in the given SiteInstance if none
       // exists. Since an opener can point to a subframe, do this on the root
       // frame of the current opener's frame tree.
       if (SiteIsolationPolicy::IsSwappedOutStateForbidden()) {
         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);
       }
     }
   }
 }
 
 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