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

content/browser/frame_host/navigation_controller_impl_browsertest.cc

 // Copyright 2014 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.
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/strings/stringprintf.h"
 #include "content/browser/frame_host/frame_navigation_entry.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/web_contents/web_contents_impl.h"
 #include "content/public/browser/render_view_host.h"
 #include "content/public/browser/web_contents.h"
 #include "content/public/browser/web_contents_observer.h"
 #include "content/public/common/bindings_policy.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/test/browser_test_utils.h"
 #include "content/public/test/content_browser_test.h"
 #include "content/public/test/content_browser_test_utils.h"
 #include "content/public/test/test_navigation_observer.h"
 #include "content/public/test/test_utils.h"
 #include "content/shell/browser/shell.h"
 #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 {
 
 class NavigationControllerBrowserTest : public ContentBrowserTest {
  protected:
   void SetUpOnMainThread() override {
     host_resolver()->AddRule("*", "127.0.0.1");
     ASSERT_TRUE(embedded_test_server()->InitializeAndWaitUntilReady());
   }
 };
@@ skipping 331 matching lines @@
 
   // The transition_type of the last navigation.
   ui::PageTransition transition_type_;
 
   // The MessageLoopRunner used to spin the message loop.
   scoped_refptr<MessageLoopRunner> message_loop_runner_;
 };
 
 }  // 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.
 
 // Verify that navigations for NAVIGATION_TYPE_NEW_PAGE are correctly
 // classified.
 IN_PROC_BROWSER_TEST_F(NavigationControllerBrowserTest,
                        NavigationTypeClassification_NewPage) {
   NavigateToURL(shell(), GURL(url::kAboutBlankURL));
 
   FrameTreeNode* root =
@@ skipping 512 matching lines @@
     FrameNavigationEntry* frame_entry =
         entry->root_node()->children[0]->frame_entry.get();
     EXPECT_EQ(frame_url, frame_entry->url());
   } else {
     // There are no subframe FrameNavigationEntries by default.
     EXPECT_EQ(0U, entry->root_node()->children.size());
   }
 }
 
 }  // namespace content