PlzNavigate: Speculatively spawns a renderer process for navigations.

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"
 #include "content/browser/frame_host/navigation_request_info.h"
 #include "content/browser/frame_host/navigator.h"
 #include "content/browser/frame_host/navigator_impl.h"
 #include "content/browser/frame_host/render_frame_host_manager.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/streams/stream.h"
 #include "content/common/navigation_params.h"
 #include "content/public/browser/stream_handle.h"
+#include "content/public/common/content_switches.h"
 #include "content/public/common/url_constants.h"
 #include "content/public/common/url_utils.h"
+#include "content/public/test/mock_render_process_host.h"
 #include "content/test/browser_side_navigation_test_utils.h"
 #include "content/test/test_navigation_url_loader.h"
 #include "content/test/test_render_frame_host.h"
 #include "content/test/test_web_contents.h"
 #include "net/base/load_flags.h"
 #include "net/http/http_response_headers.h"
 #include "net/url_request/redirect_info.h"
 #include "ui/base/page_transition_types.h"
 #include "url/url_constants.h"
 
@@ skipping 36 matching lines @@
                 controller().GetBrowserContext())));
     static_cast<NavigatorImpl*>(node->navigator())->RequestNavigation(
         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) {
+    return static_cast<TestRenderFrameHost*>(
+        node->render_manager()->speculative_render_frame_host_.get());
+  }
+
+  // Checks if this RenderFrameHost sent a single FrameMsg_CommitNavigation
+  // since the last clearing of the sink.
+  // Note: user must call ClearMessages on the sink before the commit happens.
+  bool DidRenderFrameHostCommit(RenderFrameHostImpl* rfh) {
+    MockRenderProcessHost* rph =
+        static_cast<MockRenderProcessHost*>(rfh->GetProcess());

[clamy, 2014/12/30 16:57:45]

You can just call process() instead (from RenderViewHostTestHarness).

[carlosk, 2014/12/31 13:39:18]

I can't. For this check to work I need the mock process associated with the
specific RFH that was passed in which is not necessarily the main frame one.

[clamy, 2014/12/31 14:07:09]

Acknowledged.

+    const FrameMsg_CommitNavigation* commitMessage =
+        static_cast<const FrameMsg_CommitNavigation*>(
+            rph->sink().GetUniqueMessageMatching(
+                FrameMsg_CommitNavigation::ID));
+    return commitMessage && rfh->GetRoutingID() == commitMessage->routing_id();
+  }
 };
 
+// PlzNavigate: Test final state after a complete navigation (to avoid repeating

[clamy, 2014/12/30 16:57:45]

nit: "(to avoid repeating these checks)" may be unnecessary.

[carlosk, 2014/12/31 13:39:18]

I'd prefer to leave that note to indicate to future test implementors that they
should not repeat these checks. Sound reasonable?

I also tried to make it clearer: "to avoid repeating these checks in other
tests".

[clamy, 2014/12/31 14:07:09]

Acknowledged.

+// these checks).
+TEST_F(NavigatorTestWithBrowserSideNavigation, NavigationFinishedState) {
+  const GURL kUrl("http://chromium.org/");
+  contents()->NavigateAndCommit(kUrl);
+  ASSERT_TRUE(main_test_rfh());
+  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, main_test_rfh()->rfh_state());
+  EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrl),
+            main_test_rfh()->GetSiteInstance()->GetSiteURL());

[clamy, 2014/12/30 16:57:45]

nit: add an extra-line here.

[carlosk, 2014/12/31 13:39:18]

Done.

+  // After a navigation is finished no speculative RenderFrameHost should
+  // exist.
+  FrameTreeNode* node = main_test_rfh()->frame_tree_node();
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));

[clamy, 2014/12/30 16:57:45]

nit: add an extra line here.

[carlosk, 2014/12/31 13:39:19]

Done.

+  // With PlzNavigate enabled a pending RenderFrameHost should never exist.
+  EXPECT_FALSE(node->render_manager()->pending_frame_host());
+}
+
 // PlzNavigate: Test that a proper NavigationRequest is created by
 // BeginNavigation.
 // Note that all PlzNavigate methods on the browser side require the use of the
 // flag kEnableBrowserSideNavigation.
 TEST_F(NavigatorTestWithBrowserSideNavigation, BeginNavigation) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
   const GURL kUrl3("http://www.gmail.com/");
 
   contents()->NavigateAndCommit(kUrl1);
