PlzNavigate: test updates post beforeUnload IPC refactor.

content/test/test_render_frame_host.cc

 // Copyright 2013 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 "content/test/test_render_frame_host.h"
 
 #include "base/command_line.h"
 #include "content/browser/frame_host/frame_tree.h"
 #include "content/browser/frame_host/navigation_request.h"
 #include "content/browser/frame_host/navigator.h"
@@ skipping 182 matching lines @@
   params.page_state = PageState::CreateForTesting(
       url,
       false,
       file_path_for_history_item ? "data" : NULL,
       file_path_for_history_item);
 
   FrameHostMsg_DidCommitProvisionalLoad msg(GetRoutingID(), params);
   OnDidCommitProvisionalLoad(msg);
 }
 
-void TestRenderFrameHost::SendBeginNavigationWithURL(const GURL& url,
-                                                     bool has_user_gesture) {
-  BeginNavigationParams begin_params("GET", std::string(), net::LOAD_NORMAL,
-                                     has_user_gesture);
-  CommonNavigationParams common_params;
-  common_params.url = url;
-  common_params.referrer = Referrer(GURL(), blink::WebReferrerPolicyDefault);
-  common_params.transition = ui::PAGE_TRANSITION_LINK;
-  OnBeginNavigation(common_params, begin_params,
-                    scoped_refptr<ResourceRequestBody>());
+void TestRenderFrameHost::SendRendererInitiatedNavigationRequest(
+    const GURL& url,
+    bool has_user_gesture) {
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    BeginNavigationParams begin_params("GET", std::string(), net::LOAD_NORMAL,
+                                       has_user_gesture);
+    CommonNavigationParams common_params;
+    common_params.url = url;
+    common_params.referrer = Referrer(GURL(), blink::WebReferrerPolicyDefault);
+    common_params.transition = ui::PAGE_TRANSITION_LINK;
+    OnBeginNavigation(common_params, begin_params,
+                      scoped_refptr<ResourceRequestBody>());
+  }
 }
 
 void TestRenderFrameHost::DidDisownOpener() {
   OnDidDisownOpener();
 }
 
-void TestRenderFrameHost::PrepareForCommit(const GURL& url) {
-  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kEnableBrowserSideNavigation)) {
-    SendBeforeUnloadACK(true);
-    return;
-  }
+void TestRenderFrameHost::PrepareForCommit() {
+  PrepareForCommitInternal(nullptr);
+}
 
-  // PlzNavigate
-  // Simulate the network stack commit without any redirects.
-  NavigationRequest* request =
-      static_cast<NavigatorImpl*>(frame_tree_node_->navigator())
-          ->GetNavigationRequestForNodeForTesting(frame_tree_node_);
-
-  // We are simulating a renderer-initiated user-initiated navigation.
-  if (!request) {
-    SendBeginNavigationWithURL(url, true);
-    request = static_cast<NavigatorImpl*>(frame_tree_node_->navigator())
-        ->GetNavigationRequestForNodeForTesting(frame_tree_node_);
-  }
-  ASSERT_TRUE(request);
-
-  // We may not have simulated the renderer response to the navigation request.
-  // Do that now.
-  if (request->state() == NavigationRequest::WAITING_FOR_RENDERER_RESPONSE)
-    SendBeforeUnloadACK(true);
-
-  // We have already simulated the IO thread commit. Only the
-  // DidCommitProvisionalLoad from the renderer is missing.
-  if (request->state() == NavigationRequest::RESPONSE_STARTED)
-    return;
-
-  ASSERT_TRUE(request->state() == NavigationRequest::STARTED);
-  TestNavigationURLLoader* url_loader =
-      static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
-  ASSERT_TRUE(url_loader);
-  scoped_refptr<ResourceResponse> response(new ResourceResponse);
-  url_loader->CallOnResponseStarted(response, MakeEmptyStream());
+void TestRenderFrameHost::PrepareForCommitWithServerRedirect(
+    const GURL& redirect_url) {
+  PrepareForCommitInternal(&redirect_url);
 }
 
 void TestRenderFrameHost::SendBeforeUnloadHandlersPresent(bool present) {
   OnBeforeUnloadHandlersPresent(present);
 }
 
 void TestRenderFrameHost::SendUnloadHandlersPresent(bool present) {
   OnUnloadHandlersPresent(present);
 }
 
+void TestRenderFrameHost::PrepareForCommitInternal(const GURL* redirect_url) {
+  if (!base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kEnableBrowserSideNavigation)) {
+    // Non PlzNavigate
+    SendBeforeUnloadACK(true);
+    return;
+  }
+
+  // PlzNavigate
+  NavigationRequest* request =
+      static_cast<NavigatorImpl*>(frame_tree_node_->navigator())
+          ->GetNavigationRequestForNodeForTesting(frame_tree_node_);
+  CHECK(request);
+
+  // Simulate a beforeUnload ACK from the renderer if the browser is waiting for
+  // it.
+  if (request->state() == NavigationRequest::WAITING_FOR_RENDERER_RESPONSE)
+    SendBeforeUnloadACK(true);
+
+  // We have already simulated the IO thread commit. Only the

[nasko, 2015/02/26 01:27:42]

This comment is confusing. Commit doesn't happen until we get
CallOnResponseStarted, which is at the end of this method, yet the comment
implies that it must have happened. Shouldn't it be conditional?

[carlosk, 2015/02/26 11:03:45]

Yes, this was/is in the wrong place. I'm moving it inside the if-block.

+  // DidCommitProvisionalLoad from the renderer is missing.
+  if (request->state() == NavigationRequest::RESPONSE_STARTED) {
+    // If a server redirect was requested, the response to the navigation
+    // request should not have started.
+    CHECK(!redirect_url);
+    return;
+  }
+
+  CHECK(request->state() == NavigationRequest::STARTED);
+
+  TestNavigationURLLoader* url_loader =
+      static_cast<TestNavigationURLLoader*>(request->loader_for_testing());
+  CHECK(url_loader);
+
+  // If a |redirect_url| was provided, simulate a server redirect
+  if (redirect_url)
+    url_loader->SimulateServerRedirect(*redirect_url);
+
+  // Simulate the network stack commit
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  url_loader->CallOnResponseStarted(response, MakeEmptyStream());

[nasko, 2015/02/26 01:27:42]

This technically will commit, right? Does it belong here?

[carlosk, 2015/02/26 11:03:45]

Yes, see the reply to your comment in navigation_controller_impl_unittest.cc.

+}
+
 }  // namespace content