Do not filter IPC messages until the new RVH commits, to avoid a race.

content/browser/tab_contents/render_view_host_manager_unittest.cc

 // Copyright (c) 2012 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 "content/browser/browser_thread_impl.h"
 #include "content/browser/browser_url_handler.h"
 #include "content/browser/mock_content_browser_client.h"
 #include "content/browser/renderer_host/test_render_view_host.h"
 #include "content/browser/site_instance.h"
 #include "content/browser/tab_contents/navigation_controller_impl.h"
@@ skipping 410 matching lines @@
   EXPECT_FALSE(host->site_instance()->has_site());
   host->site_instance()->SetSite(kUrl1);
 
   // 2) Cross-site navigate to next site. -------------------------
   const GURL kUrl2("http://www.example.com");
   NavigationEntryImpl entry2(
       NULL /* instance */, -1 /* page_id */, kUrl2, content::Referrer(),
       string16() /* title */, content::PAGE_TRANSITION_TYPED,
       false /* is_renderer_init */);
   RenderViewHost* host2 = manager.Navigate(entry2);
+  int host2_process_id = host2->process()->GetID();
 
   // A new RenderViewHost should be created.
   EXPECT_TRUE(manager.pending_render_view_host());
   ASSERT_EQ(host2, manager.pending_render_view_host());
+  EXPECT_NE(host2, host);
 
   // Check that the navigation is still suspended because the old RVH
   // is not swapped out, yet.
+  EXPECT_TRUE(host2->are_navigations_suspended());
   MockRenderProcessHost* test_process_host2 =
       static_cast<MockRenderProcessHost*>(host2->process());
   test_process_host2->sink().ClearMessages();
   host2->NavigateToURL(kUrl2);
   EXPECT_FALSE(test_process_host2->sink().GetUniqueMessageMatching(
       ViewMsg_Navigate::ID));
 
   // Allow closing the current Render View (precondition for swapping out
   // the RVH): Simulate response from RenderView for ViewMsg_ShouldClose sent by
-  // FirePageBeforeUnload
+  // FirePageBeforeUnload.
   TestRenderViewHost* test_host = static_cast<TestRenderViewHost*>(host);
   MockRenderProcessHost* test_process_host =
       static_cast<MockRenderProcessHost*>(test_host->process());
   EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
       ViewMsg_ShouldClose::ID));
   test_host->SendShouldCloseACK(true);
 
-  // CrossSiteResourceHandler::StartCrossSiteTransition can trigger a
+  // CrossSiteResourceHandler::StartCrossSiteTransition triggers a
   // call of RenderViewHostManager::OnCrossSiteResponse before
-  // RenderViewHostManager::DidNavigateMainFrame is called. In this case the
-  // RVH is swapped out.
+  // RenderViewHostManager::DidNavigateMainFrame is called.
+  // The RVH is not swapped out until the commit.
   manager.OnCrossSiteResponse(host2->process()->GetID(),
                               host2->GetPendingRequestId());
   EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
       ViewMsg_SwapOut::ID));
   test_host->OnSwapOutACK();
 
   EXPECT_EQ(host, manager.current_host());
-  EXPECT_TRUE(manager.current_host()->is_swapped_out());
+  EXPECT_FALSE(manager.current_host()->is_swapped_out());
   EXPECT_EQ(host2, manager.pending_render_view_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,
                              content::Referrer(), string16() /* title */,
                              content::PAGE_TRANSITION_TYPED,
                              false /* is_renderer_init */);
+  test_process_host->sink().ClearMessages();
   RenderViewHost* host3 = manager.Navigate(entry3);
 
-  // A new RenderViewHost should be created.
+  // A new RenderViewHost should be created. host2 is now deleted.
   EXPECT_TRUE(manager.pending_render_view_host());
   ASSERT_EQ(host3, manager.pending_render_view_host());
+  EXPECT_NE(host3, host);
+  EXPECT_NE(host3->process()->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.
+  EXPECT_TRUE(host3->are_navigations_suspended());
   EXPECT_EQ(host, manager.current_host());
-  EXPECT_TRUE(manager.current_host()->is_swapped_out());
+  EXPECT_FALSE(manager.current_host()->is_swapped_out());
 
-  // The navigation should not be suspended because the RVH |host| has been
-  // swapped out already. Therefore, the RVH should send a navigation event
-  // immediately.
-  MockRenderProcessHost* test_process_host3 =
-      static_cast<MockRenderProcessHost*>(host3->process());
-  test_process_host3->sink().ClearMessages();
+  // Simulate a response to the second beforeunload request.
+  EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
+      ViewMsg_ShouldClose::ID));
+  test_host->SendShouldCloseACK(true);
 
-  // Usually TabContents::NavigateToEntry would call
-  // RenderViewHostManager::Navigate followed by RenderViewHost::Navigate.
-  // Here we need to call the latter ourselves.
-  host3->NavigateToURL(kUrl3);
-  EXPECT_TRUE(test_process_host3->sink().GetUniqueMessageMatching(
-      ViewMsg_Navigate::ID));
+  // CrossSiteResourceHandler::StartCrossSiteTransition triggers a
+  // call of RenderViewHostManager::OnCrossSiteResponse before
+  // RenderViewHostManager::DidNavigateMainFrame is called.
+  // The RVH is not swapped out until the commit.
+  manager.OnCrossSiteResponse(host3->process()->GetID(),
+                              host3->GetPendingRequestId());
+  EXPECT_TRUE(test_process_host->sink().GetUniqueMessageMatching(
+      ViewMsg_SwapOut::ID));
+  test_host->OnSwapOutACK();
 
   // Commit.
   manager.DidNavigateMainFrame(host3);
   EXPECT_TRUE(host3 == manager.current_host());
   ASSERT_TRUE(host3);
   EXPECT_TRUE(host3->site_instance()->has_site());
   // Check the pending RenderViewHost has been committed.
   EXPECT_FALSE(manager.pending_render_view_host());
 
   // We should observe a notification.
@@ skipping 105 matching lines @@
   // current one.
   EXPECT_TRUE(contents()->GetRenderManagerForTesting()->
       pending_render_view_host() == NULL);
   EXPECT_EQ(evil_rvh, contents()->GetRenderManagerForTesting()->current_host());
 
   // Also we should not have a pending navigation entry.
   NavigationEntry* entry = contents()->GetController().GetActiveEntry();
   ASSERT_TRUE(entry != NULL);
   EXPECT_EQ(kUrl2, entry->GetURL());
 }