Move WebUI ownership from the RenderFrameHostManager to the RenderFrameHost.

content/browser/frame_host/render_frame_host_impl.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_impl.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/metrics/histogram.h"
 #include "base/process/kill.h"
 #include "base/time/time.h"
 #include "content/browser/accessibility/accessibility_mode_helper.h"
 #include "content/browser/accessibility/ax_tree_id_registry.h"
 #include "content/browser/accessibility/browser_accessibility_manager.h"
 #include "content/browser/accessibility/browser_accessibility_state_impl.h"
 #include "content/browser/bad_message.h"
 #include "content/browser/child_process_security_policy_impl.h"
+#include "content/browser/child_process_security_policy_impl.h"
 #include "content/browser/devtools/render_frame_devtools_agent_host.h"
 #include "content/browser/frame_host/cross_process_frame_connector.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
 #include "content/browser/frame_host/frame_mojo_shell.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/frame_tree_node.h"
 #include "content/browser/frame_host/navigation_handle_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/navigator_impl.h"
 #include "content/browser/frame_host/render_frame_host_delegate.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/frame_host/render_widget_host_view_child_frame.h"
 #include "content/browser/geolocation/geolocation_service_context.h"
 #include "content/browser/permissions/permission_service_context.h"
 #include "content/browser/permissions/permission_service_impl.h"
 #include "content/browser/presentation/presentation_service_impl.h"
 #include "content/browser/renderer_host/input/input_router.h"
 #include "content/browser/renderer_host/input/timeout_monitor.h"
 #include "content/browser/renderer_host/render_process_host_impl.h"
 #include "content/browser/renderer_host/render_view_host_delegate.h"
 #include "content/browser/renderer_host/render_view_host_delegate_view.h"
 #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"
 #include "content/common/inter_process_time_ticks_converter.h"
 #include "content/common/navigation_params.h"
 #include "content/common/render_frame_setup.mojom.h"
 #include "content/common/site_isolation_policy.h"
 #include "content/common/swapped_out_messages.h"
 #include "content/public/browser/ax_event_notification_details.h"
 #include "content/public/browser/browser_accessibility_state.h"
@@ skipping 131 matching lines @@
       routing_id_(routing_id),
       render_frame_created_(false),
       navigations_suspended_(false),
       is_waiting_for_beforeunload_ack_(false),
       unload_ack_is_for_navigation_(false),
       is_loading_(false),
       pending_commit_(false),
       accessibility_reset_token_(0),
       accessibility_reset_count_(0),
       no_create_browser_accessibility_manager_for_testing_(false),
