Create FrameNavigationEntries for manual subframe navigations.

content/browser/frame_host/navigation_entry_impl.cc

Index: content/browser/frame_host/navigation_entry_impl.cc
diff --git a/content/browser/frame_host/navigation_entry_impl.cc b/content/browser/frame_host/navigation_entry_impl.cc
index c2b7675519eedc753a6aaa720c64f54fb45f489f..bcfa77dd3bc55b4fc0c3360d45218cff5f087764 100644
--- a/content/browser/frame_host/navigation_entry_impl.cc
+++ b/content/browser/frame_host/navigation_entry_impl.cc
@@ -33,12 +33,36 @@ NavigationEntryImpl::TreeNode::TreeNode(FrameNavigationEntry* frame_entry)
 NavigationEntryImpl::TreeNode::~TreeNode() {
 }
 
-NavigationEntryImpl::TreeNode* NavigationEntryImpl::TreeNode::Clone() const {
+bool NavigationEntryImpl::TreeNode::MatchesFrame(
+    FrameTreeNode* frame_tree_node) const {
+  if (frame_tree_node->frame_tree_node_id() ==
+      frame_entry->frame_tree_node_id())
+    return true;
+
+  // For now, we set the root FNE's FrameTreeNode ID to -1.
+  return frame_tree_node->IsMainFrame() &&
+         frame_entry->frame_tree_node_id() == -1;
+}
+
+NavigationEntryImpl::TreeNode* NavigationEntryImpl::TreeNode::CloneAndReplace(

[Avi (use Gerrit), 2015/05/19 19:36:05]

Oh! If we're returning a new object, make the return value here a scoped_ptr so
that the ownership is clear from the function declaration.

[Charlie Reis, 2015/05/21 20:09:33]

Done.

+    FrameTreeNode* frame_tree_node,
+    FrameNavigationEntry* frame_navigation_entry) const {
+  if (frame_tree_node && MatchesFrame(frame_tree_node)) {
+    // Replace this node in the cloned tree and prune its children.
+    return new NavigationEntryImpl::TreeNode(frame_navigation_entry);
+  }
+
   // Clone the tree using a copy of the FrameNavigationEntry, without sharing.
+  // TODO(creis): Share FNEs unless it's for another tab.
   NavigationEntryImpl::TreeNode* copy =
       new NavigationEntryImpl::TreeNode(frame_entry->Clone());
 
-  // TODO(creis): Clone children once we add them.
+  // Recursively clone the children.
+  for (auto& child : children) {
+    copy->children.push_back(

[Avi (use Gerrit), 2015/05/19 19:36:05]

... and you can push_back a scoped_ptr onto a ScopedVector, so that works great.

[Charlie Reis, 2015/05/21 20:09:33]

Done.

+        child->CloneAndReplace(frame_tree_node, frame_navigation_entry));
+  }
+
   return copy;
 }
 
@@ -335,11 +359,18 @@ void NavigationEntryImpl::ClearExtraData(const std::string& key) {
 }
 
 NavigationEntryImpl* NavigationEntryImpl::Clone() const {
+  return NavigationEntryImpl::CloneAndReplace(nullptr, nullptr);
+}
+
+NavigationEntryImpl* NavigationEntryImpl::CloneAndReplace(
+    FrameTreeNode* frame_tree_node,
+    FrameNavigationEntry* frame_navigation_entry) const {
   NavigationEntryImpl* copy = new NavigationEntryImpl();
 
   // TODO(creis): Only share the same FrameNavigationEntries if cloning within
   // the same tab.
-  copy->frame_tree_.reset(frame_tree_->Clone());
+  copy->frame_tree_.reset(
+      frame_tree_->CloneAndReplace(frame_tree_node, frame_navigation_entry));
 
   // Copy all state over, unless cleared in ResetForCommit.
   copy->unique_id_ = unique_id_;
@@ -472,7 +503,6 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
   // We should already have a TreeNode for the parent node by the time this node
   // commits.  Find it first.
   DCHECK(frame_tree_node->parent());
-  int parent_ftn_id = frame_tree_node->parent()->frame_tree_node_id();
   bool found = false;
   NavigationEntryImpl::TreeNode* parent_node = nullptr;
   std::queue<NavigationEntryImpl::TreeNode*> work_queue;
@@ -480,11 +510,10 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
   while (!found && !work_queue.empty()) {
     parent_node = work_queue.front();
     work_queue.pop();
-    // The root FNE will have an ID of -1, so check for that as well.
-    if (parent_node->frame_entry->frame_tree_node_id() == parent_ftn_id ||
-        (parent_node->frame_entry->frame_tree_node_id() == -1 &&
-         parent_node == root_node() &&
-         frame_tree_node->parent()->IsMainFrame())) {
+    if (parent_node->MatchesFrame(frame_tree_node->parent())) {
+      // Only the root TreeNode should have a FTN ID of -1.
+      DCHECK_IMPLIES(parent_node->frame_entry->frame_tree_node_id() == -1,
+                     parent_node == root_node());
       found = true;
       break;
     }
@@ -494,9 +523,8 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
   }
   if (!found) {
     // The renderer should not send a commit for a subframe before its parent.
-    // TODO(creis): This can currently happen because we don't yet clone the
-    // FrameNavigationEntry tree on manual subframe navigations.  Once that's
-    // added, we should kill the renderer if we get here.
+    // TODO(creis): Kill the renderer if we get here.
+    NOTREACHED() << "Shouldn't see a commit for a subframe before parent.";
     return;
   }
 
@@ -504,7 +532,7 @@ void NavigationEntryImpl::AddOrUpdateFrameEntry(FrameTreeNode* frame_tree_node,
   int frame_tree_node_id = frame_tree_node->frame_tree_node_id();
   for (TreeNode* child : parent_node->children) {
     if (child->frame_entry->frame_tree_node_id() == frame_tree_node_id) {
-      // Update the existing FrameNavigationEntry.
+      // Update the existing FrameNavigationEntry (e.g., for replaceState).
       child->frame_entry->UpdateEntry(site_instance, url, referrer);
       return;
     }