Have the unload event execute in background on cross-site navigations

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/strings/utf_string_conversions.h"
 #include "content/browser/frame_host/navigation_controller_impl.h"
 #include "content/browser/frame_host/navigation_entry_impl.h"
 #include "content/browser/frame_host/render_frame_host_manager.h"
 #include "content/browser/site_instance_impl.h"
 #include "content/browser/webui/web_ui_controller_factory_registry.h"
@@ skipping 151 matching lines @@
         increment_active_view_count();
 
     TestRenderViewHost* dest_rvh = static_cast<TestRenderViewHost*>(
         contents()->GetRenderManagerForTesting()->pending_render_view_host());
     CHECK(dest_rvh);
     EXPECT_NE(ntp_rvh, dest_rvh);
 
     // BeforeUnload finishes.
     ntp_rvh->SendShouldCloseACK(true);
 
-    // Assume SwapOutACK times out, so the dest_rvh proceeds and commits.
     dest_rvh->SendNavigate(101, kDestUrl);
+    ntp_rvh->OnSwappedOut(false);
 
-    EXPECT_TRUE(ntp_rvh->is_swapped_out());
+    EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, ntp_rvh->rvh_state());
     return ntp_rvh;
   }
 
  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
@@ skipping 103 matching lines @@
 
   // The new RVH should be able to update its title.
   const base::string16 dest_title = base::ASCIIToUTF16("Google");
   EXPECT_TRUE(dest_rvh->OnMessageReceived(
       ViewHostMsg_UpdateTitle(rvh()->GetRoutingID(), 101, dest_title,
                               direction)));
   EXPECT_EQ(dest_title, contents()->GetTitle());
 
   // The old renderer, being slow, now updates the title. It should be filtered
   // out and not take effect.
-  EXPECT_TRUE(ntp_rvh->is_swapped_out());
+  EXPECT_NE(RenderViewHostImpl::STATE_LIVE, ntp_rvh->rvh_state());

[nasko, 2014/01/27 19:09:51]

Shouldn't this be test for states where we filter IPCs?

[clamy, 2014/01/28 12:27:21]

Done.

   EXPECT_TRUE(ntp_rvh->OnMessageReceived(
       ViewHostMsg_UpdateTitle(rvh()->GetRoutingID(), 0, ntp_title, direction)));
   EXPECT_EQ(dest_title, contents()->GetTitle());
 
   // We cannot filter out synchronous IPC messages, because the renderer would
   // be left waiting for a reply.  We pick RunBeforeUnloadConfirm as an example
   // that can run easily within a unit test, and that needs to receive a reply
   // without showing an actual dialog.
   MockRenderProcessHost* ntp_process_host =
       static_cast<MockRenderProcessHost*>(ntp_rvh->GetProcess());
@@ skipping 44 matching lines @@
   ViewHostMsg_DidActivateAcceleratedCompositing msg(
       rvh()->GetRoutingID(), true);
   EXPECT_TRUE(swapped_out_rvh->OnMessageReceived(msg));
   EXPECT_TRUE(swapped_out_rvh->is_accelerated_compositing_active());
 }
 
 // Test if RenderViewHost::GetRenderWidgetHosts() only returns active
 // widgets.
 TEST_F(RenderFrameHostManagerTest, GetRenderWidgetHostsReturnsActiveViews) {
   TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
-  EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
+  EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT,
+            swapped_out_rvh->rvh_state());
 
   scoped_ptr<RenderWidgetHostIterator> widgets(
       RenderWidgetHost::GetRenderWidgetHosts());
   // We know that there is the only one active widget. Another view is
   // now swapped out, so the swapped out view is not included in the
   // list.
   RenderWidgetHost* widget = widgets->GetNextHost();
   EXPECT_FALSE(widgets->GetNextHost());
   RenderViewHost* rvh = RenderViewHost::From(widget);