+      web_ui_type_(WebUI::kNoWebUI),
       weak_ptr_factory_(this) {
   bool is_swapped_out = !!(flags & CREATE_RF_SWAPPED_OUT);
   bool hidden = !!(flags & CREATE_RF_HIDDEN);
   frame_tree_->AddRenderViewHostRef(render_view_host_);
   GetProcess()->AddRoute(routing_id_, this);
   g_routing_id_frame_map.Get().insert(std::make_pair(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_),
       this));
 
   if (is_swapped_out) {
@@ skipping 19 matching lines @@
       render_widget_host_ = new RenderWidgetHostImpl(rwh_delegate, GetProcess(),
                                                      widget_routing_id, hidden);
       render_widget_host_->set_owned_by_render_frame_host(true);
     } else {
       DCHECK(!render_widget_host_->owned_by_render_frame_host());
     }
   }
 }
 
 RenderFrameHostImpl::~RenderFrameHostImpl() {
+  // Release the WebUI before all else as the WebUI accesses the RenderFrameHost
+  // during cleanup.
+  ResetWebUI();
+
   GetProcess()->RemoveRoute(routing_id_);
   g_routing_id_frame_map.Get().erase(
       RenderFrameHostID(GetProcess()->GetID(), routing_id_));
 
   if (delegate_ && render_frame_created_)
     delegate_->RenderFrameDeleted(this);
 
   // If this was swapped out, it already decremented the active frame count of
   // the SiteInstance it belongs to.
   if (IsRFHStateActive(rfh_state_))
@@ skipping 782 matching lines @@
   // gone through this, therefore just return.
   if (rfh_state_ != RenderFrameHostImpl::STATE_DEFAULT) {
     NOTREACHED() << "RFH should be in default state when calling SwapOut.";
     return;
   }
 
   SetState(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT);
   swapout_event_monitor_timeout_->Start(
       base::TimeDelta::FromMilliseconds(RenderViewHostImpl::kUnloadTimeoutMS));
 
+  ResetWebUI();

[nasko, 2015/10/29 22:13:15]

This should probably be done when we receive the SwapOut ack. Otherwise the
WebUI object will be gone and if the page is calling any JS in its unload
handler that communicates with the WebUI, it will fail.

[carlosk, 2015/10/30 10:35:23]

See similar question and my answer here:
https://codereview.chromium.org/1352813006/diff/460001/content/browser/frame_...

[carlosk, 2015/11/03 09:50:30]

Done (as discussed in the link above).

+
   // There may be no proxy if there are no active views in the process.
   int proxy_routing_id = MSG_ROUTING_NONE;
   FrameReplicationState replication_state;
   if (proxy) {
     set_render_frame_proxy_host(proxy);
     proxy_routing_id = proxy->GetRoutingID();
     replication_state = proxy->frame_tree_node()->current_replication_state();
   }
 
   if (IsRenderFrameLive()) {
@@ skipping 936 matching lines @@
 bool RenderFrameHostImpl::IsFocused() {
   // TODO(mlamouri,kenrb): call GetRenderWidgetHost() directly when it stops
   // returning nullptr in some cases. See https://crbug.com/455245.
   return RenderWidgetHostImpl::From(
             GetView()->GetRenderWidgetHost())->is_focused() &&
          frame_tree_->GetFocusedFrame() &&
          (frame_tree_->GetFocusedFrame() == frame_tree_node() ||
           frame_tree_->GetFocusedFrame()->IsDescendantOf(frame_tree_node()));
 }
 
+bool RenderFrameHostImpl::UpdateWebUI(const GURL& dest_url,
+                                      int entry_bindings) {
+  WebUI::TypeID new_web_ui_type = WebUI::kNoWebUI;
+  if (dest_url.is_valid()) {
+    new_web_ui_type =
+        WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
+            GetSiteInstance()->GetBrowserContext(), dest_url);
+  }
+
+  WebUIImpl* prev_web_ui_ptr = web_ui_.get();

[nasko, 2015/10/29 22:13:15]

nit: No need to add _ptr in the variable name.

[carlosk, 2015/10/30 10:35:23]

Done.

+  if (new_web_ui_type == WebUI::kNoWebUI) {
+    ResetWebUI();
+  } else {
+    // The current WebUI should be reused when dest_url requires a WebUI and its
+    // type matches the current. Otherwise replace it with a new one.
+    if (web_ui_type_ != new_web_ui_type) {
+      web_ui_ = delegate_->CreateWebUIForRenderFrameHost(dest_url);
+      DCHECK(web_ui_);
+      web_ui_type_ = new_web_ui_type;
+
+      // 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 (entry_bindings != NavigationEntryImpl::kInvalidBindings &&
+          web_ui_->GetBindings() != entry_bindings) {
+        RecordAction(
+            base::UserMetricsAction("ProcessSwapBindingsMismatch_RVHM"));
+        ResetWebUI();
+        return prev_web_ui_ptr;

[nasko, 2015/10/29 22:13:15]

The return type of this method is a boolean value, not a pointer.

[carlosk, 2015/10/30 10:35:23]

Fixed. Trybots also brought that one to my attention. 

+      }
+    }
+  }
+  DCHECK_EQ(!web_ui_, 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 if (render_view_host_->is_active()) {
+    // 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.
+    bool url_acceptable_for_webui =
+        WebUIControllerFactoryRegistry::GetInstance()->IsURLAcceptableForWebUI(
+            GetSiteInstance()->GetBrowserContext(), dest_url);
+    if (!url_acceptable_for_webui) {
+      CHECK(!ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
+          GetProcess()->GetID()));
+    }
+  }
+  return prev_web_ui_ptr != web_ui_.get();
+}
+
 const image_downloader::ImageDownloaderPtr&
 RenderFrameHostImpl::GetMojoImageDownloader() {
   if (!mojo_image_downloader_.get() && GetServiceRegistry()) {
     GetServiceRegistry()->ConnectToRemoteService(
         mojo::GetProxy(&mojo_image_downloader_));
   }
   return mojo_image_downloader_;
 }
 
 bool RenderFrameHostImpl::IsSameSiteInstance(
@@ skipping 289 matching lines @@
   // Copy the common fields.
   *dst = src;
 
   if (src.routing_id != -1)
     dst->tree_id = RoutingIDToAXTreeID(src.routing_id);
 
   if (src.parent_routing_id != -1)
     dst->parent_tree_id = RoutingIDToAXTreeID(src.parent_routing_id);
 }
 
+void RenderFrameHostImpl::ResetWebUI() {
+  web_ui_type_ = WebUI::kNoWebUI;
+  web_ui_.reset();
+}
+
 }  // namespace content