PlzNavigate: add support in several navigation controller unit tests

content/test/test_web_contents.cc

Index: content/test/test_web_contents.cc
diff --git a/content/test/test_web_contents.cc b/content/test/test_web_contents.cc
index 5d46f61acd3f5eec56f2894790a13881cc899ebe..79c38105cdd927673dee84c0539c8c8e000e4a1c 100644
--- a/content/test/test_web_contents.cc
+++ b/content/test/test_web_contents.cc
@@ -137,30 +137,18 @@ void TestWebContents::NavigateAndCommit(const GURL& url) {
   BrowserURLHandlerImpl::GetInstance()->RewriteURLIfNecessary(
       &loaded_url, GetBrowserContext(), &reverse_on_redirect);
 
-  // PlzNavigate
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableBrowserSideNavigation)) {
-    // Simulate the renderer response if there was a live renderer when the
-    // navigation started. Otherwise, it will have been sent directly to the
-    // network stack.
-    if (has_live_renderer)
-      GetMainFrame()->SendBeginNavigationWithURL(url);
-
-    // Now simulate the network stack commit without any redirects. This will
-    // cause the navigation to commit at the same url.
-    FrameTreeNode* frame_tree_node = GetMainFrame()->frame_tree_node();
-    NavigationRequest* request =
-        static_cast<NavigatorImpl*>(frame_tree_node->navigator())
-            ->GetNavigationRequestForNodeForTesting(frame_tree_node);
-    TestNavigationURLLoader* url_loader =
-        static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
-    scoped_refptr<ResourceResponse> response(new ResourceResponse);
-    url_loader->CallOnResponseStarted(response, MakeEmptyStream());
-  }
+  TestRenderFrameHost* old_rfh = GetMainFrame();
+  TestRenderFrameHost* rfh = GetPendingMainFrame();
 
   // LoadURL created a navigation entry, now simulate the RenderView sending
   // a notification that it actually navigated.
-  CommitPendingNavigation();
+  CommitNavigationWithParams(-1, loaded_url, has_live_renderer, false,
+                             NavigationEntryImpl::kInvalidBindings);
+
+  // Simulate the SwapOut_ACK. This is needed when cross-site navigation happens
+  // (old_rfh != rfh).

[nasko, 2014/12/11 00:51:45]

Why have in comment the same condition you are testing in code right after?

[clamy, 2014/12/15 17:01:39]

Done.