-  EXPECT_FALSE(static_cast<RenderViewHostImpl*>(rvh)->is_swapped_out());
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE,
+            static_cast<RenderViewHostImpl*>(rvh)->rvh_state());
 }
 
 // Test if RenderViewHost::GetRenderWidgetHosts() returns a subset of
 // RenderViewHostImpl::GetAllRenderWidgetHosts().
 // RenderViewHost::GetRenderWidgetHosts() returns only active widgets, but
 // RenderViewHostImpl::GetAllRenderWidgetHosts() returns everything
 // including swapped out ones.
 TEST_F(RenderFrameHostManagerTest,
        GetRenderWidgetHostsWithinGetAllRenderWidgetHosts) {
   TestRenderViewHost* swapped_out_rvh = CreateSwappedOutRenderViewHost();
-  EXPECT_TRUE(swapped_out_rvh->is_swapped_out());
+  EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT,
+            swapped_out_rvh->rvh_state());
 
   scoped_ptr<RenderWidgetHostIterator> widgets(
       RenderWidgetHost::GetRenderWidgetHosts());
 
   while (RenderWidgetHost* w = widgets->GetNextHost()) {
     bool found = false;
     scoped_ptr<RenderWidgetHostIterator> all_widgets(
         RenderWidgetHostImpl::GetAllRenderWidgetHosts());
     while (RenderWidgetHost* widget = all_widgets->GetNextHost()) {
       if (w == widget) {
@@ skipping 365 matching lines @@
       static_cast<TestRenderViewHost*>(host->render_view_host());
   MockRenderProcessHost* test_process_host =
       static_cast<MockRenderProcessHost*>(test_host->GetProcess());
   EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
       ViewMsg_ShouldClose::ID));
   test_host->SendShouldCloseACK(true);
 
   // CrossSiteResourceHandler::StartCrossSiteTransition triggers a
   // call of RenderFrameHostManager::SwapOutOldPage before
   // RenderFrameHostManager::DidNavigateMainFrame is called.
-  // The RVH is not swapped out until the commit.
+  // The RVH is swapped out after receiving the unload ack.
   manager->SwapOutOldPage();
   EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
       ViewMsg_SwapOut::ID));
   test_host->OnSwappedOut(false);
 
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(manager->current_frame_host()->is_swapped_out());
   EXPECT_EQ(host2, manager->pending_frame_host());
   // There should be still no navigation messages being sent.
   EXPECT_FALSE(test_process_host2->sink().GetUniqueMessageMatching(
       ViewMsg_Navigate::ID));
 
   // 3) Cross-site navigate to next site before 2) has committed. --------------
   const GURL kUrl3("http://webkit.org/");
   NavigationEntryImpl entry3(NULL /* instance */, -1 /* page_id */, kUrl3,
                              Referrer(), base::string16() /* title */,
                              PAGE_TRANSITION_TYPED,
                              false /* is_renderer_init */);
   test_process_host->sink().ClearMessages();
   RenderFrameHostImpl* host3 = manager->Navigate(entry3);
 
   // A new RenderFrameHost should be created. host2 is now deleted.
   EXPECT_TRUE(manager->pending_frame_host());
   ASSERT_EQ(host3, manager->pending_frame_host());
   EXPECT_NE(host3, host);
   EXPECT_NE(host3->GetProcess()->GetID(), host2_process_id);
 
-  // Navigations in the new RVH should be suspended, which is ok because the
-  // old RVH is not yet swapped out and can respond to a second beforeunload
-  // request.
+  // Navigations in the new RVH should be suspended.
   EXPECT_TRUE(static_cast<RenderViewHostImpl*>(
       host3->render_view_host())->are_navigations_suspended());
   EXPECT_EQ(host, manager->current_frame_host());
   EXPECT_FALSE(manager->current_frame_host()->is_swapped_out());
 
   // Simulate a response to the second beforeunload request.
   EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
       ViewMsg_ShouldClose::ID));
   test_host->SendShouldCloseACK(true);
 
   // CrossSiteResourceHandler::StartCrossSiteTransition triggers a
   // call of RenderFrameHostManager::SwapOutOldPage before
   // RenderFrameHostManager::DidNavigateMainFrame is called.
