PlzNavigate: make content unit tests work with browser side navigation

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 243 matching lines @@
   void set_should_create_webui(bool should_create_webui) {
     factory_.set_should_create_webui(should_create_webui);
   }
 
   void NavigateActiveAndCommit(const GURL& url) {
     // Note: we navigate the active RenderFrameHost because previous navigations
     // won't have committed yet, so NavigateAndCommit does the wrong thing
     // for us.
     controller().LoadURL(
         url, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
+
+    // Simulate the BeforeUnload_ACK that is received from the current renderer
+    // for a cross-site navigation.
+    // PlzNavigate: it is necessary to call PrepareForCommit before getting the
+    // main and the pending frame because when we are trying to navigate to a
+    // webui from a new tab, a RenderFrameHost is created to display it that is

[nasko, 2015/01/21 00:41:32]

nit: s/webui/WebUI/

[clamy, 2015/01/21 13:22:11]

Done.

+    // committed immediately (since it is a new tab). Therefore the main frame

[nasko, 2015/01/21 00:41:32]

Is this only the case for new tab with WebUI? What about window.open?

[clamy, 2015/01/21 13:22:11]

For this to happen, you need the current renderer to not be live, and to not be
reused for the navigation. So in the case of the WebUI I think it does not have
the right bindings, but for window.open, wouldn't we use the current RFH if it
is not live?

+    // is replaced without a pending frame being created, and we don't get the
+    // right values for the RFH to navigate: we try to use the old one that has
+    // been deleted in the meantime.
+    contents()->GetMainFrame()->PrepareForCommit(url);
+
     TestRenderFrameHost* old_rfh = contents()->GetMainFrame();
     TestRenderFrameHost* active_rfh = contents()->GetPendingMainFrame()
                                           ? contents()->GetPendingMainFrame()
                                           : old_rfh;
-
-    // Simulate the BeforeUnload_ACK that is received from the current renderer
-    // for a cross-site navigation.
-    if (old_rfh != active_rfh) {
-      old_rfh->SendBeforeUnloadACK(true);
-      EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, old_rfh->rfh_state());
-    }
+    EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, old_rfh->rfh_state());
 
     // Commit the navigation with a new page ID.
     int32 max_page_id = contents()->GetMaxPageIDForSiteInstance(
         active_rfh->GetSiteInstance());
 
     // Use an observer to avoid accessing a deleted renderer later on when the
     // state is being checked.
     RenderFrameHostDeletedObserver rfh_observer(old_rfh);
     RenderViewHostDeletedObserver rvh_observer(old_rfh->GetRenderViewHost());
     active_rfh->SendNavigate(max_page_id + 1, url);
@@ skipping 52 matching lines @@
     const GURL kChromeURL("chrome://foo");
     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(kDestUrl);
     EXPECT_TRUE(contents()->cross_navigation_pending());
 
     // 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);
 
     // BeforeUnload finishes.
     ntp_rfh->SendBeforeUnloadACK(true);
 
     dest_rfh->SendNavigate(101, kDestUrl);
     ntp_rfh->OnSwappedOut();
 
     EXPECT_TRUE(ntp_rfh->is_swapped_out());
     return ntp_rfh;
   }
 
