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: caller must invoke ClearMessages on the sink at some point before
+  // the tracked commit happens to clear up commit messages from previous
+  // navigations.
+  bool DidRenderFrameHostCommit(RenderFrameHostImpl* rfh) {
+    MockRenderProcessHost* rph =
+        static_cast<MockRenderProcessHost*>(rfh->GetProcess());
+    const FrameMsg_CommitNavigation* commit_message =
+        static_cast<const FrameMsg_CommitNavigation*>(
+            rph->sink().GetUniqueMessageMatching(
+                FrameMsg_CommitNavigation::ID));
+    return commit_message &&
+           rfh->GetRoutingID() == commit_message->routing_id();

[Charlie Reis, 2015/01/12 23:30:51]

nit: Wrong indent (should be 4 spaces).  I'll defer to clang-format if it
disagrees.

[carlosk, 2015/01/13 15:54:12]

It disagrees. :)

I've been trying to run it always before uploading a new patch set. I have the
impression this behavior changed recently where it gives precedence to column
align expressions.

[Charlie Reis, 2015/01/13 18:48:21]

Acknowledged.

+  }
 };
 
+// PlzNavigate: Test final state after a complete navigation (to avoid repeating
+// these checks in other tests).
+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),

[Charlie Reis, 2015/01/12 23:30:52]

This is a good idea to test, but it's equally important to test the actual URL
that commits.  Please add a check that contents()->GetLastCommittedURL() is kUrl
as well.

[carlosk, 2015/01/13 15:54:12]

Done.

I wanted to be sure with this expectation the SI is the correct one as it is the
most important piece in regards to the specific renderer process being used.

+            main_test_rfh()->GetSiteInstance()->GetSiteURL());
+
+  // After a navigation is finished no speculative RenderFrameHost should
+  // exist.
+  FrameTreeNode* node = main_test_rfh()->frame_tree_node();
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
+
+  // 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);
 
   // 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,
+  FrameTreeNode* root_node = contents()->GetFrameTree()->root();
+  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);
+  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 never create a speculative RenderFrameHost,
+  // unless site-per-process is enabled. In that case as the subframe navigation

[Charlie Reis, 2015/01/12 23:30:51]

nit: Comma after "In that case"

[carlosk, 2015/01/13 15:54:12]

Done.

+  // is to a different site and is still ongoing, it should have one.
+  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 navigation to a different site should use a speculative
+  // RenderFrameHost.
+  EXPECT_TRUE(GetSpeculativeRenderFrameHost(root_node));
+
+  // As the main frame hasn't yet committed the subframe still exists. So the

[Charlie Reis, 2015/01/12 23:30:52]

nit: The second sentence is a sentence fragment.
s/So/Thus,/

[carlosk, 2015/01/13 15:54:12]

Done.

+  // above situation regarding subframe navigations is valid here.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          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

[Charlie Reis, 2015/01/12 23:30:52]

English nit: "Since" or "Because" would be easier to read here than "As."  "As
we're..." sounds like the two things are happening at the same time, rather than
one because of the other.  (Same in the cases above, and apologies for English
being such a terribly confusing language!)

[carlosk, 2015/01/13 15:54:12]

Done. You didn't invent it (I assume) so no reason to be sorry. :)

+  // 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.
+  process()->sink().ClearMessages();
   SendRequestNavigation(node, kUrl2);
   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(DidRenderFrameHostCommit(main_test_rfh()));
   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.
+  process()->sink().ClearMessages();
   SendRequestNavigation(node, kUrl2);
   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(DidRenderFrameHostCommit(main_test_rfh()));
   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());
+  RenderFrameHostImpl* initial_rfh = main_test_rfh();
   FrameTreeNode* node = main_test_rfh()->frame_tree_node();
 
   // Navigate to a different site.
+  process()->sink().ClearMessages();
   SendRequestNavigation(node, kUrl2);
   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);
+  EXPECT_TRUE(DidRenderFrameHostCommit(speculative_rfh));
+  EXPECT_FALSE(DidRenderFrameHostCommit(main_test_rfh()));

[Charlie Reis, 2015/01/12 23:30:52]

This is very confusing for those of us familiar with the current navigation
flow.  Did...Commit sounds like it refers to the time after the renderer process
has committed (i.e., DidCommitProvisionalLoad).

