PlzNavigate: Properly set the pending entry in the NavigatorImpl unit test.

content/browser/frame_host/navigator_impl_unittest.cc

Index: content/browser/frame_host/navigator_impl_unittest.cc
diff --git a/content/browser/frame_host/navigator_impl_unittest.cc b/content/browser/frame_host/navigator_impl_unittest.cc
index d5b9da663184bb623d161a1f03a422a56ba09fd9..389c3a104be7b9c00c29c2180a49b94bc2f4a01a 100644
--- a/content/browser/frame_host/navigator_impl_unittest.cc
+++ b/content/browser/frame_host/navigator_impl_unittest.cc
@@ -47,26 +47,41 @@ class NavigatorTestWithBrowserSideNavigation
   }
 
   void RequestNavigation(FrameTreeNode* node, const GURL& url) {
-    RequestNavigationWithParameters(node, url, Referrer(),
-                                    ui::PAGE_TRANSITION_LINK,
+    NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
+        NavigationController::CreateNavigationEntry(
+            url,
+            Referrer(),
+            ui::PAGE_TRANSITION_LINK,
+            false,
+            std::string(),
+            controller().GetBrowserContext()));
+
+    RequestNavigationWithParameters(entry, node, url,
                                     NavigationController::NO_RELOAD);
   }
 
+  void RequestReloadNavigation(
+      FrameTreeNode* node,
+      const GURL& url,
+      NavigationController::ReloadType reload_type) {
+    NavigationEntryImpl* entry = NavigationEntryImpl::FromNavigationEntry(
+        controller().GetLastCommittedEntry());
+    RequestNavigationWithParameters(entry, node, url, reload_type);
+  }
+
   void RequestNavigationWithParameters(
+      NavigationEntryImpl* entry,
       FrameTreeNode* node,
       const GURL& url,
-      const Referrer& referrer,
-      ui::PageTransition transition_type,
       NavigationController::ReloadType reload_type) {
-    scoped_ptr<NavigationEntryImpl> entry(
-        NavigationEntryImpl::FromNavigationEntry(
-            NavigationController::CreateNavigationEntry(
-                url,
-                referrer,
-                transition_type,
-                false,
-                std::string(),
-                controller().GetBrowserContext())));
+    // In ordinary code, NavigatorImpl::RequestNavigation is called from
+    // NavigatorImpl::NavigateToEntry which must be called only with the pending
+    // entry (and thus is only called by NavigatorImpl::NavigateToPendingEntry).
+    // Therefore, if we are calling directly into RequestNavigation, we must
+    // satisfy that constraint.
+    NavigationControllerImpl& navigation_controller_impl =
+        static_cast<NavigationControllerImpl&>(controller());
+    navigation_controller_impl.SetPendingEntry(entry);
     static_cast<NavigatorImpl*>(node->navigator())->RequestNavigation(

[clamy, 2015/03/19 12:18:04]

What I had in mind is to replace this call to NavigatorImpl::RequestNavigation
by a call to NavigationController::LoadURL or
NavigationController::LoadURLWithParams. Both will properly set the pending
entry and then eventually call  NavigatorImpl::RequestNavigation. Also since it
is in the helper function you don't need to rewrite all the tests. It is
possible that the subframe test will fail below, but that's because there are
things we don't properly handle there, so it can be disabled until we do that.

         node, *entry, reload_type, base::TimeTicks::Now());
   }
@@ -184,7 +199,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_EQ(NavigationRequest::RESPONSE_STARTED, request->state());
 
   // Commit the navigation.
-  main_test_rfh()->SendNavigate(0, kUrl2);
+  main_test_rfh()->SendNavigate(1, kUrl2);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, main_test_rfh()->rfh_state());
   EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrl2),
             main_test_rfh()->GetSiteInstance()->GetSiteURL());
@@ -229,7 +244,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_EQ(NavigationRequest::RESPONSE_STARTED, request->state());
 
   // Commit the navigation.
-  main_test_rfh()->SendNavigate(0, kUrl2);
+  main_test_rfh()->SendNavigate(1, kUrl2);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, main_test_rfh()->rfh_state());
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
   EXPECT_FALSE(GetNavigationRequestForFrameTreeNode(node));
@@ -613,7 +628,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
 
   // Commit the navigation.
-  main_test_rfh()->SendNavigate(0, kUrl2);
+  main_test_rfh()->SendNavigate(1, kUrl2);
 
   // Confirm that the commit corresponds to the new request.
   ASSERT_TRUE(main_test_rfh());
@@ -661,7 +676,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
 
   // Commit the navigation.
-  main_test_rfh()->SendNavigate(0, kUrl1);
+  main_test_rfh()->SendNavigate(1, kUrl1);
   EXPECT_EQ(kUrl1, contents()->GetLastCommittedURL());
 }
 
@@ -757,7 +772,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation,
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
 
   // Commit the navigation.
-  main_test_rfh()->SendNavigate(0, kUrl2);
+  main_test_rfh()->SendNavigate(1, kUrl2);
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
 
   // The SiteInstance did not change.
@@ -772,9 +787,7 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, Reload) {
   contents()->NavigateAndCommit(kUrl);
 
   FrameTreeNode* node = main_test_rfh()->frame_tree_node();
-  RequestNavigationWithParameters(node, kUrl, Referrer(),
-                                  ui::PAGE_TRANSITION_LINK,
-                                  NavigationController::RELOAD);
+  RequestReloadNavigation(node, kUrl, NavigationController::RELOAD);
   // A NavigationRequest should have been generated.
   NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(main_request != NULL);
@@ -787,9 +800,8 @@ TEST_F(NavigatorTestWithBrowserSideNavigation, Reload) {
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Now do a shift+reload.
-  RequestNavigationWithParameters(node, kUrl, Referrer(),
-                                  ui::PAGE_TRANSITION_LINK,
-                                  NavigationController::RELOAD_IGNORING_CACHE);
+  RequestReloadNavigation(
+      node, kUrl, NavigationController::RELOAD_IGNORING_CACHE);
   // A NavigationRequest should have been generated.
   main_request = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(main_request != NULL);