+  FrameTreeNode* root_node = contents()->GetFrameTree()->root();

[clamy, 2014/12/30 16:57:45]

nit: this line does not need to be moved.

[carlosk, 2014/12/31 13:39:18]

Done.

+  RenderFrameHostManager* root_rfhm = root_node->render_manager();
 
   // Add a subframe.
-  FrameTreeNode* root = contents()->GetFrameTree()->root();
-  TestRenderFrameHost* subframe_rfh = static_cast<TestRenderFrameHost*>(
-      contents()->GetFrameTree()->AddFrame(
-          root, root->current_frame_host()->GetProcess()->GetID(), 14,
+  TestRenderFrameHost* subframe_rfh =
+      static_cast<TestRenderFrameHost*>(contents()->GetFrameTree()->AddFrame(
+          root_node, root_node->current_frame_host()->GetProcess()->GetID(), 14,
           "Child"));
-  EXPECT_TRUE(subframe_rfh);
+  ASSERT_TRUE(subframe_rfh);
 
   FrameTreeNode* subframe_node = subframe_rfh->frame_tree_node();
-  SendRequestNavigation(subframe_rfh->frame_tree_node(), kUrl2);
+  EXPECT_NE(root_rfhm, subframe_node->render_manager());

[clamy, 2014/12/30 16:57:45]

This is unnecessary (you are testing FrameTree functionality here, and not
navigation behavior).

[carlosk, 2014/12/31 13:39:19]

Done.

+
+  SendRequestNavigation(subframe_node, kUrl2);
   // There is no previous renderer in the subframe, so BeginNavigation is
   // handled already.
   NavigationRequest* subframe_request =
       GetNavigationRequestForFrameTreeNode(subframe_node);
   TestNavigationURLLoader* subframe_loader =
       GetLoaderForNavigationRequest(subframe_request);
   ASSERT_TRUE(subframe_request);
   EXPECT_EQ(kUrl2, subframe_request->common_params().url);
   EXPECT_EQ(kUrl2, subframe_loader->common_params().url);
   // First party for cookies url should be that of the main frame.
   EXPECT_EQ(kUrl1, subframe_loader->request_info()->first_party_for_cookies);
   EXPECT_FALSE(subframe_loader->request_info()->is_main_frame);
   EXPECT_TRUE(subframe_loader->request_info()->parent_is_main_frame);
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(root_node));
 
-  SendRequestNavigation(root, kUrl3);
+  // Subframe navigations should not create a speculative RenderFrameHost.
+  // (unless site-per-process is enabled).
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kSitePerProcess)) {
+    EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node));
+  } else {
+    EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node));
+  }
+
+  SendRequestNavigation(root_node, kUrl3);
   // Simulate a BeginNavigation IPC on the main frame.
-  contents()->GetMainFrame()->SendBeginNavigationWithURL(kUrl3);
-  NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(root);
+  main_test_rfh()->SendBeginNavigationWithURL(kUrl3);
+  NavigationRequest* main_request =
+      GetNavigationRequestForFrameTreeNode(root_node);
   TestNavigationURLLoader* main_loader =
       GetLoaderForNavigationRequest(main_request);
   ASSERT_TRUE(main_request);
   EXPECT_EQ(kUrl3, main_request->common_params().url);
   EXPECT_EQ(kUrl3, main_loader->common_params().url);
   EXPECT_EQ(kUrl3, main_loader->request_info()->first_party_for_cookies);
   EXPECT_TRUE(main_loader->request_info()->is_main_frame);
   EXPECT_FALSE(main_loader->request_info()->parent_is_main_frame);