+  // Returns the RenderFrameHost that should be used in the navigation to
+  // |entry|.
+  RenderFrameHostImpl* GetFrameHostForNavigation(

[carlosk, 2015/01/20 15:38:02]

Oh, I see what you did here! Initially I didn't like the name choice but I
realized that in the future, once PlzNavigate is done, we'll directly invoke the
likely named method from the |manager|. Nice!

+      RenderFrameHostManager* manager,
+      const NavigationEntryImpl& entry) {
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kEnableBrowserSideNavigation)) {
+      return manager->GetFrameHostForNavigation(
+          entry.GetURL(), entry.GetTransitionType());
+    }
+    return manager->Navigate(entry);
+  }
+
+  // Returns the pending RenderFrameHost.
+  // PlzNavigate: returns the speculative RenderFrameHost.
+  RenderFrameHostImpl* GetPendingFrameHost(

[carlosk, 2015/01/20 15:38:02]

Thinking about the future again, maybe here we should move away from the
"pending" name?

I understand that once the currently existing "pending" stuff is gone we could
rename speculative to that. But I think that will be somewhat confusing.

WDYT?

[clamy, 2015/01/21 13:22:11]

I was trying to match what is done with the
TestWebContents::GetPendingMainFrame(). I think it is better to keep a similar
name here, for consistency.

+      RenderFrameHostManager* manager) {
+    if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+        switches::kEnableBrowserSideNavigation)) {
+      return manager->speculative_render_frame_host_.get();
+    }
+    return manager->pending_frame_host();
+  }
+
  private:
   RenderFrameHostManagerTestWebUIControllerFactory factory_;
 };
 
 // Tests that when you navigate from a chrome:// url to another page, and
 // then do that same thing in another tab, that the two resulting pages have
 // different SiteInstances, BrowsingInstances, and RenderProcessHosts. This is
 // a regression test for bug 9364.
 TEST_F(RenderFrameHostManagerTest, NewTabPageProcesses) {
   set_should_create_webui(true);
@@ skipping 10 matching lines @@
 
   // Make a second tab.
   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(kChromeUrl);
   TestRenderFrameHost* ntp_rfh2 = contents2->GetMainFrame();
   EXPECT_FALSE(contents2->cross_navigation_pending());
   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(kDestUrl);
   EXPECT_TRUE(contents2->cross_navigation_pending());
   TestRenderFrameHost* dest_rfh2 = contents2->GetPendingMainFrame();
   ASSERT_TRUE(dest_rfh2);
 
-  ntp_rfh2->SendBeforeUnloadACK(true);
   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(
                    contents()->GetMainFrame()->GetSiteInstance()));
 
   // Navigate both to the new tab page, and verify that they share a
   // RenderProcessHost (not a SiteInstance).
   NavigateActiveAndCommit(kChromeUrl);
   EXPECT_FALSE(contents()->GetPendingMainFrame());
 
   contents2->GetController().LoadURL(
       kChromeUrl, Referrer(), ui::PAGE_TRANSITION_LINK, std::string());
-  dest_rfh2->SendBeforeUnloadACK(true);
+  contents2->GetMainFrame()->PrepareForCommit(kChromeUrl);
   contents2->GetPendingMainFrame()->SendNavigate(102, kChromeUrl);
 
   EXPECT_NE(contents()->GetMainFrame()->GetSiteInstance(),
             contents2->GetMainFrame()->GetSiteInstance());
   EXPECT_EQ(contents()->GetMainFrame()->GetSiteInstance()->GetProcess(),
             contents2->GetMainFrame()->GetSiteInstance()->GetProcess());
 }
 
 // Ensure that the browser ignores most IPC messages that arrive from a
 // RenderViewHost that has been swapped out.  We do not want to take
