PlzNavigate: track pending commits

content/browser/frame_host/render_frame_host_manager_unittest.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 "base/command_line.h"
 #include "base/files/file_path.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/histogram_tester.h"
 #include "base/time/time.h"
 #include "content/browser/frame_host/cross_site_transferring_request.h"
@@ skipping 334 matching lines @@
     const GURL kDestUrl("http://www.google.com/");
 
     // Navigate our first tab to a chrome url and then to the destination.
     NavigateActiveAndCommit(kChromeURL);
     TestRenderFrameHost* ntp_rfh = contents()->GetMainFrame();
 
     // Navigate to a cross-site URL.
     contents()->GetController().LoadURL(
         kDestUrl, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
     contents()->GetMainFrame()->PrepareForCommit();
-    EXPECT_TRUE(contents()->cross_navigation_pending());
+    EXPECT_TRUE(contents()->CrossNavigationPending());
 
     // Manually increase the number of active frames in the
     // SiteInstance that ntp_rfh belongs to, to prevent it from being
     // destroyed when it gets swapped out.
     ntp_rfh->GetSiteInstance()->increment_active_frame_count();
 
     TestRenderFrameHost* dest_rfh = contents()->GetPendingMainFrame();
     CHECK(dest_rfh);
     EXPECT_NE(ntp_rfh, dest_rfh);
 
@@ skipping 12 matching lines @@
   RenderFrameHostImpl* GetFrameHostForNavigation(
       RenderFrameHostManager* manager,
       const NavigationEntryImpl& entry) {
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
         switches::kEnableBrowserSideNavigation)) {
       scoped_ptr<NavigationRequest> navigation_request =
           NavigationRequest::CreateBrowserInitiated(
               manager->frame_tree_node_, entry, FrameMsg_Navigate_Type::NORMAL,
               base::TimeTicks::Now(),
               static_cast<NavigationControllerImpl*>(&controller()));
-      return manager->GetFrameHostForNavigation(*navigation_request);
+      TestRenderFrameHost* frame_host = static_cast<TestRenderFrameHost*>(
+          manager->GetFrameHostForNavigation(*navigation_request));
+      CHECK(frame_host);
+      frame_host->set_pending_commit(true);
+      return frame_host;
     }
     return manager->Navigate(entry);
   }
 
   // Returns the pending RenderFrameHost.
   // PlzNavigate: returns the speculative RenderFrameHost.
   RenderFrameHostImpl* GetPendingFrameHost(
       RenderFrameHostManager* manager) {
     if (base::CommandLine::ForCurrentProcess()->HasSwitch(
         switches::kEnableBrowserSideNavigation)) {
@@ skipping 45 matching lines @@
   scoped_ptr<TestWebContents> contents2(
       TestWebContents::Create(browser_context(), NULL));
 
   // Load the two URLs in the second tab. Note that the first navigation creates
   // a RFH that's not pending (since there is no cross-site transition), so
   // we use the committed one.
   contents2->GetController().LoadURL(
       kChromeUrl, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   contents2->GetMainFrame()->PrepareForCommit();
   TestRenderFrameHost* ntp_rfh2 = contents2->GetMainFrame();
-  EXPECT_FALSE(contents2->cross_navigation_pending());
+  EXPECT_FALSE(contents2->CrossNavigationPending());
   ntp_rfh2->SendNavigate(100, kChromeUrl);
 
   // The second one is the opposite, creating a cross-site transition and
   // requiring a beforeunload ack.
   contents2->GetController().LoadURL(
       kDestUrl, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   contents2->GetMainFrame()->PrepareForCommit();
-  EXPECT_TRUE(contents2->cross_navigation_pending());
+  EXPECT_TRUE(contents2->CrossNavigationPending());
   TestRenderFrameHost* dest_rfh2 = contents2->GetPendingMainFrame();
   ASSERT_TRUE(dest_rfh2);
 
   dest_rfh2->SendNavigate(101, kDestUrl);
 
   // The two RFH's should be different in every way.
   EXPECT_NE(contents()->GetMainFrame()->GetProcess(), dest_rfh2->GetProcess());
   EXPECT_NE(contents()->GetMainFrame()->GetSiteInstance(),
             dest_rfh2->GetSiteInstance());
   EXPECT_FALSE(dest_rfh2->GetSiteInstance()->IsRelatedSiteInstance(
@@ skipping 1085 matching lines @@
   RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting();
 
   RenderFrameHostImpl* host = NULL;
 
   // 1) The first navigation. --------------------------
   const GURL kUrl1("http://www.google.com/");
   NavigationEntryImpl entry1(
       NULL /* instance */, -1 /* page_id */, kUrl1, Referrer(),
       base::string16() /* title */, ui::PAGE_TRANSITION_TYPED,
       false /* is_renderer_init */);
-  host = manager->Navigate(entry1);
+  host = GetFrameHostForNavigation(manager, entry1);

[nasko, 2015/04/15 21:12:45]

It seems a bit strange to replace a "Navigate" call with a "GetRFH" call. I
think it is fine in this case, but want to get Charlie's opinion on it as well.

[Charlie Reis, 2015/04/15 21:54:17]

I agree that the name of the helper function feels wrong.  (I'd go further and
say it doesn't seem right for this case.)

The helper function is actually calling Navigate in the non-PlzNavigate case. 
Can we find a name that conveys this and works for PlzNavigate as well?

[clamy, 2015/04/16 13:18:36]

I've renamed the function NavigateToEntry since it is what we are trying to
simulate there.

 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_TRUE(host == manager->current_frame_host());
   EXPECT_FALSE(manager->pending_frame_host());
   EXPECT_EQ(manager->current_frame_host()->GetSiteInstance(), instance);
 
   // Commit.
   manager->DidNavigateFrame(host, true);
   // Commit to SiteInstance should be delayed until RenderFrame commit.
   EXPECT_EQ(host, manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
 
   // 2) Navigate to a different domain. -------------------------
   // Guests stay in the same process on navigation.
   const GURL kUrl2("http://www.chromium.org");
   NavigationEntryImpl entry2(
       NULL /* instance */, -1 /* page_id */, kUrl2,
       Referrer(kUrl1, blink::WebReferrerPolicyDefault),
       base::string16() /* title */, ui::PAGE_TRANSITION_LINK,
       true /* is_renderer_init */);
-  host = manager->Navigate(entry2);
+  host = GetFrameHostForNavigation(manager, entry2);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(manager->pending_frame_host());
 
   // Commit.
   manager->DidNavigateFrame(host, true);
   EXPECT_EQ(host, manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_EQ(host->GetSiteInstance(), instance);
@@ skipping 14 matching lines @@
                           Source<WebContents>(web_contents.get()));
 
   RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting();
 
   // 1) The first navigation. --------------------------
   const GURL kUrl1("http://www.google.com/");
   NavigationEntryImpl entry1(NULL /* instance */, -1 /* page_id */, kUrl1,
                              Referrer(), base::string16() /* title */,
                              ui::PAGE_TRANSITION_TYPED,
                              false /* is_renderer_init */);
-  RenderFrameHostImpl* host = manager->Navigate(entry1);
+  RenderFrameHostImpl* host = GetFrameHostForNavigation(manager, entry1);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_EQ(host, manager->current_frame_host());
-  EXPECT_FALSE(manager->pending_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
 
   // We should observe a notification.
   EXPECT_TRUE(
       notifications.Check1AndReset(NOTIFICATION_RENDER_VIEW_HOST_CHANGED));
   notifications.Reset();
 
   // Commit.
   manager->DidNavigateFrame(host, true);
 
   // Commit to SiteInstance should be delayed until RenderFrame commits.
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(host->GetSiteInstance()->HasSite());
   host->GetSiteInstance()->SetSite(kUrl1);
 
   // 2) Cross-site navigate to next site. -------------------------
   const GURL kUrl2("http://www.example.com");
   NavigationEntryImpl entry2(
       NULL /* instance */, -1 /* page_id */, kUrl2, Referrer(),
       base::string16() /* title */, ui::PAGE_TRANSITION_TYPED,
       false /* is_renderer_init */);
-  RenderFrameHostImpl* host2 = manager->Navigate(entry2);
+  RenderFrameHostImpl* host2 = GetFrameHostForNavigation(manager, entry2);
 
   // A new RenderFrameHost should be created.
-  ASSERT_EQ(host2, manager->pending_frame_host());
+  ASSERT_EQ(host2, GetPendingFrameHost(manager));
   EXPECT_NE(host2, host);
 
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(manager->current_frame_host()->is_swapped_out());
-  EXPECT_EQ(host2, manager->pending_frame_host());
+  EXPECT_EQ(host2, GetPendingFrameHost(manager));
 
   // 3) Close the tab. -------------------------
   notifications.ListenFor(NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
                           Source<RenderWidgetHost>(host2->render_view_host()));
   manager->OnBeforeUnloadACK(false, true, base::TimeTicks());
 
   EXPECT_TRUE(
       notifications.Check1AndReset(NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED));
-  EXPECT_FALSE(manager->pending_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
   EXPECT_EQ(host, manager->current_frame_host());
 }
 
 TEST_F(RenderFrameHostManagerTest, CloseWithPendingWhileUnresponsive) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
 
   CloseWebContentsDelegate close_delegate;
   contents()->SetDelegate(&close_delegate);
 
   // Navigate to the first page.
   contents()->NavigateAndCommit(kUrl1);
   TestRenderFrameHost* rfh1 = contents()->GetMainFrame();
 
   // Start to close the tab, but assume it's unresponsive.
   rfh1->render_view_host()->ClosePage();
   EXPECT_TRUE(rfh1->render_view_host()->is_waiting_for_close_ack());
 
   // Start a navigation to a new site.
   controller().LoadURL(
       kUrl2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   if (base::CommandLine::ForCurrentProcess()->HasSwitch(
         switches::kEnableBrowserSideNavigation)) {
     rfh1->PrepareForCommit();
   }
-  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_TRUE(contents()->CrossNavigationPending());
 
   // Simulate the unresponsiveness timer.  The tab should close.
   contents()->RendererUnresponsive(rfh1->render_view_host());
   EXPECT_TRUE(close_delegate.is_closed());
 }
 
 // Tests that the RenderFrameHost is properly deleted when the SwapOutACK is
 // received.  (SwapOut and the corresponding ACK always occur after commit.)
 // Also tests that an early SwapOutACK is properly ignored.
 TEST_F(RenderFrameHostManagerTest, DeleteFrameAfterSwapOutACK) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
 
   // Navigate to the first page.
   contents()->NavigateAndCommit(kUrl1);
   TestRenderFrameHost* rfh1 = contents()->GetMainFrame();
   RenderFrameHostDeletedObserver rfh_deleted_observer(rfh1);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
 
   // Navigate to new site, simulating onbeforeunload approval.
   controller().LoadURL(
       kUrl2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   contents()->GetMainFrame()->PrepareForCommit();
-  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_TRUE(contents()->CrossNavigationPending());
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
   TestRenderFrameHost* rfh2 = contents()->GetPendingMainFrame();
 
   // Simulate the swap out ack, unexpectedly early (before commit).  It should
   // have no effect.
   rfh1->OnSwappedOut();
-  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_TRUE(contents()->CrossNavigationPending());
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
 
   // The new page commits.
   contents()->TestDidNavigate(rfh2, 1, kUrl2, ui::PAGE_TRANSITION_TYPED);
-  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_FALSE(contents()->CrossNavigationPending());
   EXPECT_EQ(rfh2, contents()->GetMainFrame());
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state());
   EXPECT_TRUE(
       rfh1->frame_tree_node()->render_manager()->IsPendingDeletion(rfh1));
 
   // Simulate the swap out ack.
   rfh1->OnSwappedOut();
 
@@ skipping 15 matching lines @@
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
 
   // Increment the number of active frames in SiteInstanceImpl so that rfh1 is
   // not deleted on swap out.
   rfh1->GetSiteInstance()->increment_active_frame_count();
 
   // Navigate to new site, simulating onbeforeunload approval.
   controller().LoadURL(
       kUrl2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   contents()->GetMainFrame()->PrepareForCommit();
-  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_TRUE(contents()->CrossNavigationPending());
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
   TestRenderFrameHost* rfh2 = contents()->GetPendingMainFrame();
 
   // The new page commits.
   contents()->TestDidNavigate(rfh2, 1, kUrl2, ui::PAGE_TRANSITION_TYPED);
-  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_FALSE(contents()->CrossNavigationPending());
   EXPECT_EQ(rfh2, contents()->GetMainFrame());
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state());
 
   // Simulate the swap out ack.
   rfh1->OnSwappedOut();
 
   // rfh1 should be swapped out.
   EXPECT_FALSE(rfh_deleted_observer.deleted());
@@ skipping 16 matching lines @@
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
 
   // Increment the number of active frames in SiteInstanceImpl so that rfh1 is
   // not deleted on swap out.
   rfh1->GetSiteInstance()->increment_active_frame_count();
 
   // Navigate to new site, simulating onbeforeunload approval.
   controller().LoadURL(
       kUrl2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   rfh1->PrepareForCommit();
-  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_TRUE(contents()->CrossNavigationPending());
   TestRenderFrameHost* rfh2 = contents()->GetPendingMainFrame();
 
   // The new page commits.
   contents()->TestDidNavigate(rfh2, 1, kUrl2, ui::PAGE_TRANSITION_TYPED);
-  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_FALSE(contents()->CrossNavigationPending());
   EXPECT_EQ(rfh2, contents()->GetMainFrame());
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state());
 
   // Simulate the swap out ack.
   rfh1->OnSwappedOut();
 
   // rfh1 should be swapped out.
   EXPECT_FALSE(rfh_deleted_observer.deleted());
@@ skipping 18 matching lines @@
   controller().LoadURL(
       kUrl2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   {
     pending_rfh = contents()->GetFrameTree()->root()->render_manager()
         ->pending_frame_host();
     RenderFrameHostDeletedObserver rfh_deleted_observer(pending_rfh);
 
     // Cancel the navigation by simulating a declined beforeunload dialog.
     contents()->GetMainFrame()->OnMessageReceived(
         FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
-    EXPECT_FALSE(contents()->cross_navigation_pending());
+    EXPECT_FALSE(contents()->CrossNavigationPending());
 
     // Since the pending RFH is the only one for the new SiteInstance, it should
     // be deleted.
     EXPECT_TRUE(rfh_deleted_observer.deleted());
   }
 
   // Start another cross-site navigation.
   controller().LoadURL(
       kUrl2, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
   {
     pending_rfh = contents()->GetFrameTree()->root()->render_manager()
         ->pending_frame_host();
     RenderFrameHostDeletedObserver rfh_deleted_observer(pending_rfh);
 
     // Increment the number of active frames in the new SiteInstance, which will
     // cause the pending RFH to be swapped out instead of deleted.
     pending_rfh->GetSiteInstance()->increment_active_frame_count();
 
     contents()->GetMainFrame()->OnMessageReceived(
         FrameHostMsg_BeforeUnload_ACK(0, false, now, now));
-    EXPECT_FALSE(contents()->cross_navigation_pending());
+    EXPECT_FALSE(contents()->CrossNavigationPending());
     EXPECT_FALSE(rfh_deleted_observer.deleted());
   }
 }
 
 // Test that a pending RenderFrameHost in a non-root frame tree node is properly
 // deleted when the node is detached. Motivated by http://crbug.com/441357 and
 // http://crbug.com/444955.
 TEST_F(RenderFrameHostManagerTest, DetachPendingChild) {
   base::CommandLine::ForCurrentProcess()->AppendSwitch(
       switches::kSitePerProcess);
@@ skipping 50 matching lines @@
   EXPECT_EQ(host2, GetPendingFrameHost(iframe2));
   EXPECT_TRUE(RenderFrameHostImpl::IsRFHStateActive(
       GetPendingFrameHost(iframe1)->rfh_state()));
   EXPECT_TRUE(RenderFrameHostImpl::IsRFHStateActive(
       GetPendingFrameHost(iframe2)->rfh_state()));
   EXPECT_NE(GetPendingFrameHost(iframe1), GetPendingFrameHost(iframe2));
   EXPECT_EQ(GetPendingFrameHost(iframe1)->GetSiteInstance(),
             GetPendingFrameHost(iframe2)->GetSiteInstance());
   EXPECT_NE(iframe1->current_frame_host(), GetPendingFrameHost(iframe1));
   EXPECT_NE(iframe2->current_frame_host(), GetPendingFrameHost(iframe2));
-  EXPECT_FALSE(contents()->cross_navigation_pending())
-      << "There should be no top-level pending navigation.";
+  EXPECT_FALSE(contents()->CrossNavigationPending());

[nasko, 2015/04/15 21:12:45]

Why did we lose the string here?

[clamy, 2015/04/16 13:18:35]

Done.

 
   RenderFrameHostDeletedObserver delete_watcher1(GetPendingFrameHost(iframe1));
   RenderFrameHostDeletedObserver delete_watcher2(GetPendingFrameHost(iframe2));
   EXPECT_FALSE(delete_watcher1.deleted());
   EXPECT_FALSE(delete_watcher2.deleted());
 
   // Keep the SiteInstance alive for testing.
   scoped_refptr<SiteInstanceImpl> site_instance =
       GetPendingFrameHost(iframe1)->GetSiteInstance();
   EXPECT_TRUE(site_instance->HasSite());
@@ skipping 97 matching lines @@
   EXPECT_FALSE(contents2->GetMainFrame()->IsRenderFrameLive());
   contents2->NavigateAndCommit(kUrl3);
   EXPECT_TRUE(contents2->GetMainFrame()->IsRenderFrameLive());
   EXPECT_NE(nullptr,
             iframe->GetRenderFrameProxyHost(contents1->GetSiteInstance()));
   EXPECT_EQ(nullptr,
             iframe->GetRenderFrameProxyHost(contents2->GetSiteInstance()));
 }
 
 }  // namespace content