Remove existing FrameNavigationEntry when new named frame is added.

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>
 
 #include "base/bind.h"
 #include "base/callback.h"
 #include "base/containers/hash_tables.h"
 #include "base/lazy_instance.h"
 #include "base/memory/ptr_util.h"
 #include "content/browser/frame_host/frame_tree_node.h"
+#include "content/browser/frame_host/navigation_entry_impl.h"

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

I suppose we should include NavigationController as well.  I'm not thrilled
about adding this dependency here, but I suppose there's no good reason to add a
hop through NavigatorImpl for this.

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

Acknowledged.

 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/render_frame_host_factory.h"
 #include "content/browser/frame_host/render_frame_host_impl.h"
 #include "content/browser/frame_host/render_frame_proxy_host.h"
 #include "content/browser/renderer_host/render_view_host_factory.h"
 #include "content/browser/renderer_host/render_view_host_impl.h"
 #include "content/common/content_switches_internal.h"
 #include "content/common/frame_owner_properties.h"
 #include "content/common/input_messages.h"
 #include "content/common/site_isolation_policy.h"
@@ skipping 149 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;
 
+  // If the last committed NavigationEntry has FrameNavigationEntry with the
+  // same |frame_unique_name|, it has to be removed to avoid conflicts with
+  // future updates.

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

Let's clarify a bit how this is an expected case:

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.

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

Done.

+  NavigationEntryImpl* last_committed_entry = static_cast<NavigationEntryImpl*>(
+      root_->navigator()->GetController()->GetLastCommittedEntry());

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

This should be parent->navigator() to be consistent with line 199 below.

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

Done.

+  if (last_committed_entry)
+    last_committed_entry->ClearNamedFrameEntries(frame_unique_name);
+
   // 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);
 
   // Set sandbox flags and make them effective immediately, since initial
   // sandbox flags should apply to the initial empty document in the frame.
@@ skipping 251 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