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 210 matching lines @@
   low_water_mark_ = high_water_mark_ = t;
   return t;
 }
 
 NavigationControllerImpl::NavigationControllerImpl(
     NavigationControllerDelegate* delegate,
     BrowserContext* browser_context)
     : browser_context_(browser_context),
       pending_entry_(NULL),
       failed_pending_entry_id_(0),
-      failed_pending_entry_should_replace_(false),
       last_committed_entry_index_(-1),
       pending_entry_index_(-1),
       transient_entry_index_(-1),
       delegate_(delegate),
       max_restored_page_id_(-1),
       ssl_manager_(this),
       needs_reload_(false),
       is_initial_navigation_(true),
       in_navigate_to_pending_entry_(false),
       pending_reload_(NO_RELOAD),
@@ skipping 611 matching lines @@
 
   // If there is a pending entry at this point, it should have a SiteInstance,
   // except for restored entries.
   DCHECK(pending_entry_index_ == -1 ||
          pending_entry_->site_instance() ||
          pending_entry_->restore_type() != NavigationEntryImpl::RESTORE_NONE);
   if (pending_entry_ &&
       pending_entry_->restore_type() != NavigationEntryImpl::RESTORE_NONE)
     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.
-  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();
-  }
+  // The renderer tells us whether the navigation replaces the current entry.
+  details->did_replace_entry = params.should_replace_current_entry;
 
   // 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/317872.
+  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 963 matching lines @@
 
 void NavigationControllerImpl::DiscardPendingEntry(bool was_failure) {
   // It is not safe to call DiscardPendingEntry while NavigateToEntry is in
   // progress, since this will cause a use-after-free.  (We only allow this
   // when the tab is being destroyed for shutdown, since it won't return to
   // NavigateToEntry in that case.)  http://crbug.com/347742.
   CHECK(!in_navigate_to_pending_entry_ || delegate_->IsBeingDestroyed());
 
   if (was_failure && pending_entry_) {
     failed_pending_entry_id_ = pending_entry_->GetUniqueID();
-    failed_pending_entry_should_replace_ =
-        pending_entry_->should_replace_entry();
   } else {
     failed_pending_entry_id_ = 0;
   }
 
   if (pending_entry_index_ == -1)
     delete pending_entry_;
   pending_entry_ = NULL;
   pending_entry_index_ = -1;
 }
 
@@ skipping 60 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content