@@ skipping 314 matching lines @@
   // one will create a new SiteInstance. Because current_instance and
   // new_instance will be different, a new RenderViewHost will be created for
   // the second navigation. We have to avoid this in order to exercise the
   // target code patch.
   NavigateActiveAndCommit(kChromeUrl);
 
   // Navigate.
   controller().LoadURL(
       kUrl, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
   // Simulate response from RenderFrame for DispatchBeforeUnload.
-  base::TimeTicks now = base::TimeTicks::Now();
-  contents()->GetMainFrame()->OnMessageReceived(FrameHostMsg_BeforeUnload_ACK(
-      contents()->GetMainFrame()->GetRoutingID(), true, now, now));
+  contents()->GetMainFrame()->PrepareForCommit(kUrl);
   ASSERT_TRUE(contents()->GetPendingMainFrame())
       << "Expected new pending RenderFrameHost to be created.";
   RenderFrameHost* last_rfh = contents()->GetPendingMainFrame();
   int32 new_id =
       contents()->GetMaxPageIDForSiteInstance(last_rfh->GetSiteInstance()) + 1;
   contents()->GetPendingMainFrame()->SendNavigate(new_id, kUrl);
   EXPECT_EQ(controller().GetLastCommittedEntryIndex(), 1);
   ASSERT_TRUE(controller().GetLastCommittedEntry());
   EXPECT_TRUE(kUrl == controller().GetLastCommittedEntry()->GetURL());
   EXPECT_FALSE(controller().GetPendingEntry());
   // Because we're using TestWebContents and TestRenderViewHost in this
   // unittest, no one calls WebContentsImpl::RenderViewCreated(). So, we see no
   // EnableViewSourceMode message, here.
 
   // Clear queued messages before load.
   process()->sink().ClearMessages();
   // Navigate, again.
   controller().LoadURL(
       kUrl, Referrer(), ui::PAGE_TRANSITION_TYPED, std::string());
+  contents()->GetMainFrame()->PrepareForCommit(kUrl);
   // The same RenderViewHost should be reused.
   EXPECT_FALSE(contents()->GetPendingMainFrame());
   EXPECT_TRUE(last_rfh == contents()->GetMainFrame());
   // Navigate using the returned page_id.
   contents()->GetMainFrame()->SendNavigate(new_id, kUrl);
   EXPECT_EQ(controller().GetLastCommittedEntryIndex(), 1);
   EXPECT_FALSE(controller().GetPendingEntry());
   // New message should be sent out to make sure to enter view-source mode.
   EXPECT_TRUE(process()->sink().GetUniqueMessageMatching(
       ViewMsg_EnableViewSourceMode::ID));
@@ skipping 36 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);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_TRUE(host == manager->current_frame_host());
-  EXPECT_FALSE(manager->pending_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
 
   // Commit.
   manager->DidNavigateFrame(host, true);
   // Commit to SiteInstance should be delayed until RenderFrame commit.
   EXPECT_TRUE(host == manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_FALSE(host->GetSiteInstance()->HasSite());
   host->GetSiteInstance()->SetSite(kUrl1);
 
   // 2) Navigate to next site. -------------------------
   const GURL kUrl2("http://www.google.com/foo");
   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_TRUE(host == manager->current_frame_host());
-  EXPECT_FALSE(manager->pending_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
 
   // Commit.
   manager->DidNavigateFrame(host, true);
   EXPECT_TRUE(host == manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
 
   // 3) Cross-site navigate to next site. --------------
   const GURL kUrl3("http://webkit.org/");
   NavigationEntryImpl entry3(
       NULL /* instance */, -1 /* page_id */, kUrl3,
       Referrer(kUrl2, blink::WebReferrerPolicyDefault),
       base::string16() /* title */, ui::PAGE_TRANSITION_LINK,
       false /* is_renderer_init */);
-  host = manager->Navigate(entry3);
+  host = GetFrameHostForNavigation(manager, entry3);
 
   // A new RenderFrameHost should be created.
-  EXPECT_TRUE(manager->pending_frame_host());
-  ASSERT_EQ(host, manager->pending_frame_host());
+  EXPECT_TRUE(GetPendingFrameHost(manager));
+  ASSERT_EQ(host, GetPendingFrameHost(manager));
 
   notifications.Reset();
 
   // Commit.
-  manager->DidNavigateFrame(manager->pending_frame_host(), true);
+  manager->DidNavigateFrame(GetPendingFrameHost(manager), true);
   EXPECT_TRUE(host == manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
   // Check the pending RenderFrameHost has been committed.
-  EXPECT_FALSE(manager->pending_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
 
   // We should observe a notification.
   EXPECT_TRUE(
       notifications.Check1AndReset(NOTIFICATION_RENDER_VIEW_HOST_CHANGED));
 }
 
 // Tests WebUI creation.
 TEST_F(RenderFrameHostManagerTest, WebUI) {
   set_should_create_webui(true);
   SiteInstance* instance = SiteInstance::Create(browser_context());
 
   scoped_ptr<TestWebContents> web_contents(
       TestWebContents::Create(browser_context(), instance));
   RenderFrameHostManager* manager = web_contents->GetRenderManagerForTesting();
 
   EXPECT_FALSE(manager->current_host()->IsRenderViewLive());
 
   const GURL kUrl("chrome://foo");
   NavigationEntryImpl entry(NULL /* instance */, -1 /* page_id */, kUrl,
                             Referrer(), base::string16() /* title */,
                             ui::PAGE_TRANSITION_TYPED,
                             false /* is_renderer_init */);
-  RenderFrameHostImpl* host = manager->Navigate(entry);
+  RenderFrameHostImpl* host = GetFrameHostForNavigation(manager, entry);
 
   // We commit the pending RenderFrameHost immediately because the previous
   // RenderFrameHost was not live.  We test a case where it is live in
   // WebUIInNewTab.
   EXPECT_TRUE(host);
   EXPECT_EQ(host, manager->current_frame_host());
-  EXPECT_FALSE(manager->pending_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
 
   // It's important that the site instance get set on the Web UI page as soon
   // as the navigation starts, rather than lazily after it commits, so we don't
   // try to re-use the SiteInstance/process for non Web UI things that may
   // get loaded in between.
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
   EXPECT_EQ(kUrl, host->GetSiteInstance()->GetSiteURL());
 
   // The Web UI is committed immediately because the RenderViewHost has not been
   // used yet. UpdateStateForNavigate() took the short cut path.
@@ skipping 22 matching lines @@
       base::string16(), -1, MSG_ROUTING_NONE, -1, false);
   EXPECT_TRUE(manager1->current_host()->IsRenderViewLive());
   EXPECT_TRUE(manager1->current_frame_host()->IsRenderFrameLive());
 
   // Navigate to a WebUI page.
   const GURL kUrl1("chrome://foo");
   NavigationEntryImpl entry1(NULL /* instance */, -1 /* page_id */, kUrl1,
                              Referrer(), base::string16() /* title */,
                              ui::PAGE_TRANSITION_TYPED,
                              false /* is_renderer_init */);
-  RenderFrameHostImpl* host1 = manager1->Navigate(entry1);
+  RenderFrameHostImpl* host1 = GetFrameHostForNavigation(manager1, entry1);
 
   // We should have a pending navigation to the WebUI RenderViewHost.
   // It should already have bindings.
-  EXPECT_EQ(host1, manager1->pending_frame_host());
+  EXPECT_EQ(host1, GetPendingFrameHost(manager1));
   EXPECT_NE(host1, manager1->current_frame_host());
   EXPECT_TRUE(
       host1->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
   // Commit and ensure we still have bindings.
   manager1->DidNavigateFrame(host1, true);
   SiteInstance* webui_instance = host1->GetSiteInstance();
   EXPECT_EQ(host1, manager1->current_frame_host());
   EXPECT_TRUE(
       host1->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
   // Now simulate clicking a link that opens in a new tab.
   scoped_ptr<TestWebContents> web_contents2(
       TestWebContents::Create(browser_context(), webui_instance));
   RenderFrameHostManager* manager2 =
       web_contents2->GetRenderManagerForTesting();
   // Make sure the new RVH is considered live.  This is usually done in
   // RenderWidgetHost::Init when opening a new tab from a link.
   manager2->current_host()->CreateRenderView(
       base::string16(), -1, MSG_ROUTING_NONE, -1, false);
 
   const GURL kUrl2("chrome://foo/bar");
   NavigationEntryImpl entry2(NULL /* instance */, -1 /* page_id */, kUrl2,
                              Referrer(), base::string16() /* title */,
                              ui::PAGE_TRANSITION_LINK,
                              true /* is_renderer_init */);
-  RenderFrameHostImpl* host2 = manager2->Navigate(entry2);
+  RenderFrameHostImpl* host2 = GetFrameHostForNavigation(manager2, entry2);
 
   // No cross-process transition happens because we are already in the right
   // SiteInstance.  We should grant bindings immediately.
   EXPECT_EQ(host2, manager2->current_frame_host());
   EXPECT_TRUE(
       host2->render_view_host()->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 
   manager2->DidNavigateFrame(host2, true);
 }
 
 // Tests that we don't end up in an inconsistent state if a page does a back and
 // then reload. http://crbug.com/51680
 // Also tests that only user-gesture navigations can interrupt cross-process
 // navigations. http://crbug.com/75195
 TEST_F(RenderFrameHostManagerTest, PageDoesBackAndReload) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.evil-site.com/");
 
   // Navigate to a safe site, then an evil site.
   // This will switch RenderFrameHosts.  We cannot assert that the first and
   // second RFHs are different, though, because the first one may be promptly
   // deleted.
   contents()->NavigateAndCommit(kUrl1);
   contents()->NavigateAndCommit(kUrl2);
   TestRenderFrameHost* evil_rfh = contents()->GetMainFrame();
 
   // Now let's simulate the evil page calling history.back().
   contents()->OnGoToEntryAtOffset(-1);
+  contents()->GetMainFrame()->PrepareForCommit(kUrl1);
   // We should have a new pending RFH.
   // Note that in this case, the navigation has not committed, so evil_rfh will
   // not be deleted yet.
   EXPECT_NE(evil_rfh, contents()->GetPendingMainFrame());
   EXPECT_NE(evil_rfh->GetRenderViewHost(),
             contents()->GetPendingMainFrame()->GetRenderViewHost());
 
   // Before that RFH has committed, the evil page reloads itself.
   FrameHostMsg_DidCommitProvisionalLoad_Params params;
   params.page_id = 1;
@@ skipping 63 matching lines @@
   rfh1->GetSiteInstance()->increment_active_frame_count();
 
   contents()->NavigateAndCommit(kUrl2);
   TestRenderFrameHost* rfh2 = main_test_rfh();
   rfh2->GetSiteInstance()->increment_active_frame_count();
 
   // Now go back, but suppose the SwapOut_ACK isn't received.  This shouldn't
   // happen, but we have seen it when going back quickly across many entries
   // (http://crbug.com/93427).
   contents()->GetController().GoBack();
-  EXPECT_TRUE(rfh2->is_waiting_for_beforeunload_ack());
-  contents()->ProceedWithCrossSiteNavigation();
-  EXPECT_FALSE(rfh2->is_waiting_for_beforeunload_ack());
+  EXPECT_TRUE(rfh2->IsWaitingForBeforeUnloadACK());
+  contents()->GetMainFrame()->PrepareForCommit(kUrl1);
+  EXPECT_FALSE(rfh2->IsWaitingForBeforeUnloadACK());
 
   // The back navigation commits.
   const NavigationEntry* entry1 = contents()->GetController().GetPendingEntry();
   rfh1->SendNavigate(entry1->GetPageID(), entry1->GetURL());
   EXPECT_TRUE(rfh2->IsWaitingForUnloadACK());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh2->rfh_state());
 
   // We should be able to navigate forward.
   contents()->GetController().GoForward();
-  contents()->ProceedWithCrossSiteNavigation();
+  contents()->GetMainFrame()->PrepareForCommit(kUrl2);
   const NavigationEntry* entry2 = contents()->GetController().GetPendingEntry();
   rfh2->SendNavigate(entry2->GetPageID(), entry2->GetURL());
   EXPECT_EQ(rfh2, main_test_rfh());
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state());
   EXPECT_EQ(RenderFrameHostImpl::STATE_PENDING_SWAP_OUT, rfh1->rfh_state());
   rfh1->OnSwappedOut();
   EXPECT_TRUE(rfh1->is_swapped_out());
   EXPECT_EQ(RenderFrameHostImpl::STATE_SWAPPED_OUT, rfh1->rfh_state());
 }
 
@@ skipping 150 matching lines @@
   EXPECT_TRUE(contents()->HasOpener());
 
   // Navigate to a cross-site URL (different SiteInstance but same
   // BrowsingInstance).
   contents()->NavigateAndCommit(kUrl2);
   TestRenderFrameHost* rfh2 = main_test_rfh();
   EXPECT_NE(rfh1->GetSiteInstance(), rfh2->GetSiteInstance());
 
   // Start a back navigation so that rfh1 becomes the pending RFH.
   contents()->GetController().GoBack();
-  contents()->ProceedWithCrossSiteNavigation();
+  contents()->GetMainFrame()->PrepareForCommit(kUrl1);
 
   // Disown the opener from rfh2.
   rfh2->DidDisownOpener();
 
   // Ensure the opener is cleared.
   EXPECT_FALSE(contents()->HasOpener());
 
   // The back navigation commits.
   const NavigationEntry* entry1 = contents()->GetController().GetPendingEntry();
   rfh1->SendNavigate(entry1->GetPageID(), entry1->GetURL());
@@ skipping 20 matching lines @@
 
   // Navigate to a cross-site URL (different SiteInstance but same
   // BrowsingInstance).
   contents()->NavigateAndCommit(kUrl2);
   TestRenderFrameHost* rfh2 = main_test_rfh();
   EXPECT_NE(rfh1->GetSiteInstance(), rfh2->GetSiteInstance());
 
   // Commit a back navigation before the DidDisownOpener message arrives.
   // rfh1 will be kept alive because of the opener tab.
   contents()->GetController().GoBack();
-  contents()->ProceedWithCrossSiteNavigation();
+  contents()->GetMainFrame()->PrepareForCommit(kUrl1);
   const NavigationEntry* entry1 = contents()->GetController().GetPendingEntry();
   rfh1->SendNavigate(entry1->GetPageID(), entry1->GetURL());
 
   // Disown the opener from rfh2.
   rfh2->DidDisownOpener();
   EXPECT_FALSE(contents()->HasOpener());
 }
 
 // Test that we clean up swapped out RenderViewHosts when a process hosting
 // those associated RenderViews crashes. http://crbug.com/258993
@@ skipping 35 matching lines @@
       Details<RenderProcessHost::RendererClosedDetails>(&details));
   rvh1->set_render_view_created(false);
 
   // Ensure that the swapped out RenderViewHost has been deleted.
   EXPECT_FALSE(opener1_manager->GetSwappedOutRenderViewHost(
       rvh1->GetSiteInstance()));
 
   // Reload the initial tab. This should recreate the opener's swapped out RVH
   // in the original SiteInstance.
   contents()->GetController().Reload(true);
+  contents()->GetMainFrame()->PrepareForCommit(kUrl1);
   EXPECT_EQ(opener1_manager->GetSwappedOutRenderViewHost(
                 rvh1->GetSiteInstance())->GetRoutingID(),
             test_rvh()->opener_route_id());
 }
 
 // Test that RenderViewHosts created for WebUI navigations are properly
 // granted WebUI bindings even if an unprivileged swapped out RenderViewHost
 // is in the same process (http://crbug.com/79918).
 TEST_F(RenderFrameHostManagerTest, EnableWebUIWithSwappedOutOpener) {
   set_should_create_webui(true);
@@ skipping 172 matching lines @@
   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(kUrl2);
+  }
   EXPECT_TRUE(contents()->cross_navigation_pending());
 
   // 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());
-  base::TimeTicks now = base::TimeTicks::Now();
-  contents()->GetMainFrame()->OnMessageReceived(
-      FrameHostMsg_BeforeUnload_ACK(0, true, now, now));
+  contents()->GetMainFrame()->PrepareForCommit(kUrl2);
   EXPECT_TRUE(contents()->cross_navigation_pending());
   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_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh1->rfh_state());
 
@@ skipping 27 matching lines @@
   RenderFrameHostDeletedObserver rfh_deleted_observer(rfh1);
   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());