+
+  // Main frame navigations to different sites should use a speculative
+  // RenderFrameHost.
+  EXPECT_TRUE(GetSpeculativeRenderFrameHost(root_node));
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(

[clamy, 2014/12/30 16:57:46]

nit: maybe you should precise that this check is different because the subframe
navigation has not committed yet.

[carlosk, 2014/12/31 13:39:18]

Done.

+          switches::kSitePerProcess)) {
+    EXPECT_TRUE(GetSpeculativeRenderFrameHost(subframe_node));
+  } else {
+    EXPECT_FALSE(GetSpeculativeRenderFrameHost(subframe_node));
+  }
 }
 
 // PlzNavigate: Test that RequestNavigation creates a NavigationRequest and that
 // RenderFrameHost is not modified when the navigation commits.
 TEST_F(NavigatorTestWithBrowserSideNavigation, NoLiveRenderer) {
   const GURL kUrl("http://www.google.com/");
 
   EXPECT_FALSE(main_test_rfh()->render_view_host()->IsRenderViewLive());
-  FrameTreeNode* node = main_test_rfh()->frame_tree_node();
+  RenderFrameHostImpl* rfh = main_test_rfh();
+  FrameTreeNode* node = rfh->frame_tree_node();
   SendRequestNavigation(node, kUrl);
+
+  // A NavigationRequest should have been generated.
   NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(node);
-  // A NavigationRequest should have been generated.
   EXPECT_TRUE(main_request != NULL);
-  RenderFrameHostImpl* rfh = main_test_rfh();
+
+  // As we're re-using the current RenderFrameHost, no speculative one should be
+  // created.
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Now return the response without any redirects. This will cause the
   // navigation to commit at the same URL.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
       response, MakeEmptyStream());
   main_request = GetNavigationRequestForFrameTreeNode(node);
 
   // The main RFH should not have been changed, and the renderer should have
   // been initialized.
   EXPECT_EQ(rfh, main_test_rfh());
   EXPECT_TRUE(main_test_rfh()->IsRenderFrameLive());
   EXPECT_TRUE(main_test_rfh()->render_view_host()->IsRenderViewLive());
 }
 
-// PlzNavigate: Test that commiting an HTTP 204 or HTTP 205 response cancels the
-// navigation.
+// PlzNavigate: Test that committing an HTTP 204 or HTTP 205 response cancels
+// the navigation.
 TEST_F(NavigatorTestWithBrowserSideNavigation, NoContent) {
   const GURL kUrl1("http://www.chromium.org/");
   const GURL kUrl2("http://www.google.com/");
 
   // Load a URL.
   contents()->NavigateAndCommit(kUrl1);
-  RenderFrameHostImpl* rfh = main_test_rfh();
-  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh->rfh_state());
   FrameTreeNode* node = main_test_rfh()->frame_tree_node();
 
   // Navigate to a different site.
-  SendRequestNavigation(node, kUrl2);
+  contents()->GetController().LoadURL(kUrl2, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
   main_test_rfh()->SendBeginNavigationWithURL(kUrl2);
+
   NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(main_request);
 
+  // Navigations to a different site do create a speculative RenderFrameHost.
+  EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
+
   // Commit an HTTP 204 response.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   const char kNoContentHeaders[] = "HTTP/1.1 204 No Content\0\0";
   response->head.headers = new net::HttpResponseHeaders(
       std::string(kNoContentHeaders, arraysize(kNoContentHeaders)));
   GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
       response, MakeEmptyStream());
 
-  // There should be no pending RenderFrameHost; the navigation was aborted.
+  // There should be no pending nor speculative RenderFrameHost; the navigation
+  // was aborted.
   EXPECT_FALSE(GetNavigationRequestForFrameTreeNode(node));
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Now, repeat the test with 205 Reset Content.
 
   // Navigate to a different site again.
-  SendRequestNavigation(node, kUrl2);
+  contents()->GetController().LoadURL(kUrl2, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
   main_test_rfh()->SendBeginNavigationWithURL(kUrl2);
+
   main_request = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(main_request);
+  EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
 
   // Commit an HTTP 205 response.
   response = new ResourceResponse;
   const char kResetContentHeaders[] = "HTTP/1.1 205 Reset Content\0\0";
   response->head.headers = new net::HttpResponseHeaders(
       std::string(kResetContentHeaders, arraysize(kResetContentHeaders)));
   GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
       response, MakeEmptyStream());
 
