Don't clear the forward history when replacing entries.

content/browser/frame_host/navigation_controller_impl.cc

Index: content/browser/frame_host/navigation_controller_impl.cc
diff --git a/content/browser/frame_host/navigation_controller_impl.cc b/content/browser/frame_host/navigation_controller_impl.cc
index 2229ff0952c79f9f8b9d32bb552f026c7c78cb48..19f57cabcd9553d372b31782ddf84da176fa6378 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -1651,17 +1651,28 @@ void NavigationControllerImpl::InsertOrReplaceEntry(
   DiscardNonCommittedEntriesInternal();
 
   int current_size = static_cast<int>(entries_.size());
-  DCHECK_IMPLIES(replace, current_size > 0);
+
+  // When replacing, don't prune the forward history.
+  if (replace) {
+    DCHECK_GT(current_size, 0);
+    int32 page_id = entry->GetPageID();
+
+    // ScopedVectors don't automatically delete the replaced value, so make sure

[Charlie Reis, 2015/07/16 23:51:14]

Nick and I took a look, and there doesn't appear to be a safe way to replace an
entry in a ScopedVector without leaking the old value, since operator[] deals
with direct references in the underlying array.  This isn't great, but it makes
sure the old value gets deleted, and we couldn't think of a cleaner option.

[Avi (use Gerrit), 2015/07/17 02:17:24]

Yeah. As soon as we can use the C++11 library we'll switch all use of
ScopedVector to std::vector<std::unique_ptr>. Until then, this is about as good
as we can make it.

[dcheng, 2015/07/17 03:49:59]

An alternate way of doing this which maintains the unique ownership invariant of
scoped_ptr is this:
entries_.push_back(entry);
using std::swap;
swap(entries[last_committed_entry_index_], entries[entries.size() - 1]);
entries_.pop_back();

[ncarter (slow), 2015/07/17 16:54:20]

reinterpret_cast<scoped_ptr<NavigationEntryImpl>&>(entries[last_commited_entry_index_])
= entry.Pass();

, obviously. Although I'm kidding, this is actually how the google3
PointerVector's operator[] worked until recently. (cl/60585078)

[Charlie Reis, 2015/07/17 17:00:11]

Heh, I've stumbled into a fun corner case, I see.  :)

I'm inclined to avoid the extra array operations and stick with the current
approach if we'll be able to use the C++11 library before too long. 
std::vector<std::unique_ptr> seems like it will be the best option.

+    // the previous value gets deleted.
+    scoped_ptr<NavigationEntryImpl> old_entry(
+        entries_[last_committed_entry_index_]);
+    entries_[last_committed_entry_index_] = entry.release();
+
+    // This is a new page ID, so we need everybody to know about it.
+    delegate_->UpdateMaxPageID(page_id);
+    return;
+  }
 
   if (current_size > 0) {
     // Prune any entries which are in front of the current entry.
-    // Also prune the current entry if we are to replace the current entry.
     // last_committed_entry_index_ must be updated here since calls to
     // NotifyPrunedEntries() below may re-enter and we must make sure
     // last_committed_entry_index_ is not left in an invalid state.
-    if (replace)
-      --last_committed_entry_index_;
-
     int num_pruned = 0;
     while (last_committed_entry_index_ < (current_size - 1)) {
       num_pruned++;