-  base::TimeTicks now = base::TimeTicks::Now();
-  contents()->GetMainFrame()->OnMessageReceived(
-      FrameHostMsg_BeforeUnload_ACK(0, true, now, now));
+  contents()->GetMainFrame()->PrepareForCommit(kUrl2);
   EXPECT_TRUE(contents()->cross_navigation_pending());
   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_EQ(rfh2, contents()->GetMainFrame());
   EXPECT_TRUE(contents()->GetPendingMainFrame() == NULL);
   EXPECT_EQ(RenderFrameHostImpl::STATE_DEFAULT, rfh2->rfh_state());
@@ skipping 22 matching lines @@
   RenderFrameHostDeletedObserver rfh_deleted_observer(rfh1);
   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());
-  base::TimeTicks now = base::TimeTicks::Now();
-  rfh1->OnMessageReceived(
-      FrameHostMsg_BeforeUnload_ACK(0, true, now, now));
+  rfh1->PrepareForCommit(kUrl2);
   EXPECT_TRUE(contents()->cross_navigation_pending());
   TestRenderFrameHost* rfh2 = contents()->GetPendingMainFrame();
 
   // The new page commits.
   contents()->TestDidNavigate(rfh2, 1, kUrl2, ui::PAGE_TRANSITION_TYPED);
   EXPECT_FALSE(contents()->cross_navigation_pending());
   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());