-  // The RVH is not swapped out until the commit.
   manager->SwapOutOldPage();
   EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
       ViewMsg_SwapOut::ID));
   test_host->OnSwappedOut(false);
 
   // Commit.
   manager->DidNavigateMainFrame(host3->render_view_host());
   EXPECT_TRUE(host3 == manager->current_frame_host());
   ASSERT_TRUE(host3);
   EXPECT_TRUE(static_cast<SiteInstanceImpl*>(host3->GetSiteInstance())->
@@ skipping 192 matching lines @@
       increment_active_view_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(rvh2->is_waiting_for_beforeunload_ack());
   contents()->ProceedWithCrossSiteNavigation();
   EXPECT_FALSE(rvh2->is_waiting_for_beforeunload_ack());
   rvh2->SwapOut();
-  EXPECT_TRUE(rvh2->is_waiting_for_unload_ack());
+  EXPECT_TRUE(rvh2->IsWaitingForUnloadACK());
 
-  // The back navigation commits.  We should proactively clear the
-  // is_waiting_for_unload_ack state to be safe.
+  // The back navigation commits.
   const NavigationEntry* entry1 = contents()->GetController().GetPendingEntry();
   rvh1->SendNavigate(entry1->GetPageID(), entry1->GetURL());
-  EXPECT_TRUE(rvh2->is_swapped_out());
-  EXPECT_FALSE(rvh2->is_waiting_for_unload_ack());

[nasko, 2014/01/27 19:09:51]

Why is this expectation dropped?

[clamy, 2014/01/28 12:27:21]

If the RVH is in STATE_PENDING_SWAP_OUT then IsWaitingForUnloadAck is true.
Since I dropped the boolean is_waiting_for_unload_ack_, IsWaitingForUnloadAck
returns true when the RVH is in STATE_WAITING_FOR_UNLOAD_ACK,
STATE_WAITING_FOR_CLOSE, STATE_PENDING_SWAP_OUT, STATE_PENDING_SHUTDOWN.

+  EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh2->rvh_state());
 
   // We should be able to navigate forward.
   contents()->GetController().GoForward();
   contents()->ProceedWithCrossSiteNavigation();
   const NavigationEntry* entry2 = contents()->GetController().GetPendingEntry();
   rvh2->SendNavigate(entry2->GetPageID(), entry2->GetURL());
   EXPECT_EQ(rvh2, rvh());
-  EXPECT_FALSE(rvh2->is_swapped_out());
-  EXPECT_TRUE(rvh1->is_swapped_out());
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE, rvh2->rvh_state());
+  EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh1->rvh_state());

[nasko, 2014/01/27 19:09:51]

Why not add a call to OnSwapOut() and then check that we have moved rvh1 to
swapped out state? It will make the test match a bit closer what it used to
test.

[clamy, 2014/01/28 12:27:21]

Done.

 }
 
 // Test that we create swapped out RVHs for the opener chain when navigating an
 // opened tab cross-process.  This allows us to support certain cross-process
 // JavaScript calls (http://crbug.com/99202).
 TEST_F(RenderFrameHostManagerTest, CreateSwappedOutOpenerRVHs) {
   const GURL kUrl1("http://www.google.com/");
   const GURL kUrl2("http://www.chromium.org/");
   const GURL kChromeUrl("chrome://foo");
 
@@ skipping 26 matching lines @@
 
   // Ensure rvh1 is placed on swapped out list of the current tab.
   EXPECT_TRUE(manager->IsRVHOnSwappedOutList(rvh1));
   EXPECT_EQ(rvh1,
             manager->GetSwappedOutRenderViewHost(rvh1->GetSiteInstance()));
 
   // Ensure a swapped out RVH is created in the first opener tab.
   TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>(
       opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
   EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh));
