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

content/browser/frame_host/navigation_controller_impl.h

 // 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.
 
 #ifndef CONTENT_BROWSER_FRAME_HOST_NAVIGATION_CONTROLLER_IMPL_H_
 #define CONTENT_BROWSER_FRAME_HOST_NAVIGATION_CONTROLLER_IMPL_H_
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/gtest_prod_util.h"
@@ skipping 181 matching lines @@
   // Takes a screenshot of the page at the current state.
   void TakeScreenshot();
 
   // Sets the screenshot manager for this NavigationControllerImpl. The
   // controller takes ownership of the screenshot manager and destroys it when
   // a new screenshot-manager is set, or when the controller is destroyed.
   // Setting a NULL manager recreates the default screenshot manager and uses
   // that.
   void SetScreenshotManager(NavigationEntryScreenshotManager* manager);
 
-  // Discards only the pending entry.
-  void DiscardPendingEntry();
+  // Discards only the pending entry. |was_failure| should be set if the pending
+  // entry is being discarded because it failed to load.
+  void DiscardPendingEntry(bool was_failure);
 
  private:
   friend class RestoreHelper;
 
   FRIEND_TEST_ALL_PREFIXES(NavigationControllerTest,
                            PurgeScreenshot);
   FRIEND_TEST_ALL_PREFIXES(TimeSmoother, Basic);
   FRIEND_TEST_ALL_PREFIXES(TimeSmoother, SingleDuplicate);
   FRIEND_TEST_ALL_PREFIXES(TimeSmoother, ManyDuplicates);
   FRIEND_TEST_ALL_PREFIXES(TimeSmoother, ClockBackwardsJump);
@@ skipping 124 matching lines @@
 
   // An entry we haven't gotten a response for yet.  This will be discarded
   // when we navigate again.  It's used only so we know what the currently
   // displayed tab is.
   //
   // This may refer to an item in the entries_ list if the pending_entry_index_
   // == -1, or it may be its own entry that should be deleted. Be careful with
   // the memory management.
   NavigationEntryImpl* pending_entry_;
 
-  // currently visible entry
+  // If a new entry fails loading, it is temporarily held here until the error
+  // page is shown.

[Charlie Reis, 2015/04/03 20:59:46]

This needs to mention that it will stick around until the next commit if there
is no error page (e.g., Stop, 204, downloads).

We should also have a TODO with bug # to remove this and find a different way to
match up DidFail to the corresponding error page's DidCommit, because this
approach can't handle navigations in multiple frames.

[Avi (use Gerrit), 2015/04/06 19:38:17]

Done.

+  scoped_ptr<NavigationEntryImpl> failed_pending_entry_;

[Charlie Reis, 2015/04/03 20:59:46]

How bad would it be to store just the things we need from the failed entry?  The
entry is pretty big and (as noted above) it may stick around too long in a lot
of cases.  I'm concerned about things that the entry might be keeping alive,
like site_instance_, source_site_instance_, and  browser_initiated_post_data_.

(This may also make it easier to see what needs to stick around on a per-frame
basis before DidFail and a potential error page DidCommit.)

[Avi (use Gerrit), 2015/04/06 19:38:17]

Done.

+
+  // The index of the currently visible entry.
   int last_committed_entry_index_;
 
-  // index of pending entry if it is in entries_, or -1 if pending_entry_ is a
-  // new entry (created by LoadURL).
+  // The index of the pending entry if it is in entries_, or -1 if
+  // pending_entry_ is a new entry (created by LoadURL).
   int pending_entry_index_;
 
   // The index for the entry that is shown until a navigation occurs.  This is
   // used for interstitial pages. -1 if there are no such entry.
   // Note that this entry really appears in the list of entries, but only
   // temporarily (until the next navigation).  Any index pointing to an entry
   // after the transient entry will become invalid if you navigate forward.
   int transient_entry_index_;
 
   // The delegate associated with the controller. Possibly NULL during
@@ skipping 45 matching lines @@
   TimeSmoother time_smoother_;
 
   scoped_ptr<NavigationEntryScreenshotManager> screenshot_manager_;
 
   DISALLOW_COPY_AND_ASSIGN(NavigationControllerImpl);
 };
 
 }  // namespace content
 
 #endif  // CONTENT_BROWSER_FRAME_HOST_NAVIGATION_CONTROLLER_IMPL_H_