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

content/browser/frame_host/navigator_impl_unittest.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/command_line.h"
 #include "base/macros.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/navigation_request.h"
@@ skipping 29 matching lines @@
     EnableBrowserSideNavigation();
     RenderViewHostImplTestHarness::SetUp();
   }
 
   TestNavigationURLLoader* GetLoaderForNavigationRequest(
       NavigationRequest* request) const {
     return static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
   }
 
   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()));
+    NavigationControllerImpl& navigation_controller_impl =
+        static_cast<NavigationControllerImpl&>(controller());
+    navigation_controller_impl.SetPendingEntry(entry);
+
+    RequestNavigationWithParameters(entry, node, url,
                                     NavigationController::NO_RELOAD);
   }
 
-  void RequestNavigationWithParameters(
+  void RequestReloadNavigation(
       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())));
+    NavigationControllerImpl& navigation_controller_impl =
+        static_cast<NavigationControllerImpl&>(controller());
+    navigation_controller_impl.SetPendingEntryAndIndexForTesting(
+        navigation_controller_impl.GetLastCommittedEntryIndex());
+    NavigationEntryImpl* entry =
+        navigation_controller_impl.GetPendingEntry();
+
+    RequestNavigationWithParameters(entry, node, url, reload_type);
+  }
+
+  void RequestNavigationWithParameters(
+      NavigationEntryImpl* entry,
+      FrameTreeNode* node,
+      const GURL& url,
+      NavigationController::ReloadType reload_type) {
     static_cast<NavigatorImpl*>(node->navigator())->RequestNavigation(

[Charlie Reis, 2015/03/17 22:20:22]

The goal of this change is to make sure that |entry| is the pending entry at
this point, right?  Can you add a DCHECK_EQ(controller()->GetPendingEntry(),
entry) here, and maybe a comment for why it matters?

(Or Camille, does that check belong inside NavigatorImpl::RequestNavigation
itself?)

[Avi (use Gerrit), 2015/03/17 23:28:16]

Done.

         node, *entry, reload_type, base::TimeTicks::Now());
   }
 
   NavigationRequest* GetNavigationRequestForFrameTreeNode(
       FrameTreeNode* frame_tree_node) {
     return static_cast<NavigatorImpl*>(frame_tree_node->navigator())
         ->GetNavigationRequestForNodeForTesting(frame_tree_node);
   }
 
   TestRenderFrameHost* GetSpeculativeRenderFrameHost(FrameTreeNode* node) {
@@ skipping 96 matching lines @@
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Have the current RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(request)
       ->CallOnResponseStarted(response, MakeEmptyStream());
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   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());
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
   EXPECT_FALSE(GetNavigationRequestForFrameTreeNode(node));
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
 }
 
 // PlzNavigate: Test a complete renderer-initiated navigation that should be
@@ skipping 24 matching lines @@
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Have the current RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(request)
       ->CallOnResponseStarted(response, MakeEmptyStream());
   EXPECT_TRUE(DidRenderFrameHostRequestCommit(main_test_rfh()));
   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));
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
 
   // The SiteInstance did not change.
   EXPECT_EQ(site_instance_id_1, main_test_rfh()->GetSiteInstance()->GetId());
 }
 
@@ skipping 363 matching lines @@
   // Confirm that the speculative RenderFrameHost was destroyed.
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(request2)
       ->CallOnResponseStarted(response, MakeEmptyStream());
   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());
   EXPECT_EQ(kUrl2, contents()->GetLastCommittedURL());
 }
 
 // PlzNavigate: Test that a renderer-initiated user-initiated navigation is NOT
 // canceled if a renderer-initiated non-user-initiated request is issued in the
 // meantime.
 TEST_F(NavigatorTestWithBrowserSideNavigation,
@@ skipping 27 matching lines @@
   EXPECT_TRUE(request2->begin_params().has_user_gesture);
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(request2)
       ->CallOnResponseStarted(response, MakeEmptyStream());
   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());
 }
 
 // PlzNavigate: Test that a browser-initiated navigation is NOT canceled if a
 // renderer-initiated non-user-initiated request is issued in the meantime.
 TEST_F(NavigatorTestWithBrowserSideNavigation,
        RendererNonUserInitiatedNavigationDoesntCancelBrowserInitiated) {
   const GURL kUrl0("http://www.wikipedia.org/");
   const GURL kUrl1("http://www.chromium.org/");
   const GURL kUrl2("http://www.google.com/");
@@ skipping 75 matching lines @@
   // Confirm that the first loader got destroyed.
   EXPECT_FALSE(loader1);
 
   // Have the RenderFrameHost commit the navigation.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(request2)
       ->CallOnResponseStarted(response, MakeEmptyStream());
   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.
   EXPECT_EQ(site_instance_id_0, main_test_rfh()->GetSiteInstance()->GetId());
 }
 
 // PlzNavigate: Test that a reload navigation is properly signaled to the
 // RenderFrame when the navigation can commit. A speculative RenderFrameHost
 // should not be created at any step.
 TEST_F(NavigatorTestWithBrowserSideNavigation, Reload) {
   const GURL kUrl("http://www.google.com/");
   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);
   EXPECT_EQ(FrameMsg_Navigate_Type::RELOAD,
             main_request->common_params().navigation_type);
   main_test_rfh()->PrepareForCommit();
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   main_test_rfh()->SendNavigate(0, kUrl);
   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);
   EXPECT_EQ(FrameMsg_Navigate_Type::RELOAD_IGNORING_CACHE,
             main_request->common_params().navigation_type);
   main_test_rfh()->PrepareForCommit();
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 }
 
 // PlzNavigate: Confirm that a speculative RenderFrameHost is used when
@@ skipping 199 matching lines @@
   main_rfh->SendRendererInitiatedNavigationRequest(kUrl2, true);
   navigation_request = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(navigation_request);
   EXPECT_EQ(NavigationRequest::RESPONSE_STARTED,
             navigation_request->state());
   EXPECT_FALSE(navigation_request->loader_for_testing());
   EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 }
 
 }  // namespace content