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 12 matching lines @@
 #include "content/browser/frame_host/navigation_request.h"
 #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/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/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"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/referrer.h"
 #include "content/public/common/url_constants.h"
 
 namespace content {
 
 namespace {
 
 // Helper function to add the FrameTree of the given node's opener to the list
@@ skipping 160 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) {
   // Create a RenderViewHost and RenderFrameHost, once we have an instance.  It
@@ 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 28 matching lines @@
     // site that is handled via Mojo, then Mojo WebUI code in //chrome will
     // add a service to this RFH's ServiceRegistry).
     dest_render_frame_host->SetUpMojoIfNeeded();
 
     // Recreate the opener chain.
     CreateOpenerProxies(dest_render_frame_host->GetSiteInstance(),
                         frame_tree_node_);
     if (!InitRenderView(dest_render_frame_host->render_view_host(), nullptr))
       return nullptr;
 
+    if (pending_web_ui()) {
+      pending_web_ui()->RenderViewCreated(
+          dest_render_frame_host->render_view_host());
+    }
+
     // 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.
     if (dest_render_frame_host != render_frame_host_) {
       if (dest_render_frame_host->GetView())
         dest_render_frame_host->GetView()->Hide();
     } else {
       // After a renderer crash we'd have marked the host as invisible, so we
       // need to set the visibility of the new View to the correct value here
       // after reload.
@@ skipping 230 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 WebUI, 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 328 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());
+
+    // 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()) {

[nasko, 2015/10/20 01:13:21]

Doesn't this depend on whether we had a different WebUI object before calling
UpdatePendingWebUI? If there wasn't one, shouldn't this call RenderViewCreated?

[carlosk, 2015/10/20 12:58:51]

That's exactly what I'm proposing as an error and trying to figure out with my
questions around the correct situations to call those two methods.

But again, this is not a regression:
- This is a fix because the current code was missing this snippet for
PlzNavigate
- This keeps the same rationale used for regular navigations [1]

[1]
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/fr...

+      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
     // this new SiteInstance.
     if (!speculative_render_frame_host_ ||
         speculative_render_frame_host_->GetSiteInstance() !=
             dest_site_instance.get()) {
       CleanUpNavigation();
-      bool success = CreateSpeculativeRenderFrameHost(
-          request.common_params().url, current_site_instance,
-          dest_site_instance.get(), request.bindings());
+      bool success = CreateSpeculativeRenderFrameHost(current_site_instance,
+                                                      dest_site_instance.get());
       DCHECK(success);
+
+      InitializeWebUI(request.common_params().url, request.bindings());
+    } else {
+      // When reusing an existing speculative RenderFrameHost its *active* WebUI
+      // must be updated to make sure it matches the current URL.

[nasko, 2015/10/20 01:13:22]

Why does it have to be committed? It isn't immediately obvious, so a comment
clarifying it will be useful to have. 

[carlosk, 2015/10/20 12:58:51]

Because as described in the design doc [1], the pending/speculative RFH should
only have an active WebUI (only the current RFH can carry a pending WebUI). This
is "by definition" to maintain sanity.

Updated this comment and the equivalent section in InitializeWebUI

[1]
https://docs.google.com/document/d/1jrge5ulZ1YtqSR3WkNXzrtxhd3JXcWm8_zWBhEjaJ...

+      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.
     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 navigation to
       // complete. Just switch to the speculative 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.)
       CommitPending();
     }
   }
   DCHECK(navigation_rfh &&
          (navigation_rfh == render_frame_host_.get() ||
           navigation_rfh == speculative_render_frame_host_.get()));
 
   // If the RenderFrame that needs to navigate is not live (its process was just
   // created or has crashed), initialize it.
   if (!navigation_rfh->IsRenderFrameLive()) {
     // Recreate the opener chain.
     CreateOpenerProxies(navigation_rfh->GetSiteInstance(), frame_tree_node_);
-    if (!InitRenderView(navigation_rfh->render_view_host(), nullptr)) {
+    if (!InitRenderView(navigation_rfh->render_view_host(), nullptr))
       return nullptr;
+
+    if (speculative_web_ui()) {
+      speculative_web_ui()->RenderViewCreated(
+          navigation_rfh->render_view_host());
     }
 
     if (navigation_rfh == render_frame_host_) {
       // TODO(nasko): This is a very ugly hack. The Chrome extensions process
       // manager still uses NotificationService and expects to see a
       // RenderViewHost changed notification after WebContents and
       // RenderFrameHostManager are completely initialized. This should be
       // removed once the process manager moves away from NotificationService.
       // See https://crbug.com/462682.
       delegate_->NotifyMainFrameSwappedFromRenderManager(
           nullptr, render_frame_host_->render_view_host());
     }
   }
 
   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();
   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 312 matching lines @@
   // 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_ =
+      CreateRenderFrame(new_instance, 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 61 matching lines @@
   }
 
   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);
 }
 
 // PlzNavigate
 bool RenderFrameHostManager::CreateSpeculativeRenderFrameHost(
-    const GURL& url,
     SiteInstance* old_instance,
-    SiteInstance* new_instance,
-    int bindings) {
+    SiteInstance* new_instance) {
   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);
