OOPIF: Fix subframe back/forward after recreating FTNs.

content/browser/frame_host/navigator_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/navigator_impl.h"
 
 #include <utility>
 
 #include "base/metrics/histogram.h"
 #include "base/time/time.h"
@@ skipping 416 matching lines @@
 }
 
 bool NavigatorImpl::NavigateNewChildFrame(
     RenderFrameHostImpl* render_frame_host,
     const std::string& unique_name) {
   NavigationEntryImpl* entry =
       controller_->GetEntryWithUniqueID(render_frame_host->nav_entry_id());
   if (!entry)
     return false;
 
+  DCHECK_EQ(render_frame_host->frame_tree_node()
+                ->current_replication_state()
+                .unique_name,
+            unique_name);

[alexmos, 2016/04/06 17:14:22]

Would we want to strengthen this to a CHECK at some point, and eventually remove
frame_unique_name from  FrameHostMsg_OpenURL_Params, which seems redundant now
that we have it in replication state?

[Charlie Reis, 2016/04/12 16:25:51]

We avoid CHECKs on messages received from the renderer, since it could lie just
to take down the browser process.  This is more of a sanity check that we don't
diverge during testing.

And yes, we can probably remove the unique_name from this IPC eventually.  I
added a TODO for it.

[alexmos, 2016/04/12 18:55:40]

Acknowledged.

   FrameNavigationEntry* frame_entry =
-      entry->GetFrameEntryByUniqueName(unique_name);
+      entry->GetFrameEntry(render_frame_host->frame_tree_node());
   if (!frame_entry)
     return false;
 
-  // Update the FrameNavigationEntry's FrameTreeNode ID (which is currently the
-  // ID of the old FrameTreeNode that no longer exists) to be the ID of the
-  // newly created frame.
-  frame_entry->set_frame_tree_node_id(
-      render_frame_host->frame_tree_node()->frame_tree_node_id());
-
   return NavigateToEntry(render_frame_host->frame_tree_node(), *frame_entry,
                          *entry, NavigationControllerImpl::NO_RELOAD, false,
                          false);
 }
 
 void NavigatorImpl::DidNavigate(
     RenderFrameHostImpl* render_frame_host,
     const FrameHostMsg_DidCommitProvisionalLoad_Params& params) {
   FrameTree* frame_tree = render_frame_host->frame_tree_node()->frame_tree();
   bool oopifs_possible = SiteIsolationPolicy::AreCrossProcessFramesPossible();
@@ skipping 315 matching lines @@
     } else {
       // If there's no last committed entry, create an entry for about:blank
       // with a subframe entry for our destination.
       // TODO(creis): Ensure this case can't exist in https://crbug.com/524208.
       entry = NavigationEntryImpl::FromNavigationEntry(
           controller_->CreateNavigationEntry(
               GURL(url::kAboutBlankURL), referrer_to_use, page_transition,
               is_renderer_initiated, std::string(),
               controller_->GetBrowserContext()));
     }
-    entry->AddOrUpdateFrameEntry(node, std::string(), -1, -1, nullptr, dest_url,
+    entry->AddOrUpdateFrameEntry(node, -1, -1, nullptr, dest_url,
                                  referrer_to_use, PageState());
   } else {
     // Main frame case.
     entry = NavigationEntryImpl::FromNavigationEntry(
         controller_->CreateNavigationEntry(
             dest_url, referrer_to_use, page_transition, is_renderer_initiated,
             std::string(), controller_->GetBrowserContext()));
   }
 
   // The source_site_instance may matter for navigations via RenderFrameProxy.
@@ skipping 341 matching lines @@
   if (pending_entry != controller_->GetVisibleEntry() ||
       !should_preserve_entry) {
     controller_->DiscardPendingEntry(true);
 
     // Also force the UI to refresh.
     controller_->delegate()->NotifyNavigationStateChanged(INVALIDATE_TYPE_URL);
   }
 }
 
 }  // namespace content