Don't clear the forward history when replacing entries.

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 1633 matching lines @@
   // need to keep continuity with the pending entry, so copy the pending entry's
   // unique ID to the committed entry. If the pending_entry_index_ isn't -1,
   // then the renderer navigated on its own, independent of the pending entry,
   // so don't copy anything.
   if (pending_entry_ && pending_entry_index_ == -1)
     entry->set_unique_id(pending_entry_->GetUniqueID());
 
   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++;
       entries_.pop_back();
       current_size--;
     }
     if (num_pruned > 0)  // Only notify if we did prune something.
       NotifyPrunedEntries(this, false, num_pruned);
   }
 
@@ skipping 322 matching lines @@
     }
   }
 }
 
 void NavigationControllerImpl::SetGetTimestampCallbackForTest(
     const base::Callback<base::Time()>& get_timestamp_callback) {
   get_timestamp_callback_ = get_timestamp_callback;
 }
 
 }  // namespace content