+      CreateRenderFrame(new_instance, create_render_frame_flags, nullptr);
 
-  if (!speculative_render_frame_host_) {
-    speculative_web_ui_.reset();
-    return false;
+  return !!speculative_render_frame_host_;
+}
+
+void RenderFrameHostManager::InitializeWebUI(const GURL& url, int bindings) {
+  RenderFrameHostImpl* rfh;
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    rfh = speculative_render_frame_host_.get();
+  } else {
+    rfh = pending_render_frame_host_.get();
   }
-  return true;
+
+  CHECK(rfh);

[nasko, 2015/10/20 01:13:22]

DCHECK

[carlosk, 2015/10/20 12:58:51]

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);
   bool swapped_out_forbidden =
       SiteIsolationPolicy::IsSwappedOutStateForbidden();
 
   CHECK(instance);
   CHECK_IMPLIES(swapped_out_forbidden, !swapped_out);
   CHECK_IMPLIES(!SiteIsolationPolicy::AreCrossProcessFramesPossible(),
                 frame_tree_node_->IsMainFrame());
@@ skipping 96 matching lines @@
         success = InitRenderFrame(new_render_frame_host.get());
       }
     }
 
     if (success) {
       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();
-    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());
     return new_render_frame_host.Pass();
   }
   return nullptr;
 }
 
 int RenderFrameHostManager::CreateRenderFrameProxy(SiteInstance* instance) {
   // A RenderFrameProxyHost should never be created in the same SiteInstance as
   // the current RFH.
   CHECK(instance);
   CHECK_NE(instance, render_frame_host_->GetSiteInstance());
@@ skipping 95 matching lines @@
     RenderFrameProxyHost* proxy) {
   // Ensure the renderer process is initialized before creating the
   // RenderView.
   if (!render_view_host->GetProcess()->Init())
     return false;
 
   // We may have initialized this RenderViewHost for another RenderFrameHost.
   if (render_view_host->IsRenderViewLive())
     return true;
 
-  // If |render_view_host| is not for a proxy and the navigation is to a WebUI,
-  // and if the RenderView is not in a guest process, tell |render_view_host|
-  // about any bindings it will need enabled.
-  // TODO(carlosk): Move WebUI to RenderFrameHost in https://crbug.com/508850.
-  WebUIImpl* dest_web_ui = nullptr;
-  if (!proxy) {
-    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());
 
   bool created = delegate_->CreateRenderViewForRenderManager(
       render_view_host, opener_frame_routing_id,
       proxy ? proxy->GetRoutingID() : MSG_ROUTING_NONE,
       frame_tree_node_->current_replication_state());
 
   if (created && proxy)
     proxy->set_render_frame_proxy_created(true);
@@ skipping 71 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 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;
   }
 
   // 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 143 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());
 
+    // Clear any pending WebUI on the current RenderFrameHost as it won't
+    // be used.
+    render_frame_host_->DiscardPendingWebUI();

[nasko, 2015/10/20 01:13:22]

Can't this move either closer to InitializeWebUI or even be folded into it? At
least CreatePendingRenderFrameHost() should be doable before this code, as the
two shouldn't intersect in members they operate on. If moved just before
InitializeWebUI, it will allow us to have all WebUI code in one place.

[carlosk, 2015/10/20 12:58:51]

Done.

+
     // 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());
     if (!pending_render_frame_host_)
       return nullptr;
 
+    InitializeWebUI(dest_url, bindings);
+
     // 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.)
       CommitPending();
       return render_frame_host_.get();
     }
     // 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).
     // Suspend the new render view (i.e., don't let it send the cross-process
     // Navigate message) until we hear back from the old renderer's
     // beforeunload handler.  If the handler returns false, we'll have to
     // cancel the request.
     //
     DCHECK(!pending_render_frame_host_->are_navigations_suspended());
     bool is_transfer = transferred_request_id != GlobalRequestID();
@@ skipping 22 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);
+  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());
   }
 
   // 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()));
   }
 
   return render_frame_host_.get();
 }
 
 void RenderFrameHostManager::CancelPending() {
   TRACE_EVENT1("navigation", "RenderFrameHostManager::CancelPending",
                "FrameTreeNode id", frame_tree_node_->frame_tree_node_id());
+  render_frame_host_->DiscardPendingWebUI();
   DiscardUnusedFrame(UnsetPendingRenderFrameHost());
 }
 
 scoped_ptr<RenderFrameHostImpl>
 RenderFrameHostManager::UnsetPendingRenderFrameHost() {
   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