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 1899fbee890a2ea4184ec1b8e50a16579b2c0171..7f7f4f5f9ee5f3fdc9b78bc2676eed83f2f8b637 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>());
 }
@@ -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,
@@ -421,6 +393,9 @@ RenderFrameHostImpl* RenderFrameHostManager::Navigate(
                         MSG_ROUTING_NONE))
       return nullptr;
 
+    // TODO(carlosk): figure out if WebUIImpl::RenderViewCreated should
+    // be invoked here for the *correct* WebUI instance.

[nasko, 2015/10/15 17:40:09]

Are this and the same TODO below supposed to be resolved in this CL?

[carlosk, 2015/10/19 17:21:01]

My intention was not that but I was wrong so thanks for this comment! :)
With my moving of the RenderViewCreated calls out of the code path started by
InitRenderView that *must* be done here! So I'm adding the proper calls.

Even so I think there is still an issue here with interwoven navigation requests
where we might notify the wrong WebUI instance. I will investigate a bit more
and file a bug on that if that' s really the case.

+
     // Now that we've created a new renderer, be sure to hide it if it isn't
     // our primary one.  Otherwise, we might crash if we try to call Show()
     // on it later.
@@ -670,13 +645,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 WebUI, commit it.
+    if (render_frame_host_->pending_web_ui())
       CommitPending();
     return;
   }
@@ -1025,21 +999,21 @@ 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_web_ui() == render_frame_host_->pending_web_ui());
+
+    // If a pending WebUI was set on the current RenderFrameHost (be it a new
+    // one or the reused current one) and the associated RenderFrame is alive,
+    // notify the WebUI the RenderView is being reused.
+    if (pending_web_ui() && render_frame_host_->IsRenderFrameLive()) {
+      pending_web_ui()->RenderViewReused(render_frame_host_->render_view_host(),
+                                         frame_tree_node_->IsMainFrame());
     }
+
+    DCHECK(!speculative_render_frame_host_);
   } else {
     // If the SiteInstance for the final URL doesn't match the one from the
     // speculatively created RenderFrameHost, create a new RenderFrameHost using
@@ -1052,8 +1026,20 @@ 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());
+    DCHECK(!speculative_render_frame_host_->pending_web_ui());
+    DCHECK(!render_frame_host_->pending_web_ui());
+
     navigation_rfh = speculative_render_frame_host_.get();
 
     // Check if our current RFH is live.
@@ -1075,9 +1061,11 @@ 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))
       return nullptr;
-    }
+
+    // TODO(carlosk): figure out if WebUIImpl::RenderViewCreated should
+    // be invoked here for the *correct* WebUI instance.
 
     if (navigation_rfh == render_frame_host_) {
       // TODO(nasko): This is a very ugly hack. The Chrome extensions process
@@ -1098,8 +1086,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();

[carlosk, 2015/10/15 16:34:08]

Calling this here is incorrect for PlzNavigate. This will be fixed in a follow
up change.

[nasko, 2015/10/15 17:40:09]

Acknowledged.

   if (speculative_render_frame_host_)
     DiscardUnusedFrame(UnsetSpeculativeRenderFrameHost());
 }
@@ -1283,18 +1270,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,
@@ -1608,7 +1583,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;
@@ -1626,8 +1603,10 @@ 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_ =
+      CreateRenderFrame(new_instance, create_render_frame_flags, nullptr);
+
+  InitializeWebUI(url, bindings, pending_render_frame_host_.get());

[nasko, 2015/10/15 17:40:09]

Now that I think a bit more about it, this call can be made to only take url and
bindings. Then moved out of this and the create speculative one into the
respective state updating methods - UpdateStateForNavigate and
GetFrameHostForNavigation. At both of those call sites we have the proper URL
and bindings parameters to pass and at the end of the day the result will be
identical.
In the future, when we have only PlzNavigate and move the WebUI ownership to
NavigationRequest, these methods won't need to be updated. Rather, we will just
call Create*RenderFrameHost, get the result and do
rfh.TakeWebUI(NavRequest.WebUI.Pass()).

[carlosk, 2015/10/19 17:21:01]

Done.

What I like most about this format is that, for PlzNavigate, WebUI updates are
*all* triggered from RFHM::GetFrameHostForNavigation (except for the final
commit, obviously). 

 }
 
 void RenderFrameHostManager::CreateProxiesForNewRenderFrameHost(
@@ -1715,12 +1694,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
@@ -1735,19 +1708,41 @@ bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
   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);
+      CreateRenderFrame(new_instance, create_render_frame_flags, nullptr);
+
+  InitializeWebUI(url, bindings, speculative_render_frame_host_.get());
 
