Set frame policy correctly in newly created renderer proxies

content/browser/frame_host/frame_tree.cc

Index: content/browser/frame_host/frame_tree.cc
diff --git a/content/browser/frame_host/frame_tree.cc b/content/browser/frame_host/frame_tree.cc
index c22f24bb22d2525d984460aac9da25200f1db8ab..10240205af0bacb8ff89384d77769b78703cbdee 100644
--- a/content/browser/frame_host/frame_tree.cc
+++ b/content/browser/frame_host/frame_tree.cc
@@ -187,13 +187,10 @@ bool FrameTree::AddFrame(FrameTreeNode* parent,
   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
@@ -202,14 +199,18 @@ bool FrameTree::AddFrame(FrameTreeNode* parent,
   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