-  // There should be no pending RenderFrameHost; the navigation was aborted.
+  // There should be no pending nor speculative RenderFrameHost; the navigation
+  // was aborted.
   EXPECT_FALSE(GetNavigationRequestForFrameTreeNode(node));
   EXPECT_FALSE(node->render_manager()->pending_frame_host());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 }
 
 // PlzNavigate: Test that a new RenderFrameHost is created when doing a cross
 // site navigation.
 TEST_F(NavigatorTestWithBrowserSideNavigation, CrossSiteNavigation) {
   const GURL kUrl1("http://www.chromium.org/");
   const GURL kUrl2("http://www.google.com/");
 
   contents()->NavigateAndCommit(kUrl1);
-  RenderFrameHostImpl* rfh = main_test_rfh();
-  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh->rfh_state());
+  process()->sink().ClearMessages();
+  RenderFrameHostImpl* initial_rfh = main_test_rfh();
   FrameTreeNode* node = main_test_rfh()->frame_tree_node();
 
   // Navigate to a different site.
-  SendRequestNavigation(node, kUrl2);
+  contents()->GetController().LoadURL(kUrl2, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
   main_test_rfh()->SendBeginNavigationWithURL(kUrl2);
   NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(main_request);
+  EXPECT_TRUE(GetSpeculativeRenderFrameHost(node));
 
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
       response, MakeEmptyStream());
-  RenderFrameHostImpl* pending_rfh =
-      node->render_manager()->pending_frame_host();
-  ASSERT_TRUE(pending_rfh);
-  EXPECT_NE(pending_rfh, rfh);
-  EXPECT_TRUE(pending_rfh->IsRenderFrameLive());
-  EXPECT_TRUE(pending_rfh->render_view_host()->IsRenderViewLive());
+  TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  ASSERT_TRUE(speculative_rfh);
+  ASSERT_TRUE(DidRenderFrameHostCommit(speculative_rfh));

[clamy, 2014/12/30 16:57:45]

I would use an EXPECT_TRUE here.

[carlosk, 2014/12/31 13:39:18]

Done.

This reminded me to add that similar commit-checks to other tests where we need
to know the RFH that actually committed.

+
+  speculative_rfh->SendNavigate(0, kUrl2);
+
+  RenderFrameHostImpl* final_rfh = main_test_rfh();
+  ASSERT_TRUE(final_rfh);
+  EXPECT_NE(initial_rfh, final_rfh);
+  EXPECT_TRUE(final_rfh->IsRenderFrameLive());
+  EXPECT_TRUE(final_rfh->render_view_host()->IsRenderViewLive());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 }
 
-// PlzNavigate: Test that redirects are followed.
+// PlzNavigate: Test that redirects are followed and the speculative renderer
+// logic behaves as expected.
 TEST_F(NavigatorTestWithBrowserSideNavigation, RedirectCrossSite) {
   const GURL kUrl1("http://www.chromium.org/");
   const GURL kUrl2("http://www.google.com/");
 
   contents()->NavigateAndCommit(kUrl1);
   RenderFrameHostImpl* rfh = main_test_rfh();
-  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh->rfh_state());
   FrameTreeNode* node = main_test_rfh()->frame_tree_node();
 
   // Navigate to a URL on the same site.