-  EXPECT_TRUE(opener1_rvh->is_swapped_out());
+  EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, opener1_rvh->rvh_state());
 
   // Ensure a swapped out RVH is created in the second opener tab.
   TestRenderViewHost* opener2_rvh = static_cast<TestRenderViewHost*>(
       opener2_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
   EXPECT_TRUE(opener2_manager->IsRVHOnSwappedOutList(opener2_rvh));
-  EXPECT_TRUE(opener2_rvh->is_swapped_out());
+  EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, opener2_rvh->rvh_state());
 
   // Navigate to a cross-BrowsingInstance URL.
   contents()->NavigateAndCommit(kChromeUrl);
   TestRenderViewHost* rvh3 = test_rvh();
   EXPECT_NE(rvh1->GetSiteInstance(), rvh3->GetSiteInstance());
   EXPECT_FALSE(rvh1->GetSiteInstance()->IsRelatedSiteInstance(
                    rvh3->GetSiteInstance()));
 
   // No scripting is allowed across BrowsingInstances, so we should not create
   // swapped out RVHs for the opener chain in this case.
@@ skipping 81 matching lines @@
   contents()->NavigateAndCommit(kPluginUrl);
   TestRenderViewHost* rvh2 = test_rvh();
   EXPECT_NE(rvh1->GetSiteInstance(), rvh2->GetSiteInstance());
   EXPECT_TRUE(rvh1->GetSiteInstance()->IsRelatedSiteInstance(
                   rvh2->GetSiteInstance()));
 
   // Ensure a swapped out RVH is created in the first opener tab.
   TestRenderViewHost* opener1_rvh = static_cast<TestRenderViewHost*>(
       opener1_manager->GetSwappedOutRenderViewHost(rvh2->GetSiteInstance()));
   EXPECT_TRUE(opener1_manager->IsRVHOnSwappedOutList(opener1_rvh));
-  EXPECT_TRUE(opener1_rvh->is_swapped_out());
+  EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, opener1_rvh->rvh_state());
 
   // Ensure the new RVH has WebUI bindings.
   EXPECT_TRUE(rvh2->GetEnabledBindings() & BINDINGS_POLICY_WEB_UI);
 }
 
 // Test that we reuse the same guest SiteInstance if we navigate across sites.
 TEST_F(RenderFrameHostManagerTest, NoSwapOnGuestNavigations) {
   TestNotificationTracker notifications;
 
   GURL guest_url(std::string(kGuestScheme).append("://abc123"));
@@ skipping 125 matching lines @@
   notifications.ListenFor(NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED,
                           Source<RenderWidgetHost>(host2->render_view_host()));
   manager->ShouldClosePage(false, true, base::TimeTicks());
 
   EXPECT_TRUE(
       notifications.Check1AndReset(NOTIFICATION_RENDER_WIDGET_HOST_DESTROYED));
   EXPECT_FALSE(manager->pending_frame_host());
   EXPECT_EQ(host, manager->current_frame_host());
 }
 
+// This checks that the given RVH has been properly deleted.
+class RenderViewHostDestructionChecker : public WebContentsObserver {

[nasko, 2014/01/27 19:09:51]

nit: s/Checker/Observer/

[clamy, 2014/01/28 12:27:21]

Done.

+ public:
+  RenderViewHostDestructionChecker(RenderViewHost* render_view_host)
+      : WebContentsObserver(WebContents::FromRenderViewHost(render_view_host)),
+        render_view_host_(render_view_host),
+        rvh_deleted_(false) {}
+
+  bool rvh_deleted() { return rvh_deleted_; }
+
+  virtual void RenderViewDeleted(RenderViewHost* render_view_host) OVERRIDE {
+    if (render_view_host == render_view_host_)
+      rvh_deleted_ = true;
+  }
+
+ private:
+  RenderViewHost* render_view_host_;
+  bool rvh_deleted_;
+
+  DISALLOW_COPY_AND_ASSIGN(RenderViewHostDestructionChecker);
+};
+
+// Tests that the RenderViewHost is properly swapped out or deleted when the
+// SwapOutACK is received before the new page commits.
+TEST_F(RenderFrameHostManagerTest, SwapOutACKBeforeNewPageCommits) {
+  const GURL kUrl1("http://www.google.com/");
+  const GURL kUrl2("http://www.chromium.org/");
+
+  // Navigate to the first page.
+  contents()->NavigateAndCommit(kUrl1);
+  TestRenderViewHost* rvh1 = test_rvh();
+  RenderViewHostDestructionChecker destruction_checker1(rvh1);
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE, rvh1->rvh_state());
+
+  // Navigate to new site, simulating onbeforeunload approval.
+  controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
+  base::TimeTicks now = base::TimeTicks::Now();
+  rvh1->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  TestRenderViewHost* rvh2 =
+      static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
+
+  // Simulate rvh2's response, which leads to an unload request being sent to
+  // rvh1.
+  std::vector<GURL> url_chain1;
+  url_chain1.push_back(GURL());
+  contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
+      rvh2,

