Move WebUI ownership from the RenderFrameHostManager to the RenderFrameHost.

content/browser/frame_host/render_frame_host_impl.cc

Index: content/browser/frame_host/render_frame_host_impl.cc
diff --git a/content/browser/frame_host/render_frame_host_impl.cc b/content/browser/frame_host/render_frame_host_impl.cc
index 8f6c99c269f96de55707f4f34fffa855cced9901..c603f3325f5c4f7e9352bd08f56aca4984f9e297 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -41,6 +41,7 @@
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_impl.h"
 #include "content/browser/renderer_host/render_widget_host_view_base.h"
+#include "content/browser/webui/web_ui_controller_factory_registry.h"
 #include "content/common/accessibility_messages.h"
 #include "content/common/frame_messages.h"
 #include "content/common/input_messages.h"
@@ -61,6 +62,7 @@
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/stream_handle.h"
 #include "content/public/browser/user_metrics.h"
+#include "content/public/browser/web_ui_controller.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/isolated_world_ids.h"
@@ -191,6 +193,8 @@ RenderFrameHostImpl::RenderFrameHostImpl(SiteInstance* site_instance,
       accessibility_reset_token_(0),
       accessibility_reset_count_(0),
       no_create_browser_accessibility_manager_for_testing_(false),
+      web_ui_type_(WebUI::kNoWebUI),
+      should_reuse_web_ui_(false),
       weak_ptr_factory_(this) {
   bool is_swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
   bool hidden = !!(flags & CREATE_RF_HIDDEN);
@@ -221,6 +225,11 @@ RenderFrameHostImpl::RenderFrameHostImpl(SiteInstance* site_instance,
 }
 
 RenderFrameHostImpl::~RenderFrameHostImpl() {
+  // Release the WebUI before all else as the WebUI accesses the RenderFrameHost
+  // during cleanup.
+  web_ui_.reset();
+  pending_web_ui_.reset();
+
   GetProcess()->RemoveRoute(routing_id_);
   g_routing_id_frame_map.Get().erase(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_));
@@ -1018,6 +1027,9 @@ void RenderFrameHostImpl::SwapOut(
   swapout_event_monitor_timeout_->Start(
       base::TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS));
 
+  web_ui_.reset();
+  web_ui_type_ = WebUI::kNoWebUI;

[nasko, 2015/09/18 18:40:19]

Are we sure this is safe to do? What if the renderer needs to send IPCs up to
the WebUI component as part of cleanup?

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

In the cases where SwapOut is called -- when committing a navigation and when
discarding the pending/speculative RFH -- the associated WebUI is currently
destroyed before this call happens. So this seems to be safe.

+
   // There may be no proxy if there are no active views in the process.
   int proxy_routing_id = MSG_ROUTING_NONE;
   FrameReplicationState replication_state;
@@ -1950,6 +1962,57 @@ bool RenderFrameHostImpl::IsFocused() {
           frame_tree_->GetFocusedFrame()->IsDescendantOf(frame_tree_node()));
 }
 
+void RenderFrameHostImpl::InitializeWebUI(const GURL& dest_url,
+                                          int past_bindings) {
+  CHECK(!web_ui_ && !pending_web_ui_);
+  web_ui_ = CreateWebUI(dest_url, past_bindings, &web_ui_type_);
+  if (web_ui_)
+    web_ui_->GetController()->RenderViewCreated(render_view_host_);
+}
+
+void RenderFrameHostImpl::UpdatePendingWebUI(const GURL& dest_url,
+                                             int past_bindings) {
+  // TODO(carlosk): CHECK-enforce that the characteristic of this RFH having or
+  // not-having a WebUI remains unchanged with these calls.
+  WebUI::TypeID new_web_ui_type =
+      WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
+          GetSiteInstance()->GetBrowserContext(), dest_url);
+  should_reuse_web_ui_ =

[nasko, 2015/09/18 18:40:19]

Some textual comment explaining when exactly we should reuse the WebUI will be
very useful. One can parse the code that follows, but having explicit verbal
intent stated prior can help ensure correctness.

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

Done.

+      new_web_ui_type != WebUI::kNoWebUI && web_ui_type_ == new_web_ui_type;
+  DCHECK_IMPLIES(should_reuse_web_ui_, web_ui_);
+
+  if (should_reuse_web_ui_) {
+    // Resets the pending WebUI in case the current one will be reused.
+    pending_web_ui_.reset();
+    pending_web_ui_type_ = WebUI::kNoWebUI;
+  } else if (pending_web_ui_type_ != new_web_ui_type) {
+    // Otherwise create or simply reset the pending one if its type doesn't
+    // match the new type (if new type is kNoWebUI then it will be reset).
+    pending_web_ui_ =
+        CreateWebUI(dest_url, past_bindings, &pending_web_ui_type_);
+  }
+
+  CHECK_IMPLIES(pending_web_ui_, pending_web_ui_type_ != WebUI::kNoWebUI);
+  CHECK_IMPLIES(!pending_web_ui_, pending_web_ui_type_ == WebUI::kNoWebUI);
+}
+
+void RenderFrameHostImpl::CommitPendingWebUI() {
+  if (should_reuse_web_ui_) {
+    CHECK(!pending_web_ui_ && pending_web_ui_type_ == WebUI::kNoWebUI);
+    should_reuse_web_ui_ = false;
+  } else {
+    web_ui_ = pending_web_ui_.Pass();
+    web_ui_type_ = pending_web_ui_type_;
+    pending_web_ui_type_ = WebUI::kNoWebUI;
+  }
+}
+
+void RenderFrameHostImpl::DiscardPendingWebUI() {
+  pending_web_ui_.reset();
+  pending_web_ui_type_ = WebUI::kNoWebUI;
+  should_reuse_web_ui_ = false;
+}
+
 const image_downloader::ImageDownloaderPtr&
 RenderFrameHostImpl::GetMojoImageDownloader() {
   if (!mojo_image_downloader_.get() && GetServiceRegistry()) {
@@ -2248,4 +2311,46 @@ void RenderFrameHostImpl::AXContentNodeDataToAXNodeData(
   }
 }
 
+scoped_ptr<WebUIImpl> RenderFrameHostImpl::CreateWebUI(
+    const GURL& dest_url,
+    int past_bindings,
+    WebUI::TypeID* web_ui_type) {
+  scoped_ptr<WebUIImpl> web_ui;
+  *web_ui_type = WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
+      GetSiteInstance()->GetBrowserContext(), dest_url);
+  if (*web_ui_type != WebUI::kNoWebUI) {
+    web_ui = delegate_->CreateWebUIForRenderManager(dest_url);
+    // If we have assigned (zero or more) bindings to the 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 (web_ui && past_bindings != NavigationEntryImpl::kInvalidBindings &&
+        web_ui->GetBindings() != past_bindings) {
+      RecordAction(base::UserMetricsAction("ProcessSwapBindingsMismatch_RVHM"));
+      web_ui.reset();
+      *web_ui_type = WebUI::kNoWebUI;
+    }
+  }
+
+  // Check RenderViewHost for proper bindings.
+  if (web_ui && !render_view_host_->GetProcess()->IsForGuestsOnly()) {
+    // If a WebUI was created for the URL and the RenderView is not in a guest
+    // process, then enable missing bindings with the RenderViewHost.
+    int new_bindings = web_ui->GetBindings();
+    if ((render_view_host_->GetEnabledBindings() & new_bindings) !=
+        new_bindings) {
+      render_view_host_->AllowBindings(new_bindings);
+    }
+  } 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()));
+    }
+  }
+
+  return web_ui.Pass();
+}
+
 }  // namespace content