Remove existing FrameNavigationEntry when new named frame is added.

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 c50a8d2229e498f4d6bb709429192ba6f0c4a057..28335ec4024d5c705f08341994d7a444bc7cab5b 100644
--- a/content/browser/frame_host/navigation_entry_impl.cc
+++ b/content/browser/frame_host/navigation_entry_impl.cc
@@ -99,6 +99,29 @@ void RecursivelyGenerateFrameState(
   }
 }
 
+// Walk the ancestor chain for both the |frame_tree_node| and the |node|.
+// Comparing the inputs directly is not performed, as this method assumes they
+// already match each other. Returns false if a mismatch in unique name or
+// ancestor chain is detected, otherwise true.
+bool InSameTreePosition(
+    FrameTreeNode* frame_tree_node,

[Charlie Reis, 2016/08/01 21:04:25]

Style nit: I think this will fit on the previous line, with the next line
indented accordingly.

[nasko, 2016/08/01 23:05:19]

I wonder why git cl format didn't pick it up initially. Fixed.

+    NavigationEntryImpl::TreeNode* node) {
+  FrameTreeNode* ftn = frame_tree_node->parent();
+  NavigationEntryImpl::TreeNode* current_node = node->parent;
+  while (ftn && current_node) {
+    if (!current_node->MatchesFrame(ftn))
+      return false;
+
+    if (!current_node->parent)

[Charlie Reis, 2016/08/01 21:04:25]

Is this a bug?  Seems like if you call this for a first-level subframe, we'll
compare the main frame's FTN and TreeNode in the first iteration of the loop. 
They'll match on line 112, but this will make us return false.

Seems like we only want to return false if one has a parent and the other
doesn't.

(If that's right, can you make sure we have test coverage for it?)

[nasko, 2016/08/01 23:05:19]

Done.

+      return false;
+
+    ftn = ftn->parent();
+    current_node = current_node->parent;
+  }
+
+  return true;
+}
+
 }  // namespace
 
 int NavigationEntryImpl::kInvalidBindings = -1;
@@ -810,6 +833,43 @@ void NavigationEntryImpl::ClearChildren(FrameTreeNode* frame_tree_node) {
     tree_node->children.clear();
 }
 
+void NavigationEntryImpl::ClearStaleFrameEntriesForNewFrame(
+    FrameTreeNode* frame_tree_node) {
+  DCHECK(!frame_tree_node->IsMainFrame());
+
+  NavigationEntryImpl::TreeNode* node = nullptr;
+  std::queue<NavigationEntryImpl::TreeNode*> work_queue;
+  int count = 0;
+
+  work_queue.push(root_node());
+  while (!work_queue.empty()) {
+    node = work_queue.front();
+    work_queue.pop();
+
+    // Enqueue any children and keep looking if the current node doesn't match.
+    if (!node->MatchesFrame(frame_tree_node)) {
+      for (auto* child : node->children) {
+        work_queue.push(child);
+      }
+      continue;
+    }
+
+    // Remove the node from the tree if it is not for in the same position in

[Charlie Reis, 2016/08/01 21:04:25]

nit: Drop "for"

[nasko, 2016/08/01 23:05:19]

Done.

+    // the tree of FrameNavigationEntries and the FrameTree.
+    if (!InSameTreePosition(frame_tree_node, node)) {
+      NavigationEntryImpl::TreeNode* parent_node = node->parent;
+      auto it = std::find(parent_node->children.begin(),
+                          parent_node->children.end(), node);
+      DCHECK(it != parent_node->children.end());

[Charlie Reis, 2016/08/01 21:04:25]

Let's upgrade this to a CHECK.  (Otherwise, could it be a memory error below?)

[nasko, 2016/08/01 23:05:19]

Done.

+      parent_node->children.erase(it);
+    }
+    ++count;
+  }
+
+  // At most one match is expected, since it is based on unique frame name.
+  DCHECK_LE(count, 1);
+}
+
 void NavigationEntryImpl::SetScreenshotPNGData(
     scoped_refptr<base::RefCountedBytes> png_data) {
   screenshot_ = png_data;