-  SendRequestNavigation(node, kUrl1);
+  contents()->GetController().LoadURL(kUrl1, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
   main_test_rfh()->SendBeginNavigationWithURL(kUrl1);
   NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(main_request);
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // It then redirects to another site.
   net::RedirectInfo redirect_info;
   redirect_info.status_code = 302;
   redirect_info.new_method = "GET";
   redirect_info.new_url = kUrl2;
   redirect_info.new_first_party_for_cookies = kUrl2;
- scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(main_request)->CallOnRequestRedirected(
       redirect_info, response);
 
   // The redirect should have been followed.
   EXPECT_EQ(1, GetLoaderForNavigationRequest(main_request)->redirect_count());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Then it commits.
   response = new ResourceResponse;
   GetLoaderForNavigationRequest(main_request)->CallOnResponseStarted(
       response, MakeEmptyStream());
-  RenderFrameHostImpl* pending_rfh =
-      node->render_manager()->pending_frame_host();
-  ASSERT_TRUE(pending_rfh);
-  EXPECT_NE(pending_rfh, rfh);
-  EXPECT_TRUE(pending_rfh->IsRenderFrameLive());
-  EXPECT_TRUE(pending_rfh->render_view_host()->IsRenderViewLive());
+  TestRenderFrameHost* last_speculative_rfh =

[clamy, 2014/12/30 16:57:45]

nit: why is it called last_speculative_rfh if no other speculative rfh was
created before?

[carlosk, 2014/12/31 13:39:19]

Because this will not be the case for much longer... Once we get redirects to
update the speculative RFH.

[clamy, 2014/12/31 14:07:09]

nit: Then could it be called final_speculative_rfh - that works whether we have
one or several.

[carlosk, 2014/12/31 15:39:51]

Done.

+      GetSpeculativeRenderFrameHost(node);
+  EXPECT_TRUE(last_speculative_rfh);
+
+  // And commits provisional load.
+  contents()->CommitPendingNavigation();

[clamy, 2014/12/30 16:57:46]

Could you use TestRenderFrameHost::SendNavigate here?

[carlosk, 2014/12/31 13:39:18]

Done here and for the following cases. And now more surely as we're checking
fort the RFH that actually committed.

But I'm in doubt about the page_id value: do redirects under the same site
increase its value? I assumed not so left the value here at 0 as it's the 1st
time it's being committed to |kUrl2|. I used this same rationale for the
following cases.

[clamy, 2014/12/31 14:07:09]

Yes the value should not be modified by redirects (only the commit of a new
navigation permanently increases the max value).

[carlosk, 2014/12/31 15:39:51]

Acknowledged.

+  RenderFrameHostImpl* final_rfh = main_test_rfh();
+  ASSERT_TRUE(final_rfh);
+  EXPECT_NE(rfh, final_rfh);
+  EXPECT_EQ(last_speculative_rfh, final_rfh);
+  EXPECT_TRUE(final_rfh->IsRenderFrameLive());
+  EXPECT_TRUE(final_rfh->render_view_host()->IsRenderViewLive());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 }
 
