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

content/browser/frame_host/navigation_controller_impl_browsertest.cc

Index: content/browser/frame_host/navigation_controller_impl_browsertest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_browsertest.cc b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
index 997a6fdb051ae3ebcf6f6b1de0119286ea08b846..e321af2be31737b079a7ecb3380ef023fe5f567d 100644
--- a/content/browser/frame_host/navigation_controller_impl_browsertest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_browsertest.cc
@@ -25,6 +25,7 @@
 #include "content/test/content_browser_test_utils_internal.h"
 #include "net/dns/mock_host_resolver.h"
 #include "net/test/embedded_test_server/embedded_test_server.h"
+#include "net/test/url_request/url_request_failed_job.h"
 
 namespace content {
 
@@ -376,6 +377,46 @@ class LoadCommittedCapturer : public WebContentsObserver {
 
 }  // namespace
 
+IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
+                       ErrorPageReplacement) {
+  NavigationController& controller = shell()->web_contents()->GetController();
+  GURL error_url(
+      net::URLRequestFailedJob::GetMockHttpUrl(net::ERR_CONNECTION_RESET));
+  net::URLRequestFailedJob::AddUrlHandler();
+
+  NavigateToURL(shell(), GURL(url::kAboutBlankURL));
+  EXPECT_EQ(1, controller.GetEntryCount());
+
+  FrameTreeNode* root =
+      static_cast<WebContentsImpl*>(shell()->web_contents())->
+          GetFrameTree()->root();
+
+  // Navigate to a page that fails to load. It must result in an error page, the
+  // NEW_PAGE navigation type, and an addition to the history list.
+  {
+    FrameNavigateParamsCapturer capturer(root);
+    NavigateFrameToURL(root, error_url);
+    capturer.Wait();
+    EXPECT_EQ(NAVIGATION_TYPE_NEW_PAGE, capturer.details().type);
+    NavigationEntry* entry = controller.GetLastCommittedEntry();
+    EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
+    EXPECT_EQ(2, controller.GetEntryCount());
+  }
+
+  // Navigate again to the page that fails to load. It must result in an error
+  // page, the EXISTING_PAGE navigation type, and no addition to the history

[Charlie Reis, 2015/03/30 21:36:54]

Let's put a bit of your explanation in this comment.  Something like:
We do not use SAME_PAGE here; that case only differs in that it clears the
pending entry, and there is no pending entry after a load failure.

[Avi (use Gerrit), 2015/03/31 16:08:49]

Done.

+  // list.
+  {
+    FrameNavigateParamsCapturer capturer(root);
+    NavigateFrameToURL(root, error_url);
+    capturer.Wait();
+    EXPECT_EQ(NAVIGATION_TYPE_EXISTING_PAGE, capturer.details().type);
+    NavigationEntry* entry = controller.GetLastCommittedEntry();
+    EXPECT_EQ(PAGE_TYPE_ERROR, entry->GetPageType());
+    EXPECT_EQ(2, controller.GetEntryCount());
+  }
+}

[Charlie Reis, 2015/03/30 21:36:54]

Can you add a test for location.replace to error_url (after a navigation to a
real URL)?  That would cover the TODO(davidben) comment we're removing.

[Avi (use Gerrit), 2015/03/31 16:08:49]

Done.

+
 // Various tests for navigation type classifications. TODO(avi): It's rather
 // bogus that the same info is in two different enums; http://crbug.com/453555.