[nasko, 2014/01/27 19:09:51]

nit: you can put multiple arguments on one line to save some vertical space

[clamy, 2014/01/28 12:27:21]

Done.

+      GlobalRequestID(0, 0),
+      false,
+      url_chain1,
+      Referrer(),
+      PAGE_TRANSITION_TYPED,
+      1,
+      false);
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
+            rvh1->rvh_state());
+
+  // Simulate the swap out ack.
+  rvh1->OnSwappedOut(false);
+  EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_COMMIT, rvh1->rvh_state());
+
+  // The new page commits.
+  contents()->TestDidNavigate(rvh2, 1, kUrl2, PAGE_TRANSITION_TYPED);
+  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_EQ(rvh2, rvh());
+  EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE, rvh2->rvh_state());
+
+  // rvh1 should have been deleted.
+  EXPECT_TRUE(destruction_checker1.rvh_deleted());
+  rvh1 = NULL;
+
+  // Increment the number of active views in SiteInstanceImpl so that rvh2 is

[nasko, 2014/01/27 19:09:51]

The code following seems like a separate test case. Since this is unit tests and
it is cheap to start new test, might as well make it separate.

[clamy, 2014/01/28 12:27:21]

Done.

+  // not deleted on swap out.
+  static_cast<SiteInstanceImpl*>(
+      rvh2->GetSiteInstance())->increment_active_view_count();
+  RenderViewHostDestructionChecker destruction_checker2(rvh2);
+
+  // Navigate to new site, simulating onbeforeunload approval.
+  controller().LoadURL(kUrl1, Referrer(), PAGE_TRANSITION_LINK, std::string());
+  now = base::TimeTicks::Now();
+  rvh2->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  TestRenderViewHost* rvh3 =
+      static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
+
+  // Simulate rvh3's response, which leads to an unload request being sent to
+  // rvh2.
+  std::vector<GURL> url_chain2;
+  url_chain2.push_back(GURL());
+  contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
+      rvh3,
+      GlobalRequestID(0, 0),
+      false,
+      url_chain2,
+      Referrer(),
+      PAGE_TRANSITION_TYPED,
+      1,
+      false);
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
+            rvh2->rvh_state());
+
+  // Simulate the swap out ack.
+  rvh2->OnSwappedOut(false);
+  EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_COMMIT, rvh2->rvh_state());
+
+  // The new page commits.
+  contents()->TestDidNavigate(rvh3, 1, kUrl2, PAGE_TRANSITION_TYPED);
+  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_EQ(rvh3, rvh());
+  EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE, rvh3->rvh_state());
+
+  // rvh2 should be swapped out.
+  EXPECT_FALSE(destruction_checker2.rvh_deleted());
+  EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, rvh2->rvh_state());
+}
+
+// Tests that the RenderViewHost is properly swapped out or deleted when the new
+// page commits befoer the swap out ack is received.
+TEST_F(RenderFrameHostManagerTest, NewPageCommitsBeforeSwapOutACK) {
+  const GURL kUrl1("http://www.google.com/");
+  const GURL kUrl2("http://www.chromium.org/");
+
+  // Navigate to the first page.
+  contents()->NavigateAndCommit(kUrl1);
+  TestRenderViewHost* rvh1 = test_rvh();
+  RenderViewHostDestructionChecker destruction_checker1(rvh1);
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE, rvh1->rvh_state());
+
+  // Navigate to new site, simulating onbeforeunload approval.
+  controller().LoadURL(kUrl2, Referrer(), PAGE_TRANSITION_LINK, std::string());
+  base::TimeTicks now = base::TimeTicks::Now();
+  rvh1->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  TestRenderViewHost* rvh2 =
+      static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
+
+  // Simulate rvh2's response, which leads to an unload request being sent to
+  // rvh1.
+  std::vector<GURL> url_chain1;
+  url_chain1.push_back(GURL());
+  contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
+      rvh2,
+      GlobalRequestID(0, 0),
+      false,
+      url_chain1,
+      Referrer(),
+      PAGE_TRANSITION_TYPED,
+      1,
+      false);
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
+            rvh1->rvh_state());
+
+  // The new page commits.
+  contents()->TestDidNavigate(rvh2, 1, kUrl2, PAGE_TRANSITION_TYPED);
+  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_EQ(rvh2, rvh());
+  EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE, rvh2->rvh_state());
+  EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SHUTDOWN, rvh1->rvh_state());
+
+  // Simulate the swap out ack.
+  rvh1->OnSwappedOut(false);
+
+  // rvh1 should have been deleted.
+  EXPECT_TRUE(destruction_checker1.rvh_deleted());
+  rvh1 = NULL;
+
+  // Increment the number of active views in SiteInstanceImpl so that rvh2 is
+  // not deleted on swap out.
+  static_cast<SiteInstanceImpl*>(
+      rvh2->GetSiteInstance())->increment_active_view_count();
+  RenderViewHostDestructionChecker destruction_checker2(rvh2);
+
+  // Navigate to new site, simulating onbeforeunload approval.
+  controller().LoadURL(kUrl1, Referrer(), PAGE_TRANSITION_LINK, std::string());
+  now = base::TimeTicks::Now();
+  rvh2->OnMessageReceived(ViewHostMsg_ShouldClose_ACK(0, true, now, now));
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  TestRenderViewHost* rvh3 =
+      static_cast<TestRenderViewHost*>(contents()->GetPendingRenderViewHost());
+
+  // Simulate rvh3's response, which leads to an unload request being sent to
+  // rvh2.
+  std::vector<GURL> url_chain2;
+  url_chain2.push_back(GURL());
+  contents()->GetRenderManagerForTesting()->OnCrossSiteResponse(
+      rvh3,
+      GlobalRequestID(0, 0),
+      false,
+      url_chain2,
+      Referrer(),
+      PAGE_TRANSITION_TYPED,
+      1,
+      false);
+  EXPECT_TRUE(contents()->cross_navigation_pending());
+  EXPECT_EQ(RenderViewHostImpl::STATE_WAITING_FOR_UNLOAD_ACK,
+            rvh2->rvh_state());
+
+  // The new page commits.
+  contents()->TestDidNavigate(rvh3, 1, kUrl2, PAGE_TRANSITION_TYPED);
+  EXPECT_FALSE(contents()->cross_navigation_pending());
+  EXPECT_EQ(rvh3, rvh());
+  EXPECT_TRUE(contents()->GetPendingRenderViewHost() == NULL);
+  EXPECT_EQ(RenderViewHostImpl::STATE_LIVE, rvh3->rvh_state());
+  EXPECT_EQ(RenderViewHostImpl::STATE_PENDING_SWAP_OUT, rvh2->rvh_state());
+
+  // Simulate the swap out ack.
+  rvh2->OnSwappedOut(false);
+
+  // rvh2 should be swapped out.
+  EXPECT_FALSE(destruction_checker2.rvh_deleted());
+  EXPECT_EQ(RenderViewHostImpl::STATE_SWAPPED_OUT, rvh2->rvh_state());
+}
+
 }  // namespace content