-  if (!speculative_render_frame_host_) {
-    speculative_web_ui_.reset();
-    return false;
-  }
   return true;
 }
 
+void RenderFrameHostManager::InitializeWebUI(const GURL& url,
+                                             int bindings,
+                                             RenderFrameHostImpl* rfh) {

[nasko, 2015/10/15 17:40:09]

Can't we just pick pending vs speculative inside this method based on the
command line flag? Why do we need it as a parameter?

[carlosk, 2015/10/19 17:21:01]

Done.

+  rfh->UpdatePendingWebUI(url, bindings);
+  rfh->CommitPendingWebUI();
+  RenderViewHostImpl* rvh = rfh->render_view_host();
+
+  if (rfh->web_ui())
+    rfh->web_ui()->RenderViewCreated(rvh);
+
+  // 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 ((!rfh->web_ui() || rvh->GetProcess()->IsForGuestsOnly()) &&
+      rvh->is_active()) {
+    bool url_acceptable_for_webui =
+        WebUIControllerFactoryRegistry::GetInstance()->IsURLAcceptableForWebUI(
+            delegate_->GetControllerForRenderManager().GetBrowserContext(),
+            url);
+    if (!url_acceptable_for_webui) {
+      CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
+          rvh->GetProcess()->GetID()));
+    }
+  }
+}
+
 scoped_ptr<RenderFrameHostImpl> RenderFrameHostManager::CreateRenderFrame(
     SiteInstance* instance,
-    WebUIImpl* web_ui,
     int flags,
     int* view_routing_id_ptr) {
   bool swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
@@ -1866,24 +1861,10 @@ 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);
+    DCHECK(new_render_frame_host->render_view_host()->IsRenderViewLive());

[carlosk, 2015/10/15 16:34:08]

I added this check here as it seemed to be true by looking at the code. If tests
prove me right this will allow further simplifications down this code path where
renderer process were supposedly being initialized and should require
notifications to be sent to WebUI instances.

[nasko, 2015/10/15 17:40:09]

Acknowledged.

     return new_render_frame_host.Pass();
   }
   return nullptr;
@@ -2001,28 +1982,6 @@ 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()));
-    }
-  }
-
   int opener_frame_routing_id =
       GetOpenerRoutingID(render_view_host->GetSiteInstance());
 
@@ -2108,42 +2067,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 WebUI 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;
@@ -2160,7 +2094,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());
@@ -2322,9 +2257,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) {
@@ -2335,16 +2267,14 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
         "current_instance id", current_instance->GetId(),
         "new_instance id", new_instance->GetId());
 
+    // Clear any pending WebUI on the current RenderFrameHost as it won't
+    // be used.
+    render_frame_host_->DiscardPendingWebUI();
+
     // 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;
 
@@ -2360,8 +2290,11 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
     }
     // Otherwise, it's safe to treat this as a pending cross-process transition.
 
-    // We now have a pending RFH.
+    // We now have a pending RFH and possibly an associated pending WebUI.
     DCHECK(pending_render_frame_host_);
+    DCHECK_EQ(pending_web_ui(), pending_render_frame_host_->web_ui());
+    DCHECK(!pending_render_frame_host_->pending_web_ui());
+    DCHECK(!render_frame_host_->pending_web_ui());
 
     // We need to wait until the beforeunload handler has run, unless we are
     // transferring an existing request (in which case it has already run).
@@ -2404,19 +2337,12 @@ 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());
-    }
-  }
+  render_frame_host_->UpdatePendingWebUI(dest_url, bindings);
+  DCHECK(pending_web_ui() == render_frame_host_->pending_web_ui());
 
+  // If a pending WebUI was set on the current RenderFrameHost (be it a new one
+  // or the reused current one) and the associated RenderFrame is alive, notify
+  // the WebUI the RenderView is being reused.
   if (pending_web_ui() && render_frame_host_->IsRenderFrameLive()) {
     pending_web_ui()->RenderViewReused(render_frame_host_->render_view_host(),
                                        frame_tree_node_->IsMainFrame());
@@ -2436,6 +2362,7 @@ RenderFrameHostImpl* RenderFrameHostManager::UpdateStateForNavigate(
 void RenderFrameHostManager::CancelPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CancelPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
+  render_frame_host_->DiscardPendingWebUI();
   DiscardUnusedFrame(UnsetPendingRenderFrameHost());
 }
 
@@ -2451,9 +2378,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();
 }
 
@@ -2628,8 +2552,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);
       }
     }
   }