-// PlzNavigate: Test that a navigation is cancelled if another request has been
-// issued in the meantime.
+// PlzNavigate: Test that a navigation is canceled if another request has been
+// issued in the meantime. Also confirms that the speculative renderer is
+// correctly updated in the process.
 TEST_F(NavigatorTestWithBrowserSideNavigation, ReplacePendingNavigation) {
   const GURL kUrl0("http://www.wikipedia.org/");
-  const GURL kUrl0_site = SiteInstance::GetSiteForURL(browser_context(), kUrl0);
   const GURL kUrl1("http://www.chromium.org/");
+  const GURL kUrl1_site = SiteInstance::GetSiteForURL(browser_context(), kUrl1);
   const GURL kUrl2("http://www.google.com/");
   const GURL kUrl2_site = SiteInstance::GetSiteForURL(browser_context(), kUrl2);
 
   // Initialization.
   contents()->NavigateAndCommit(kUrl0);
   FrameTreeNode* node = main_test_rfh()->frame_tree_node();
-  EXPECT_EQ(kUrl0_site, main_test_rfh()->GetSiteInstance()->GetSiteURL());
 
   // Request navigation to the 1st URL.
-  SendRequestNavigation(node, kUrl1);
+  contents()->GetController().LoadURL(kUrl1, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
   main_test_rfh()->SendBeginNavigationWithURL(kUrl1);
   NavigationRequest* request1 = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(request1);
   EXPECT_EQ(kUrl1, request1->common_params().url);
   base::WeakPtr<TestNavigationURLLoader> loader1 =
       GetLoaderForNavigationRequest(request1)->AsWeakPtr();
 
+  // Confirm a speculative RFH was created.
+  TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  ASSERT_TRUE(speculative_rfh);
+  int32 site_instance_id_1 = speculative_rfh->GetSiteInstance()->GetId();
+  EXPECT_EQ(kUrl1_site, speculative_rfh->GetSiteInstance()->GetSiteURL());
+
   // Request navigation to the 2nd URL; the NavigationRequest must have been
   // replaced by a new one with a different URL.
-  SendRequestNavigation(node, kUrl2);
+  contents()->GetController().LoadURL(kUrl2, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
   main_test_rfh()->SendBeginNavigationWithURL(kUrl2);
   NavigationRequest* request2 = GetNavigationRequestForFrameTreeNode(node);
   ASSERT_TRUE(request2);
   EXPECT_EQ(kUrl2, request2->common_params().url);
 
   // Confirm that the first loader got destroyed.
   EXPECT_FALSE(loader1);
 
-  // Confirm that the commit corresponds to the new request.
+  // Confirm that a new speculative RFH was created.
+  speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  ASSERT_TRUE(speculative_rfh);
+  int32 site_instance_id_2 = speculative_rfh->GetSiteInstance()->GetId();
+  EXPECT_NE(site_instance_id_1, site_instance_id_2);
+
+  // Commit.
   scoped_refptr<ResourceResponse> response(new ResourceResponse);
   GetLoaderForNavigationRequest(request2)->CallOnResponseStarted(
       response, MakeEmptyStream());
-  RenderFrameHostImpl* pending_rfh =
-      node->render_manager()->pending_frame_host();
-  ASSERT_TRUE(pending_rfh);
-  EXPECT_EQ(kUrl2_site, pending_rfh->GetSiteInstance()->GetSiteURL());
+
+  // And commit provisional load.
+  contents()->CommitPendingNavigation();

[clamy, 2014/12/30 16:57:45]

Same as above, please use SendNavigate here.

[carlosk, 2014/12/31 13:39:18]

Done.

+
+  // Confirm that the commit corresponds to the new request.
+  ASSERT_TRUE(main_test_rfh());
+  EXPECT_EQ(kUrl2_site, main_test_rfh()->GetSiteInstance()->GetSiteURL());
+
+  // Confirm that the committed RFH is the new speculative one.
+  EXPECT_EQ(site_instance_id_2, main_test_rfh()->GetSiteInstance()->GetId());
 }
 
 // PlzNavigate: Test that a reload navigation is properly signaled to the
-// renderer when the navigation can commit.
+// renderer when the navigation can commit. Speculative renderers 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();
   SendRequestNavigationWithParameters(
       node, kUrl, Referrer(), ui::PAGE_TRANSITION_LINK,
       NavigationController::RELOAD);
-  contents()->GetMainFrame()->SendBeginNavigationWithURL(kUrl);
+  main_test_rfh()->SendBeginNavigationWithURL(kUrl);
   // 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);
-  int page_id = contents()->GetMaxPageIDForSiteInstance(
-                    main_test_rfh()->GetSiteInstance()) + 1;
-  main_test_rfh()->SendNavigate(page_id, kUrl);
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
+
+  main_test_rfh()->SendNavigate(0, kUrl);
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
 
   // Now do a shift+reload.
   SendRequestNavigationWithParameters(
       node, kUrl, Referrer(), ui::PAGE_TRANSITION_LINK,
       NavigationController::RELOAD_IGNORING_CACHE);
-  contents()->GetMainFrame()->SendBeginNavigationWithURL(kUrl);
+  main_test_rfh()->SendBeginNavigationWithURL(kUrl);
   // 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);
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
+}
+
+// PlzNavigate: Confirm that a speculative RenderFrameHost is used when
+// navigating from one site to the another.
+TEST_F(NavigatorTestWithBrowserSideNavigation,
+       SpeculativeRendererWorksBaseCase) {
+  // Navigate to an initial site.
+  const GURL kUrlInit("http://wikipedia.org/");
+  contents()->NavigateAndCommit(kUrlInit);
+  FrameTreeNode* node = main_test_rfh()->frame_tree_node();
+
+  // Begin navigating to another site.
+  const GURL kUrl("http://google.com/");
+  contents()->GetController().LoadURL(kUrl, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
+  main_test_rfh()->SendBeginNavigationWithURL(kUrl);
+  TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  ASSERT_TRUE(speculative_rfh);
+  EXPECT_NE(speculative_rfh, main_test_rfh());
+  EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrl),
+            speculative_rfh->GetSiteInstance()->GetSiteURL());
+  EXPECT_FALSE(node->render_manager()->pending_frame_host());
+  int32 site_instance_id = speculative_rfh->GetSiteInstance()->GetId();
+
+  // Ask Navigator to commit the navigation by simulating a call to
+  // OnResponseStarted.
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  GetLoaderForNavigationRequest(GetNavigationRequestForFrameTreeNode(node))
+      ->CallOnResponseStarted(response, MakeEmptyStream());
+  speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  ASSERT_TRUE(speculative_rfh);
+  EXPECT_EQ(site_instance_id, speculative_rfh->GetSiteInstance()->GetId());
+  EXPECT_FALSE(node->render_manager()->pending_frame_host());
+
+  // Invoke OnDidCommitProvisionalLoad.
+  contents()->CommitPendingNavigation();

[clamy, 2014/12/30 16:57:45]

Same as above, please use SendNavigate.

[carlosk, 2014/12/31 13:39:19]

Done.

+  EXPECT_EQ(site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
+  EXPECT_FALSE(node->render_manager()->pending_frame_host());
+}
+
+// PlzNavigate: Confirm that a speculative RenderFrameHost is thrown away when
+// the final URL's site differs from the initial one due to redirects.
+TEST_F(NavigatorTestWithBrowserSideNavigation,
+       SpeculativeRendererDiscardedAfterRedirectToAnotherSite) {
+  // Navigate to an initial site.
+  const GURL kUrlInit("http://wikipedia.org/");
+  contents()->NavigateAndCommit(kUrlInit);
+  FrameTreeNode* node = main_test_rfh()->frame_tree_node();
+  int32 init_site_instance_id = main_test_rfh()->GetSiteInstance()->GetId();
+
+  // Begin navigating to another site.
+  const GURL kUrl("http://google.com/");
+  contents()->GetController().LoadURL(kUrl, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
+  main_test_rfh()->SendBeginNavigationWithURL(kUrl);
+  TestRenderFrameHost* speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  int32 site_instance_id = speculative_rfh->GetSiteInstance()->GetId();
+  EXPECT_EQ(init_site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());
+  ASSERT_TRUE(speculative_rfh);
+  EXPECT_NE(speculative_rfh, main_test_rfh());
+  EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrl),
+            speculative_rfh->GetSiteInstance()->GetSiteURL());
+
+  // It then redirects to yet another site.
+  NavigationRequest* main_request = GetNavigationRequestForFrameTreeNode(node);
+  ASSERT_TRUE(main_request);
+  const GURL kUrlRedirect("https://www.google.com/");
+  net::RedirectInfo redirect_info;
+  redirect_info.status_code = 302;
+  redirect_info.new_method = "GET";
+  redirect_info.new_url = kUrlRedirect;
+  redirect_info.new_first_party_for_cookies = kUrlRedirect;
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  GetLoaderForNavigationRequest(main_request)
+      ->CallOnRequestRedirected(redirect_info, response);
+  EXPECT_EQ(init_site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());
+  speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  ASSERT_TRUE(speculative_rfh);
+
+  // TODO(carlosk): once the speculative RenderFrameHost updates with redirects
+  // this next check will be changed to verify that it actually happens.
+  EXPECT_EQ(site_instance_id, speculative_rfh->GetSiteInstance()->GetId());
+
+  // Commit the navigation with Navigator by simulating the call to
+  // OnResponseStarted.
+  response = new ResourceResponse;
+  GetLoaderForNavigationRequest(main_request)
+      ->CallOnResponseStarted(response, MakeEmptyStream());
+  EXPECT_EQ(init_site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());
+
+  // Once commit happens the speculative RenderFrameHost is already updated to
+  // match the known final SiteInstance.
+  speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  ASSERT_TRUE(speculative_rfh);
+  EXPECT_EQ(SiteInstanceImpl::GetSiteForURL(browser_context(), kUrlRedirect),
+            speculative_rfh->GetSiteInstance()->GetSiteURL());
+  int32 redirect_site_instance_id = speculative_rfh->GetSiteInstance()->GetId();
+
+  // Invoke OnDidCommitProvisionalLoad.
+  contents()->CommitPendingNavigation();

[clamy, 2014/12/30 16:57:46]

Please us SendNavigate here.

[carlosk, 2014/12/31 13:39:18]

Done.

+
+  // And finally on commit-provisional-load the already created RenderFrameHost
+  // is made active.

[clamy, 2014/12/30 16:57:45]

Maybe rephrase this as Finally, when the navigation has committed in the
speculative RenderFrameHost, it is swapped in.

[carlosk, 2014/12/31 13:39:18]

This doesn't sound very accurate either as the RFH did commit in the previous
step (before SendNavigate). So I made it:

Checks that the speculative RenderFrameHost was swapped in.

[clamy, 2014/12/31 14:07:09]

It depends on how you define commit. Currently, it means the renderer requested
the main resource, has begun receiving its body and informed the browser
(through the DidCommitProvisionalLoad IPC). Therefore the function called
CommitNavigation in NavigatorImpl is meant as find a renderer that will commit
the navigation and have it commit it, and not this is the commit step. Hence why
the test helper function that does those steps is called PrepareForCommit, the
actual commit being done by SendNavigate.

nit: s/Checks/Check - other than that I'm fine with the comment.

[carlosk, 2014/12/31 15:39:51]

Done.

+  EXPECT_EQ(redirect_site_instance_id,
+            main_test_rfh()->GetSiteInstance()->GetId());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
+}
+
+// PlzNavigate: Verify that a previously swapped-out RenderFrameHost is

