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"
@@ skipping 24 matching lines @@
 #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),
+      pending_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);
   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) {
     rfh_state_ = STATE_SWAPPED_OUT;
   } else {
     rfh_state_ = STATE_DEFAULT;
     GetSiteInstance()->increment_active_frame_count();
   }
 
   SetUpMojoIfNeeded();
   swapout_event_monitor_timeout_.reset(new TimeoutMonitor(base::Bind(
       &RenderFrameHostImpl::OnSwappedOut, weak_ptr_factory_.GetWeakPtr())));
 
   if (widget_routing_id != MSG_ROUTING_NONE) {
     render_widget_host_ = new RenderWidgetHostImpl(rwh_delegate, GetProcess(),
                                                    widget_routing_id, hidden);
     render_widget_host_->set_owned_by_render_frame_host(true);
   }
 }
 
 RenderFrameHostImpl::~RenderFrameHostImpl() {
+  // Release the WebUI before all else as the WebUI accesses the RenderFrameHost
+  // during cleanup.
+  web_ui_type_ = WebUI::kNoWebUI;
+  web_ui_.reset();
+  pending_web_ui_type_ = WebUI::kNoWebUI;
+  pending_web_ui_.reset();
+
   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 785 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));
 
+  web_ui_type_ = WebUI::kNoWebUI;

[Charlie Reis, 2015/10/23 06:39:25]

Sanity check: Why is this the right time to revoke WebUI?  The RenderFrameHost
will still be running its unload handler in the background.  Are there no cases
that it will try to use the WebUI object during unload?

[carlosk, 2015/10/27 14:35:44]

Comparing with the previous behavior, RFHI:SwapOut is called only by two methods
from RFHM (excluding tests) [1]:
- RFHM::SwapOutOldFrame: which itself is only called by RFHM::CommitPending for
the previously active RFH after the new one was put in place and any previous
WebUI replaced.
- RFHM::DiscardUnusedFrame which was always used to discard a speculative or
pending RFH so I don't think we need to care about the unload handler for them.

Given these two this location seemed good enough. WDYT?

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

[nasko, 2015/11/02 17:00:35]

I'm not sure how that makes much of a difference. The unload handler can be
considered complete only at OnSwapOut. Until then, unload handler could be
sending IPCs to the browser process. We might not have any of those today, but
I'd rather make this code work properly than rely on no such WebUI existing.

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

OK, I moved the destruction of the WebUI to when the swap-out ack is received.

To add to the work on making sure this makes a difference: *if* received IPCs
come through WebContentsImpl::OnMessageReceived then they will not reach a
non-active or non-pending RFH's WebUI given its implementation uses
WebContentsImpl::GetWebUI().

See
https://code.google.com/p/chromium/codesearch#chromium/src/content/browser/we...

+  web_ui_.reset();
+  pending_web_ui_type_ = WebUI::kNoWebUI;
+  pending_web_ui_.reset();

[Charlie Reis, 2015/10/23 06:39:25]

Can we make these 4 lines into a private helper (ResetWebUI), also used in
~RenderFrameHostImpl?

[carlosk, 2015/10/27 14:35:44]

Done.

+
   // 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 926 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()));
 }
 
+void RenderFrameHostImpl::UpdatePendingWebUI(const GURL& dest_url,
+                                             int entry_bindings) {
+  WebUI::TypeID new_web_ui_type =
+      WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
+          GetSiteInstance()->GetBrowserContext(), dest_url);
+
+  // The current WebUI should be reused when dest_url requires a WebUI and its
+  // type matches the current.
+  should_reuse_web_ui_ = false;
+  if (new_web_ui_type != WebUI::kNoWebUI && web_ui_type_ == new_web_ui_type) {
+    DCHECK(web_ui_);
+    should_reuse_web_ui_ = true;
+
+    // Reset the pending WebUI as 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, update or 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, entry_bindings, &pending_web_ui_type_);
+  }
+
+  DCHECK_EQ(!pending_web_ui_, pending_web_ui_type_ == WebUI::kNoWebUI);
+}
+
+void RenderFrameHostImpl::CommitPendingWebUI() {
+  if (should_reuse_web_ui_) {
+    DCHECK(!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()) {
     GetServiceRegistry()->ConnectToRemoteService(
         mojo::GetProxy(&mojo_image_downloader_));
   }
   return mojo_image_downloader_;
 }
 
 bool RenderFrameHostImpl::IsSameSiteInstance(
@@ skipping 284 matching lines @@
             ui::AX_ATTR_CHILD_TREE_ID,
             BrowserPluginInstanceIDToAXTreeID(value)));
         break;
       case AX_CONTENT_INT_ATTRIBUTE_LAST:
         NOTREACHED();
         break;
     }
   }
 }
 
+scoped_ptr<WebUIImpl> RenderFrameHostImpl::CreateWebUI(
+    const GURL& dest_url,
+    int entry_bindings,
+    WebUI::TypeID* web_ui_type) {
+  if (!dest_url.is_valid()) {
+    *web_ui_type = WebUI::kNoWebUI;
+    return nullptr;
+  }
+
+  scoped_ptr<WebUIImpl> web_ui;
+  *web_ui_type = WebUIControllerFactoryRegistry::GetInstance()->GetWebUIType(
+      GetSiteInstance()->GetBrowserContext(), dest_url);
+  if (*web_ui_type != WebUI::kNoWebUI) {
+    web_ui = delegate_->CreateWebUIForRenderFrameHost(dest_url);
+    DCHECK(web_ui);
+
+    // 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"));
+      *web_ui_type = WebUI::kNoWebUI;
+      return nullptr;
+    }
+  }
+
+  // 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);
+    }
+  }
+
+  CHECK_EQ(!web_ui, *web_ui_type == WebUI::kNoWebUI);
+  return web_ui.Pass();
+}
+
 }  // namespace content