That would make this impossible-- if the speculative RFH commits, it would
become main_test_rfh(), and thus this line couldn't pass.  I'm guessing it means
something else (i.e., the RFH has *requested* that the renderer process commit
the navigation, but it hasn't happened yet).

If that's the case, let's rename the method to DidRenderFrameHostRequestCommit.

[carlosk, 2015/01/13 15:54:12]

Agreed and done.

I was confused because the method that causes the IPC to be sent is called
RenderFrameHostImpl::CommitNavigation.

+
+  speculative_rfh->SendNavigate(0, kUrl2);
+
+  RenderFrameHostImpl* final_rfh = main_test_rfh();
+  EXPECT_EQ(speculative_rfh, 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.
+  process()->sink().ClearMessages();
   SendRequestNavigation(node, kUrl1);
   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.

[Charlie Reis, 2015/01/12 23:30:52]

This adds to my confusion.  The navigation has not committed here.  Can you
update this comment while you're here?

[carlosk, 2015/01/13 15:54:12]

Changed to: Request the RenderFrameHost to commit. Also made similar changes to
other comments.

   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* final_speculative_rfh =
+      GetSpeculativeRenderFrameHost(node);
+  EXPECT_TRUE(final_speculative_rfh);
+  EXPECT_TRUE(DidRenderFrameHostCommit(final_speculative_rfh));
+
+  // And commits provisional load.
+  final_speculative_rfh->SendNavigate(0, kUrl2);
+  RenderFrameHostImpl* final_rfh = main_test_rfh();
+  ASSERT_TRUE(final_rfh);
+  EXPECT_NE(rfh, final_rfh);
+  EXPECT_EQ(final_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.
+  process()->sink().ClearMessages();
   SendRequestNavigation(node, kUrl1);
   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);
   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());
+  EXPECT_TRUE(DidRenderFrameHostCommit(speculative_rfh));
+  EXPECT_FALSE(DidRenderFrameHostCommit(main_test_rfh()));
+
+  // And commit provisional load.
+  speculative_rfh->SendNavigate(0, kUrl2);
+
+  // 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.

[Charlie Reis, 2015/01/12 23:30:52]

nit: Drop "the"

[carlosk, 2015/01/13 15:54:12]

Done.

+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/");
+  process()->sink().ClearMessages();
+  SendRequestNavigation(node, kUrl);
+  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_TRUE(DidRenderFrameHostCommit(speculative_rfh));
+  EXPECT_EQ(site_instance_id, speculative_rfh->GetSiteInstance()->GetId());
+  EXPECT_FALSE(node->render_manager()->pending_frame_host());
+
+  // Invoke OnDidCommitProvisionalLoad.
+  speculative_rfh->SendNavigate(0, kUrl);
+  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/");
+  process()->sink().ClearMessages();
+  SendRequestNavigation(node, kUrl);
+  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());

[Charlie Reis, 2015/01/12 23:30:52]

Please verify that init_site_instance_id and site_instance_id are not the same.

[carlosk, 2015/01/13 15:54:12]

Done. Good one.

Also added a couple of these below for the next site id.

+  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

[Charlie Reis, 2015/01/12 23:30:52]

I agree with the TODO, but it's hard to follow what the test is doing in the
meantime.  Above the TODO, please add "For now, ensure that the speculative RFH
does not change after the redirect."

[carlosk, 2015/01/13 15:54:12]

Done.

+  // 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());
+  speculative_rfh = GetSpeculativeRenderFrameHost(node);
+  EXPECT_TRUE(DidRenderFrameHostCommit(speculative_rfh));
+  EXPECT_EQ(init_site_instance_id, main_test_rfh()->GetSiteInstance()->GetId());
+
+  // Once commit happens the speculative RenderFrameHost is already updated to

[Charlie Reis, 2015/01/12 23:30:52]

Drop "already," since this seems to be the point in time when the speculative
RFH is updated.

[carlosk, 2015/01/13 15:54:12]

Done.

+  // match the known final SiteInstance.
+  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.
+  speculative_rfh->SendNavigate(0, kUrlRedirect);
+
+  // Check that the speculative RenderFrameHost was swapped in.
+  EXPECT_EQ(redirect_site_instance_id,
+            main_test_rfh()->GetSiteInstance()->GetId());
+  EXPECT_FALSE(GetSpeculativeRenderFrameHost(node));
+}
+
+// PlzNavigate: Verify that a previously swapped out RenderFrameHost is
+// 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);
+  TestRenderFrameHost* 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 that the swapped out RenderFrameHost
+  // should be reused.
+  process()->sink().ClearMessages();
+  static_cast<MockRenderProcessHost*>(rfh1->GetProcess())
+      ->sink()
+      .ClearMessages();
+  SendRequestNavigation(node, kUrl1);
+  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());
+  EXPECT_TRUE(DidRenderFrameHostCommit(rfh1));
+  EXPECT_FALSE(DidRenderFrameHostCommit(main_test_rfh()));
+
+  rfh1->SendNavigate(1, kUrl1);
+  EXPECT_EQ(rfh1, main_test_rfh());
+  EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
+  EXPECT_FALSE(rfhm->IsOnSwappedOutList(rfh1));
 }
 
 }  // namespace content