Change allowed bindings to be per RenderFrame instead of per RenderView.

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 <algorithm>
 #include <utility>
 
 #include "base/bind.h"
@@ skipping 64 matching lines @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/browser/permission_manager.h"
 #include "content/public/browser/permission_type.h"
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_widget_host_view.h"
 #include "content/public/browser/resource_context.h"
 #include "content/public/browser/storage_partition.h"
 #include "content/public/browser/stream_handle.h"
 #include "content/public/browser/user_metrics.h"
+#include "content/public/common/bindings_policy.h"
 #include "content/public/common/browser_side_navigation_policy.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_features.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/file_chooser_file_info.h"
 #include "content/public/common/file_chooser_params.h"
 #include "content/public/common/form_field_data.h"
 #include "content/public/common/isolated_world_ids.h"
 #include "content/public/common/service_manager_connection.h"
 #include "content/public/common/service_names.mojom.h"
@@ skipping 851 matching lines @@
     if (created) {
       SetUpMojoIfNeeded();
       delegate_->RenderFrameCreated(this);
     } else {
       delegate_->RenderFrameDeleted(this);
     }
   }
 
   if (created && render_widget_host_)
     render_widget_host_->InitForFrame();
+
+  if (enabled_bindings_) {

[Charlie Reis, 2017/01/18 22:18:43]

Should this also check |created|?  I'm guessing we don't want to call
AllowBindings if this is about a crash / RenderFrameDeleted case.

[Sam McNally, 2017/01/19 05:30:37]

Done.

+    if (!frame_bindings_control_)
+      GetRemoteAssociatedInterfaces()->GetInterface(&frame_bindings_control_);
+    frame_bindings_control_->AllowBindings(enabled_bindings_);
+  }
 }
 
 void RenderFrameHostImpl::Init() {
   ResourceDispatcherHost::ResumeBlockedRequestsForFrameFromUI(this);
   if (!waiting_for_init_)
     return;
 
   waiting_for_init_ = false;
   if (pendinging_navigate_) {
     frame_tree_node()->navigator()->OnBeginNavigation(
@@ skipping 45 matching lines @@
   // TODO(lukasza): Call ReceivedBadMessage when |frame_unique_name| is empty.
   DCHECK(!frame_unique_name.empty());
 
   // It is possible that while a new RenderFrameHost was committed, the
   // RenderFrame corresponding to this host sent an IPC message to create a
   // frame and it is delivered after this host is swapped out.
   // Ignore such messages, as we know this RenderFrameHost is going away.
   if (!is_active() || frame_tree_node_->current_frame_host() != this)
     return;
 
-  frame_tree_->AddFrame(frame_tree_node_, GetProcess()->GetID(), new_routing_id,
-                        scope, frame_name, frame_unique_name, sandbox_flags,
-                        frame_owner_properties);
+  bool added = frame_tree_->AddFrame(

[Charlie Reis, 2017/01/18 22:18:43]

Note that AddFrame already calls SetRenderFrameCreated(true) on the new child,
which will hit your AllowBindings call above.  I think it would be better to
avoid the need for the second AllowBindings call below.

Can we inherit the parent's bindings in the RenderFrameHostImpl constructor, on
line 358 instead?  Let's include a comment with it saying that all frames in a
page are expected to have the same bindings.

[Sam McNally, 2017/01/19 05:30:37]

Done.

[Charlie Reis, 2017/01/19 17:58:55]

Thanks!  That looks better.

+      frame_tree_node_, GetProcess()->GetID(), new_routing_id, scope,
+      frame_name, frame_unique_name, sandbox_flags, frame_owner_properties);
+
+  if (added && enabled_bindings_) {
+    frame_tree_->FindByRoutingID(GetProcess()->GetID(), new_routing_id)
+        ->current_frame_host()
+        ->AllowBindings(enabled_bindings_);
+  }
 }
 
 void RenderFrameHostImpl::OnCreateNewWindow(
     int32_t render_view_route_id,
     int32_t main_frame_route_id,
     int32_t main_frame_widget_route_id,
     const mojom::CreateNewWindowParams& params,
     SessionStorageNamespace* session_storage_namespace) {
   mojom::CreateNewWindowParamsPtr validated_params(params.Clone());
   GetProcess()->FilterURL(false, &validated_params->target_url);
@@ skipping 303 matching lines @@
 }
 
 RenderWidgetHostView* RenderFrameHostImpl::GetView() {
   return GetRenderWidgetHost()->GetView();
 }
 
 GlobalFrameRoutingId RenderFrameHostImpl::GetGlobalFrameRoutingId() {
   return GlobalFrameRoutingId(GetProcess()->GetID(), GetRoutingID());
 }
 
-int RenderFrameHostImpl::GetEnabledBindings() {
-  return render_view_host_->GetEnabledBindings();
-}
-
 void RenderFrameHostImpl::SetNavigationHandle(
     std::unique_ptr<NavigationHandleImpl> navigation_handle) {
   navigation_handle_ = std::move(navigation_handle);
 
   // TODO(clamy): Remove this debug code once we understand better how we get to
   // the point of attempting to transfer a navigation from a RFH that is no
   // longer active.
   if (navigation_handle_ && !is_active())
     base::debug::DumpWithoutCrashing();
 }
@@ skipping 362 matching lines @@
 }
 
 void RenderFrameHostImpl::RequestFocusedFormFieldData(
     FormFieldDataCallback& callback) {
   static int next_id = 1;
   int request_id = ++next_id;
   form_field_data_callbacks_[request_id] = callback;
   Send(new FrameMsg_FocusedFormFieldDataRequest(GetRoutingID(), request_id));
 }
 
+void RenderFrameHostImpl::AllowBindings(int bindings_flags) {
+  // Never grant any bindings to browser plugin guests.
+  if (GetProcess()->IsForGuestsOnly()) {
+    NOTREACHED() << "Never grant bindings to a guest process.";
+    return;
+  }
+
+  // Ensure we aren't granting WebUI bindings to a process that has already
+  // been used for non-privileged views.
+  if (bindings_flags & BINDINGS_POLICY_WEB_UI &&
+      GetProcess()->HasConnection() &&
+      !ChildProcessSecurityPolicyImpl::GetInstance()->HasWebUIBindings(
+          GetProcess()->GetID())) {
+    // This process has no bindings yet. Make sure it does not have more
+    // than this single active view.
+    // --single-process only has one renderer.
+    if (GetProcess()->GetActiveViewCount() > 1 &&
+        !base::CommandLine::ForCurrentProcess()->HasSwitch(
+            switches::kSingleProcess))
+      return;
+  }
+
+  if (bindings_flags & BINDINGS_POLICY_WEB_UI) {
+    ChildProcessSecurityPolicyImpl::GetInstance()->GrantWebUIBindings(
+        GetProcess()->GetID());
+  }
+
+  enabled_bindings_ |= bindings_flags;
+  if (GetParent())
+    DCHECK_EQ(GetParent()->GetEnabledBindings(), GetEnabledBindings());
+
+  if (render_frame_created_) {
+    if (!frame_bindings_control_)
+      GetRemoteAssociatedInterfaces()->GetInterface(&frame_bindings_control_);
+    frame_bindings_control_->AllowBindings(enabled_bindings_);
+  }
+}
+
+int RenderFrameHostImpl::GetEnabledBindings() const {
+  return enabled_bindings_;
+}
+
 void RenderFrameHostImpl::OnFocusedFormFieldDataResponse(
     int request_id,
     const FormFieldData& field_data) {
   auto it = form_field_data_callbacks_.find(request_id);
   if (it != form_field_data_callbacks_.end()) {
     it->second.Run(field_data);
     form_field_data_callbacks_.erase(it);
   }
 }
 
@@ skipping 970 matching lines @@
       BrowserContext::GetServiceManagerConnectionFor(
           GetProcess()->GetBrowserContext());
   // |service_manager_connection| may be null in tests using TestBrowserContext.
   if (service_manager_connection) {
     service_manager_connection->RemoveOnConnectHandler(on_connect_handler_id_);
     on_connect_handler_id_ = 0;
   }
 
   frame_.reset();
   frame_host_binding_.Close();
+  frame_bindings_control_.reset();
 
   // Disconnect with ImageDownloader Mojo service in RenderFrame.
   mojo_image_downloader_.reset();
 }
 
 bool RenderFrameHostImpl::IsFocused() {
   return GetRenderWidgetHost()->is_focused() &&
          frame_tree_->GetFocusedFrame() &&
          (frame_tree_->GetFocusedFrame() == frame_tree_node() ||
           frame_tree_->GetFocusedFrame()->IsDescendantOf(frame_tree_node()));
@@ skipping 38 matching lines @@
             base::UserMetricsAction("ProcessSwapBindingsMismatch_RVHM"));
         ClearPendingWebUI();
       }
     }
   }
   DCHECK_EQ(!pending_web_ui_, pending_web_ui_type_ == WebUI::kNoWebUI);
 
   // Either grant or check the RenderViewHost with/for proper bindings.
   if (pending_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.

[Charlie Reis, 2017/01/18 22:18:43]

nit: Drop "with the RenderViewHost."

[Sam McNally, 2017/01/19 05:30:37]

Done.

     int new_bindings = pending_web_ui_->GetBindings();
-    if ((render_view_host_->GetEnabledBindings() & new_bindings) !=
-        new_bindings) {
-      render_view_host_->AllowBindings(new_bindings);
+    if ((GetEnabledBindings() & new_bindings) != new_bindings) {
+      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(
@@ skipping 588 matching lines @@
   // There is no pending NavigationEntry in these cases, so pass 0 as the
   // pending_nav_entry_id. If the previous handle was a prematurely aborted
   // navigation loaded via LoadDataWithBaseURL, propagate the entry id.
   return NavigationHandleImpl::Create(
       params.url, frame_tree_node_, is_renderer_initiated,
       params.was_within_same_page, base::TimeTicks::Now(),
       entry_id_for_data_nav, false);  // started_from_context_menu
 }
 
 }  // namespace content