PlzNavigate: track pending commits

content/browser/frame_host/render_frame_host_manager_unittest.cc

Index: content/browser/frame_host/render_frame_host_manager_unittest.cc
diff --git a/content/browser/frame_host/render_frame_host_manager_unittest.cc b/content/browser/frame_host/render_frame_host_manager_unittest.cc
index d1c0c74b9825a0f8bdf0f85d20d0a0aab395f0f4..4fe3e275c97466dd4210ad2a431e30b711281b89 100644
--- a/content/browser/frame_host/render_frame_host_manager_unittest.cc
+++ b/content/browser/frame_host/render_frame_host_manager_unittest.cc
@@ -352,7 +352,7 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
     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
@@ -385,7 +385,11 @@ class RenderFrameHostManagerTest : public RenderViewHostImplTestHarness {
               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);
   }
@@ -451,7 +455,7 @@ TEST_F(RenderFrameHostManagerTest, NewTabPageProcesses) {
       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
@@ -459,7 +463,7 @@ TEST_F(RenderFrameHostManagerTest, NewTabPageProcesses) {
   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);
 
@@ -1565,7 +1569,7 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) {
       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());
@@ -1587,7 +1591,7 @@ TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) {
       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());
@@ -1622,11 +1626,11 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) {
                              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(
@@ -1647,15 +1651,15 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) {
       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,
@@ -1664,7 +1668,7 @@ TEST_F(RenderFrameHostManagerTest, NavigateWithEarlyClose) {
 
   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());
 }
 
@@ -1690,7 +1694,7 @@ TEST_F(RenderFrameHostManagerTest, CloseWithPendingWhileUnresponsive) {
         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());
@@ -1714,19 +1718,19 @@ TEST_F(RenderFrameHostManagerTest, DeleteFrameAfterSwapOutACK) {
   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());
@@ -1762,13 +1766,13 @@ TEST_F(RenderFrameHostManagerTest, SwapOutFrameAfterSwapOutACK) {
   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());
@@ -1805,12 +1809,12 @@ TEST_F(RenderFrameHostManagerTest,
   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());
@@ -1849,7 +1853,7 @@ TEST_F(RenderFrameHostManagerTest,
     // 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.
@@ -1870,7 +1874,7 @@ TEST_F(RenderFrameHostManagerTest,
 
     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());
   }
 }
@@ -1941,8 +1945,7 @@ TEST_F(RenderFrameHostManagerTest, DetachPendingChild) {
             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));