Fix incorrect creation of duplicate navigation entries for repeated page load failures.

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 8ec8dc91047a1ab77effedcb448046b9479f3d1e..7b10e2150588b4829b9dc7c85641fa06d0d5fb9a 100644
--- a/content/browser/frame_host/navigation_controller_impl.cc
+++ b/content/browser/frame_host/navigation_controller_impl.cc
@@ -226,6 +226,8 @@ NavigationControllerImpl::NavigationControllerImpl(
     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),
@@ -796,11 +798,14 @@ bool NavigationControllerImpl::RendererDidNavigate(
 
   // 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.
-  details->did_replace_entry =
-      pending_entry_ && pending_entry_->should_replace_entry();
+  // 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.

[Charlie Reis, 2015/04/07 18:00:47]

Might also be worth mentioning the error page case, and that we need to remember
whether the error page should replace the current entry from DidFail to
DidCommit.

[Avi (use Gerrit), 2015/04/07 18:22:58]

Done.

+  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();
+  }
 
   // Do navigation-type specific actions. These will make and commit an entry.
   details->type = ClassifyNavigation(rfh, params);
@@ -1758,17 +1763,25 @@ void NavigationControllerImpl::FinishRestore(int selected_index,
 }
 
 void NavigationControllerImpl::DiscardNonCommittedEntriesInternal() {
-  DiscardPendingEntry();
+  DiscardPendingEntry(false);
   DiscardTransientEntry();
 }
 
-void NavigationControllerImpl::DiscardPendingEntry() {
+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;