+  if (old_rfh != rfh)
+    old_rfh->OnSwappedOut();
 }
 
 void TestWebContents::TestSetIsLoading(bool value) {
@@ -168,24 +156,14 @@ void TestWebContents::TestSetIsLoading(bool value) {
 }
 
 void TestWebContents::CommitPendingNavigation() {
-  // If we are doing a cross-site navigation, this simulates the current RVH
-  // notifying that it has unloaded so the pending RVH is resumed and can
-  // navigate.
-  ProceedWithCrossSiteNavigation();
   TestRenderFrameHost* old_rfh = GetMainFrame();
   TestRenderFrameHost* rfh = GetPendingMainFrame();
-  if (!rfh)
-    rfh = old_rfh;
 
   const NavigationEntry* entry = GetController().GetPendingEntry();
   DCHECK(entry);
-  int page_id = entry->GetPageID();
-  if (page_id == -1) {
-    // It's a new navigation, assign a never-seen page id to it.
-    page_id = GetMaxPageIDForSiteInstance(rfh->GetSiteInstance()) + 1;
-  }
+  CommitNavigationWithParams(-1, entry->GetURL(), true, false,
+                             NavigationEntryImpl::kInvalidBindings);
 
-  rfh->SendNavigate(page_id, entry->GetURL());
   // Simulate the SwapOut_ACK. This is needed when cross-site navigation happens
   // (old_rfh != rfh).
   if (old_rfh != rfh)
@@ -254,6 +232,71 @@ void TestWebContents::TestDidFailLoadWithError(
   frame_tree_.root()->current_frame_host()->OnMessageReceived(msg);
 }
 
+void TestWebContents::ProceedNavigationWithRendererResponse(const GURL& url) {

[nasko, 2014/12/11 00:51:45]

Does it matter that it is "WithRendererResponse"? 

+  // PlzNavigate
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    GetMainFrame()->SendBeginNavigationWithURL(url);
+  } else {
+    ProceedWithCrossSiteNavigation();
+  }
+}
+
+void TestWebContents::CommitPendingNavigationWithHistoryCleared(
+    bool had_live_renderer,
+    bool simulate_history_list_was_cleared) {
+  const NavigationEntry* entry = GetController().GetPendingEntry();
+  DCHECK(entry);
+  CommitNavigationWithParams(-1, entry->GetURL(), had_live_renderer,
+                             simulate_history_list_was_cleared,
+                             NavigationEntryImpl::kInvalidBindings);
+}
+
+void TestWebContents::CommitNavigationWithPageID(int32 page_id,
+                                                 const GURL& url,
+                                                 bool had_live_renderer) {
+  CommitNavigationWithParams(page_id, url, had_live_renderer, false,
+                             NavigationEntryImpl::kInvalidBindings);
+}
+
+void TestWebContents::CommitPendingNavigationWithBindings(
+    int binding_flags,
+    bool had_live_renderer) {
+  const NavigationEntry* entry = GetController().GetPendingEntry();
+  DCHECK(entry);
+  CommitNavigationWithParams(-1, entry->GetURL(), had_live_renderer, false,
+                             binding_flags);
+}
+
+void TestWebContents::CommitPendingNavigationNoLiveRenderer() {

[nasko, 2014/12/11 00:51:45]

This method is almost identical to the latter portion of
TestWebContents::NavigateAndCommit. Is there any reason to have so much code
that is the same be duplicated?

+  TestRenderFrameHost* old_rfh = GetMainFrame();
+  TestRenderFrameHost* rfh = GetPendingMainFrame();
+
+  const NavigationEntry* entry = GetController().GetPendingEntry();
+  DCHECK(entry);
+  CommitNavigationWithParams(-1, entry->GetURL(), false, false,
+                             NavigationEntryImpl::kInvalidBindings);
+
+  // Simulate the SwapOut_ACK. This is needed when cross-site navigation happens
+  // (old_rfh != rfh).

[nasko, 2014/12/11 00:51:45]

nit: Another instance of comment and code being the same.

+  if (old_rfh != rfh)
+    old_rfh->OnSwappedOut();
+}
+
+void TestWebContents::SimulateRendererInitiatedNavigation(int page_id,
+                                                          const GURL& url) {
+  // PlzNavigate
+  // Simulate the beginning of a renderer initiated navigation.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    GetMainFrame()->SendBeginNavigationWithURL(url);
+    SimulateIOThreadResponse();
+  }
+  TestRenderFrameHost* current_rfh = GetMainFrame();
+
+  current_rfh->SendNavigate(page_id, url);
+}
+
 void TestWebContents::CreateNewWindow(
     int render_process_id,
     int route_id,
@@ -284,4 +327,65 @@ void TestWebContents::ShowCreatedWidget(int route_id,
 void TestWebContents::ShowCreatedFullscreenWidget(int route_id) {
 }
 
+void TestWebContents::CommitNavigationWithParams(

[nasko, 2014/12/11 00:51:45]

Shouldn't this method name start with CommitPendingNavigation? It seems all
others have the pending part in the name.

+    int page_id,
+    const GURL& url,
+    bool had_live_renderer,
+    bool simulate_history_list_was_cleared,
+    int binding_flags) {
+  // If we are doing a cross-site navigation, this simulates the current RVH

[nasko, 2014/12/11 00:51:45]

nit: Did you mean RFH?

[clamy, 2014/12/15 17:01:39]

Done.

+  // notifying that it has unloaded so the pending RVH is resumed and can
+  // navigate.
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    ProceedWithCrossSiteNavigation();
+  } else {
+    // Simulate the renderer response if there was a live renderer when the
+    // navigation started. Otherwise, it will have been sent directly to the
+    // network stack.
+    if (had_live_renderer)
+      ProceedNavigationWithRendererResponse(url);
+
+    SimulateIOThreadResponse();
+  }
+
+  TestRenderFrameHost* old_rfh = GetMainFrame();
+  TestRenderFrameHost* rfh = GetPendingMainFrame();
+  if (!rfh)
+    rfh = old_rfh;
+
+  if (simulate_history_list_was_cleared)
+    rfh->set_simulate_history_list_was_cleared(true);
+
+  if (binding_flags != NavigationEntryImpl::kInvalidBindings)
+    rfh->GetRenderViewHost()->AllowBindings(binding_flags);
+
+  int page_id_final = page_id;
+
+  if (page_id_final == -1) {
+    const NavigationEntry* entry = GetController().GetPendingEntry();
+    DCHECK(entry);
+    // If it's a new navigation, assign a never-seen page id to it.
+    page_id_final = entry->GetPageID() == -1 ?
+        GetMaxPageIDForSiteInstance(rfh->GetSiteInstance()) + 1 :
+        entry->GetPageID();
+  }
+
+  rfh->SendNavigate(page_id_final, url);
+}
+
+// PlzNavigate
+void TestWebContents::SimulateIOThreadResponse() {
+  // Simulate the network stack commit without any redirects. This will cause
+  // the navigation to commit at the same url.
+  FrameTreeNode* frame_tree_node = GetMainFrame()->frame_tree_node();
+  NavigationRequest* request =
+      static_cast<NavigatorImpl*>(frame_tree_node->navigator())
+          ->GetNavigationRequestForNodeForTesting(frame_tree_node);
+  TestNavigationURLLoader* url_loader =
+      static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  url_loader->CallOnResponseStarted(response, MakeEmptyStream());
+}
+
 }  // namespace content