@@ skipping 73 matching lines @@
       contents()->GetMainFrame()->GetProcess()->GetNextRoutingID(),
       std::string("frame_name"), SandboxFlags::NONE);
   RenderFrameHostManager* manager =
       contents()->GetFrameTree()->root()->child_at(0)->render_manager();
 
   // 1) The first navigation. --------------------------
   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);
 
   // The RenderFrameHost created in Init will be reused.
   EXPECT_TRUE(host == manager->current_frame_host());
-  EXPECT_FALSE(manager->pending_frame_host());
+  EXPECT_FALSE(GetPendingFrameHost(manager));
 
   // Commit.
   manager->DidNavigateFrame(host, true);
   // Commit to SiteInstance should be delayed until RenderFrame commit.
   EXPECT_TRUE(host == manager->current_frame_host());
   ASSERT_TRUE(host);
   EXPECT_TRUE(host->GetSiteInstance()->HasSite());
 
   // 2) Cross-site navigate to next site. --------------
   NavigationEntryImpl entry2(NULL /* instance */, -1 /* page_id */, kUrl2,
                              Referrer(kUrl1, blink::WebReferrerPolicyDefault),
                              base::string16() /* title */,
                              ui::PAGE_TRANSITION_LINK,
                              false /* is_renderer_init */);
