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

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 b95ef8cd1813dfa5d0afa24501567072255ab1ac..59cf37d0a81bc5620713bbf07b0e2409ad20d79f 100644
--- a/content/browser/frame_host/render_frame_host_impl.cc
+++ b/content/browser/frame_host/render_frame_host_impl.cc
@@ -82,6 +82,7 @@
 #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"
@@ -953,6 +954,12 @@ void RenderFrameHostImpl::SetRenderFrameCreated(bool created) {
 
   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() {
@@ -1018,9 +1025,15 @@ void RenderFrameHostImpl::OnCreateChildFrame(
   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(
@@ -1344,10 +1357,6 @@ 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);
@@ -1730,6 +1739,48 @@ void RenderFrameHostImpl::RequestFocusedFormFieldData(
   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) {
@@ -2720,6 +2771,7 @@ void RenderFrameHostImpl::InvalidateMojoConnection() {
 
   frame_.reset();
   frame_host_binding_.Close();
+  frame_bindings_control_.reset();
 
   // Disconnect with ImageDownloader Mojo service in RenderFrame.
   mojo_image_downloader_.reset();
@@ -2780,9 +2832,8 @@ bool RenderFrameHostImpl::UpdatePendingWebUI(const GURL& dest_url,
     // 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