Don't rely on the pending NavigationEntry for location.replace.

content/browser/frame_host/navigation_controller_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.
 
 /*
  * Copyright (C) 2006, 2007, 2008, 2009 Apple Inc. All rights reserved.
  * Copyright (C) 2008 Nokia Corporation and/or its subsidiary(-ies)
  * Copyright (C) 2008, 2009 Torch Mobile Inc. All rights reserved.
  *     (http://www.torchmobile.com/)
  *
@@ skipping 829 matching lines @@
     pending_entry_->set_restore_type(NavigationEntryImpl::RESTORE_NONE);
 
   // If we are doing a cross-site reload, we need to replace the existing
   // navigation entry, not add another entry to the history. This has the side
   // effect of removing forward browsing history, if such existed. Or if we are
   // doing a cross-site redirect navigation, we will do a similar thing.
   //
   // If this is an error load, we may have already removed the pending entry
   // when we got the notice of the load failure. If so, look at the copy of the
   // pending parameters that were saved.
+  //
+  // TODO(creis): This block should be unnecessary now that we pass
+  // params.should_replace_current_entry.  Remove it once we verify with the
+  // check below.
   if (params.url_is_unreachable && failed_pending_entry_id_ != 0) {
     details->did_replace_entry = failed_pending_entry_should_replace_;
   } else {
     details->did_replace_entry = pending_entry_ &&
                                  pending_entry_->should_replace_entry();
   }
+  CHECK(!details->did_replace_entry || params.should_replace_current_entry);
+  if (params.should_replace_current_entry)
+    details->did_replace_entry = true;
 
   // Do navigation-type specific actions. These will make and commit an entry.
   details->type = ClassifyNavigation(rfh, params);
 
   // is_in_page must be computed before the entry gets committed.
   details->is_in_page = IsURLInPageNavigation(
       params.url, params.was_within_same_page, rfh);
 
   switch (details->type) {
     case NAVIGATION_TYPE_NEW_PAGE:
@@ skipping 124 matching lines @@
     // navigated on a popup navigated to about:blank (the iframe would be
     // written into the popup by script on the main page). For these cases,
     // there isn't any navigation stuff we can do, so just ignore it.
     if (!GetLastCommittedEntry())
       return NAVIGATION_TYPE_NAV_IGNORE;
 
     // Valid subframe navigation.
     return NAVIGATION_TYPE_NEW_SUBFRAME;
   }
 
+  // Cross-process location.replace navigations should be classified as New with
+  // replacement rather than ExistingPage, since it is not safe to reuse the
+  // NavigationEntry.
+  // TODO(creis): Have the renderer classify location.replace as
+  // did_create_new_entry for all cases and eliminate this special case.  This
+  // requires updating several test expectations.  See https://crbug.com/317812.

[Avi (use Gerrit), 2016/02/08 16:02:28]

Wrong bug number; you mean 317872.

[Charlie Reis, 2016/02/08 17:13:21]

Done.

+  if (!rfh->GetParent() && GetLastCommittedEntry() &&
+      GetLastCommittedEntry()->site_instance() != rfh->GetSiteInstance() &&
+      params.should_replace_current_entry) {
+    return NAVIGATION_TYPE_NEW_PAGE;
+  }
+
   // We only clear the session history when navigating to a new page.
   DCHECK(!params.history_list_was_cleared);
 
   if (rfh->GetParent()) {
     // All manual subframes would be did_create_new_entry and handled above, so
     // we know this is auto.
     if (GetLastCommittedEntry()) {
       return NAVIGATION_TYPE_AUTO_SUBFRAME;
     } else {
       // We ignore subframes created in non-committed pages; we'd appreciate if
@@ skipping 1034 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content