-  host = manager->Navigate(entry2);
+  host = GetFrameHostForNavigation(manager, entry2);
 
   // A new RenderFrameHost should be created.
-  EXPECT_TRUE(manager->pending_frame_host());
-  ASSERT_EQ(host, manager->pending_frame_host());
-  ASSERT_NE(manager->current_frame_host(), manager->pending_frame_host());
+  EXPECT_TRUE(GetPendingFrameHost(manager));
+  ASSERT_EQ(host, GetPendingFrameHost(manager));
+  ASSERT_NE(manager->current_frame_host(), GetPendingFrameHost(manager));
   EXPECT_FALSE(contents()->cross_navigation_pending())
       << "There should be no top-level pending navigation.";
 
-  RenderFrameHostDeletedObserver delete_watcher(manager->pending_frame_host());
+  RenderFrameHostDeletedObserver delete_watcher(GetPendingFrameHost(manager));
   EXPECT_FALSE(delete_watcher.deleted());
 
   // Extend the lifetime of the child frame's SiteInstance, pretending
   // that there is another reference to it.
   scoped_refptr<SiteInstanceImpl> site_instance =
-      manager->pending_frame_host()->GetSiteInstance();
+      GetPendingFrameHost(manager)->GetSiteInstance();
   EXPECT_TRUE(site_instance->HasSite());
   EXPECT_NE(site_instance, contents()->GetSiteInstance());
   EXPECT_EQ(1U, site_instance->active_frame_count());
   site_instance->increment_active_frame_count();
   EXPECT_EQ(2U, site_instance->active_frame_count());
 
   // Now detach the child FrameTreeNode. This should kill the pending host.
   manager->current_frame_host()->OnMessageReceived(
       FrameHostMsg_Detach(manager->current_frame_host()->GetRoutingID()));
 
   EXPECT_TRUE(delete_watcher.deleted());
 
   EXPECT_EQ(1U, site_instance->active_frame_count());
   site_instance->decrement_active_frame_count();
 
 #if 0
   // TODO(nick): Currently a proxy to the removed frame lingers in the parent.
   // Enable this assert below once the proxies to the subframe are correctly
   // cleaned up after detach. http://crbug.com/444955.
   ASSERT_TRUE(site_instance->HasOneRef())
       << "This SiteInstance should be destroyable now.";
 #endif
 }
 
 }  // namespace content