Set frame policy correctly in newly created renderer proxies

content/browser/frame_host/frame_tree.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/frame_tree.h"
 
 #include <stddef.h>
 
 #include <queue>
 #include <utility>
@@ skipping 169 matching lines @@
                          const FrameOwnerProperties& frame_owner_properties) {
   CHECK_NE(new_routing_id, MSG_ROUTING_NONE);
 
   // A child frame always starts with an initial empty document, which means
   // it is in the same SiteInstance as the parent frame. Ensure that the process
   // which requested a child frame to be added is the same as the process of the
   // parent node.
   if (parent->current_frame_host()->GetProcess()->GetID() != process_id)
     return false;
 
-  // AddChild is what creates the RenderFrameHost.
-  FrameTreeNode* added_node = parent->AddChild(
-      base::WrapUnique(new FrameTreeNode(
-          this, parent->navigator(), render_frame_delegate_,
-          render_widget_delegate_, manager_delegate_, parent, scope, frame_name,
-          frame_unique_name, frame_owner_properties)),
-      process_id, new_routing_id);
+  std::unique_ptr<FrameTreeNode> new_node = base::WrapUnique(new FrameTreeNode(
+      this, parent->navigator(), render_frame_delegate_,
+      render_widget_delegate_, manager_delegate_, parent, scope, frame_name,
+      frame_unique_name, frame_owner_properties));
 
   // The last committed NavigationEntry may have a FrameNavigationEntry with the
   // same |frame_unique_name|, since we don't remove FrameNavigationEntries if
   // their frames are deleted.  If there is a stale one, remove it to avoid
   // conflicts on future updates.
   NavigationEntryImpl* last_committed_entry = static_cast<NavigationEntryImpl*>(
       parent->navigator()->GetController()->GetLastCommittedEntry());
   if (last_committed_entry)
-    last_committed_entry->ClearStaleFrameEntriesForNewFrame(added_node);
+    last_committed_entry->ClearStaleFrameEntriesForNewFrame(new_node.get());

[alexmos, 2017/06/23 18:23:55]

Hmm, ClearStaleFrameEntriesForNewFrame calls InSameTreePosition, which walks up
the new node's ancestor chain.  I think that still works, since the FTN
constructor initializes the parent() pointer, but seems a bit fragile, since the
parent doesn't yet know about the new child at this point.  Can this block be
moved to be done after the AddChild, to minimize the logic that runs without the
new FTN fully in the tree?

[iclelland, 2017/06/24 03:57:44]

Done.

 
   // Set sandbox flags and container policy and make them effective immediately,
   // since initial sandbox flags and feature policy should apply to the initial
   // empty document in the frame.

[alexmos, 2017/06/23 18:23:55]

Can we update this comment to explain why those need to be done before the call
to AddChild?

[iclelland, 2017/06/24 03:57:44]

Done.

-  added_node->SetPendingSandboxFlags(sandbox_flags);
-  added_node->SetPendingContainerPolicy(container_policy);
-  added_node->CommitPendingFramePolicy();
+  new_node->SetPendingSandboxFlags(sandbox_flags);
+  new_node->SetPendingContainerPolicy(container_policy);
+  new_node->CommitPendingFramePolicy();
+
+  // Add the new node to the FrameTree, creating the RenderFrameHost.
+  FrameTreeNode* added_node =
+      parent->AddChild(std::move(new_node), process_id, new_routing_id);
 
   // Now that the new node is part of the FrameTree and has a RenderFrameHost,
   // we can announce the creation of the initial RenderFrame which already
   // exists in the renderer process.
   added_node->current_frame_host()->SetRenderFrameCreated(true);
   return true;
 }
 
 void FrameTree::RemoveFrame(FrameTreeNode* child) {
   FrameTreeNode* parent = child->parent();
@@ skipping 240 matching lines @@
   // This is only used to set page-level focus in cross-process subframes, and
   // requests to set focus in main frame's SiteInstance are ignored.
   if (instance != root_manager->current_frame_host()->GetSiteInstance()) {
     RenderFrameProxyHost* proxy =
         root_manager->GetRenderFrameProxyHost(instance);
     proxy->Send(new InputMsg_SetFocus(proxy->GetRoutingID(), is_focused));
   }
 }
 
 }  // namespace content