Remove existing FrameNavigationEntry when new named frame is added.

content/browser/frame_host/navigation_entry_impl.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/navigation_entry_impl.h"
 
 #include <stddef.h>
 
 #include <queue>
 #include <utility>
@@ skipping 770 matching lines @@
   NavigationEntryImpl::TreeNode* tree_node = FindFrameEntry(frame_tree_node);
   return tree_node ? tree_node->frame_entry.get() : nullptr;
 }
 
 void NavigationEntryImpl::ClearChildren(FrameTreeNode* frame_tree_node) {
   NavigationEntryImpl::TreeNode* tree_node = FindFrameEntry(frame_tree_node);
   if (tree_node)
     tree_node->children.clear();
 }
 
+void NavigationEntryImpl::ClearNamedFrameEntries(
+    const std::string& frame_unique_name) {
+  NavigationEntryImpl::TreeNode* node = nullptr;
+  NavigationEntryImpl::TreeNode* parent = nullptr;
+  std::queue<
+      std::pair<NavigationEntryImpl::TreeNode*, NavigationEntryImpl::TreeNode*>>

[Charlie Reis, 2016/07/27 22:48:28]

I suppose that's what we get for not having a parent pointer on TreeNode.  Ah
well.  :)

[nasko, 2016/07/29 15:52:21]

Acknowledged.

+      work_queue;
+  work_queue.push(std::make_pair(root_node(), nullptr));
+  int count = 0;
+
+  while (!work_queue.empty()) {
+    node = work_queue.front().first;
+    parent = work_queue.front().second;
+    work_queue.pop();
+
+    if (parent && node->frame_entry->frame_unique_name() == frame_unique_name) {

[Charlie Reis, 2016/07/27 22:48:28]

I'm curious if it would be better to use MatchesFrame here, as we do in
FindFrameEntry below.  Implications:
1) This method would take in a FTN rather than a unique name.
2) We would probably want to call this something like
ClearStaleFrameEntries(new_subframe), which might make its purpose clearer.
3) We wouldn't need to check |parent| on this line, since we would never pass in
a root FTN.  (Maybe we could DCHECK that the FTN has a parent on the first line
of this method.)
4) We wouldn't be adding another different way to tell whether a FNE matches
(beyond MatchesFrame vs AddOrUpdateFrameEntry).

Thoughts?

[nasko, 2016/07/29 15:52:22]

Done.

+      auto it =
+          std::find(parent->children.begin(), parent->children.end(), node);
+      if (it != parent->children.end()) {

[Charlie Reis, 2016/07/27 22:48:27]

This shouldn't ever fail, should it?

[nasko, 2016/07/29 15:52:21]

No.

+        parent->children.erase(it);
+        ++count;
+      }
+      continue;
+    }
+
+    // Enqueue any children and keep looking.
+    for (auto* child : node->children)
+      work_queue.push(std::make_pair(child, node));
+  }
+
+  // There should be at most one frame matching the |frame_unique_name|.
+  DCHECK_GE(1, count);
+}
+
 void NavigationEntryImpl::SetScreenshotPNGData(
     scoped_refptr<base::RefCountedBytes> png_data) {
   screenshot_ = png_data;
   if (screenshot_.get())
     UMA_HISTOGRAM_MEMORY_KB("Overscroll.ScreenshotSize", screenshot_->size());
 }
 
 GURL NavigationEntryImpl::GetHistoryURLForDataURL() const {
   return GetBaseURLForDataURL().is_empty() ? GURL() : GetVirtualURL();
 }
@@ skipping 10 matching lines @@
       return node;
 
     // Enqueue any children and keep looking.
     for (auto* child : node->children)
       work_queue.push(child);
   }
   return nullptr;
 }
 
 }  // namespace content