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 171 matching lines @@
       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>());
+  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 it's being expected.

[clamy, 2015/02/25 14:29:49]

nit: s/it's being expected/the browser is waiting for it.

[carlosk, 2015/02/25 16:08:00]

Done.

+  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) {
+    CHECK(!redirect_url) << "Unable to simulate server redirect to \"" <<

[clamy, 2015/02/25 14:29:49]

I noticed you like to add log to the CHECKs. This is not usually done in the
code though I cannot find an explicit thing against it in the style guide.
However it seems we are removing all stream arguments when building an official
release build
(https://code.google.com/p/chromium/codesearch#chromium/src/base/logging.h&sq=...).
So maybe we should avoid it here and just add a comment explaining why the CHECK
is there. Should it fail people will look at the code anyway and see the
comment.

[carlosk, 2015/02/25 16:08:00]

I think this still makes it easier when running a debug build as the message
will appear in the log. But I agree that the person will look at the code anyway
and see the comment, so... Done.

[clamy, 2015/02/25 17:10:33]

I would be fine if this was a DCHECK, but it is a CHECK therefore present in
Release builds as well.

[carlosk, 2015/02/25 19:30:21]

Acknowledged.

+        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());
+}
+
 }  // namespace content