[clamy, 2014/12/30 16:57:45]

nit: s/swapped-out/swapped out

[carlosk, 2014/12/31 13:39:19]

Done.

+// correctly reused when spawning a speculative RenderFrameHost in a navigation
+// using the same SiteInstance.
+TEST_F(NavigatorTestWithBrowserSideNavigation,
+       SpeculativeRendererReuseSwappedOutRFH) {
+  // Navigate to an initial site.
+  const GURL kUrl1("http://wikipedia.org/");
+  contents()->NavigateAndCommit(kUrl1);
+  RenderFrameHostImpl* rfh1 = main_test_rfh();
+  FrameTreeNode* node = rfh1->frame_tree_node();
+  RenderFrameHostManager* rfhm = node->render_manager();
+
+  // Increment active frame count to cause the RenderFrameHost to be swapped out
+  // (instead of immediately destroyed).
+  rfh1->GetSiteInstance()->increment_active_frame_count();
+
+  // Navigate to another site to swap-out the initial RenderFrameHost.
+  const GURL kUrl2("http://chromium.org/");
+  contents()->NavigateAndCommit(kUrl2);
+  ASSERT_NE(rfh1, main_test_rfh());
+  EXPECT_NE(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
+  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, main_test_rfh()->rfh_state());
+  EXPECT_TRUE(rfhm->IsOnSwappedOutList(rfh1));
+
+  // Now go back to the initial site so the swapped-out RenderFrameHost should
+  // be reused.
+  contents()->GetController().LoadURL(kUrl1, Referrer(),
+                                      ui::PAGE_TRANSITION_LINK, std::string());
+  main_test_rfh()->SendBeginNavigationWithURL(kUrl1);
+  EXPECT_EQ(rfh1, GetSpeculativeRenderFrameHost(node));
+  EXPECT_NE(RenderFrameHostImpl::STATE_DEFAULT,
+            GetSpeculativeRenderFrameHost(node)->rfh_state());
+
+  scoped_refptr<ResourceResponse> response(new ResourceResponse);
+  GetLoaderForNavigationRequest(GetNavigationRequestForFrameTreeNode(node))
+      ->CallOnResponseStarted(response, MakeEmptyStream());
+  EXPECT_EQ(rfh1, GetSpeculativeRenderFrameHost(node));
+  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT,
+            GetSpeculativeRenderFrameHost(node)->rfh_state());
+
+  contents()->CommitPendingNavigation();

[clamy, 2014/12/30 16:57:45]

Please use SendNavigate.

[carlosk, 2014/12/31 13:39:18]

Done.

+  EXPECT_EQ(rfh1, main_test_rfh());
+  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
+  EXPECT_FALSE(rfhm->IsOnSwappedOutList(rfh1));
 }
 
 }  // namespace content