Solidify Entry discarding logic (NavigationHandle keeps its ID)

content/browser/frame_host/navigation_controller_impl_unittest.cc

Index: content/browser/frame_host/navigation_controller_impl_unittest.cc
diff --git a/content/browser/frame_host/navigation_controller_impl_unittest.cc b/content/browser/frame_host/navigation_controller_impl_unittest.cc
index e0f6144a24d8b927361eca2dfb713dfddb48d01e..f8732d105d27cdcf9ebb98349e828ba2f7efe865 100644
--- a/content/browser/frame_host/navigation_controller_impl_unittest.cc
+++ b/content/browser/frame_host/navigation_controller_impl_unittest.cc
@@ -384,6 +384,35 @@ TEST_F(NavigationControllerTest, GoToOffset) {
   }
 }
 
+TEST_F(NavigationControllerTest, DontDiscardWrongPendingEntry) {

[Charlie Reis, 2016/02/05 19:36:11]

nit: Let's add a comment with the bug number and a brief description, including
your repro steps:
1. Pending entry for A is created
2. DidStartProvisionalLoad message for A
3. Pending entry for B is created
4. DidFailProvisionalLoad message for A
5. DidStartProvisionalLoad message for B

[Charlie Harrison, 2016/02/05 23:10:23]

Done.

+  NavigationControllerImpl& controller = controller_impl();
+  GURL initial_url("http://www.google.com");
+  GURL url_1("http://foo.com");
+  GURL url_2("http://foo2.com");
+
+  // Navigate inititally. This is the url that could erroneously be the visible
+  // entry when url_1 fails.
+  NavigateAndCommit(initial_url);
+
+  // Set the pending entry as url_1 and receive the DidStartProvisionalLoad
+  // message, creating the NavigationHandle.
+  controller.LoadURL(
+      url_1, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
+  main_test_rfh()->SimulateNavigationStart(url_1);
+
+  // Navigate to url_2, aborting url_1 before the DidStartProvisionalLoad
+  // message is received for url_2. Do not discard the pending entry for url_2
+  // here.
+  controller.LoadURL(url_2, Referrer(), ui::PAGE_TRANSITION_TYPED,
+                     std::string());
+  main_test_rfh()->SimulateNavigationError(url_1, net::ERR_ABORTED);
+
+  // Get the DidStartProvisionalLoad message for url_2.
+  main_test_rfh()->SimulateNavigationStart(url_2);
+
+  EXPECT_EQ(controller.GetVisibleEntry()->GetURL(), url_2);

[Charlie Reis, 2016/02/05 19:36:11]

nit: Copy this EXPECT_EQ after the LoadURL(url_2) and the
SimulateNavigationError.

Maybe also have it for url_1 after the SimulateNavigationStart(url_1)?

[Charlie Harrison, 2016/02/05 23:10:23]

Done.

+}
+
 TEST_F(NavigationControllerTest, LoadURL) {
   NavigationControllerImpl& controller = controller_impl();
   TestNotificationTracker notifications;