Fix 3 crashes related to navigations after a process dies.

content/browser/site_per_process_browsertest.cc

Index: content/browser/site_per_process_browsertest.cc
diff --git a/content/browser/site_per_process_browsertest.cc b/content/browser/site_per_process_browsertest.cc
index fbe9ae68513fffb6292d25d614329c53cde70878..4511d3480c19a34283df55dca9b2441244831baa 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -4115,6 +4115,8 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
                             ->root();
   RenderFrameHostImpl* rfh = root->current_frame_host();
   RenderViewHostImpl* rvh = rfh->render_view_host();
+  int rvh_routing_id = rvh->GetRoutingID();
+  SiteInstanceImpl* site_instance = rfh->GetSiteInstance();
   RenderFrameDeletedObserver deleted_observer(rfh);
 
   // Install a BrowserMessageFilter to drop SwapOut ACK messages in A's
@@ -4129,12 +4131,14 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
   TestFrameNavigationObserver commit_observer(root);
   shell()->LoadURL(b_url);
   commit_observer.WaitForCommit();
+  EXPECT_FALSE(deleted_observer.deleted());
+  rfh->ResetSwapOutTimerForTesting();
 
   // Since the SwapOut ACK for A->B is dropped, the first page's
   // RenderFrameHost and RenderViewHost should be pending deletion after the
   // last navigation.
   EXPECT_TRUE(root->render_manager()->IsPendingDeletion(rfh));
-  EXPECT_TRUE(rvh->is_pending_deletion());
+  EXPECT_TRUE(root->render_manager()->IsViewPendingDeletion(rvh));
 
   // Wait for process A to exit so we can reinitialize it cleanly for the next
   // navigation. This can be removed once https://crbug.com/535246 is fixed.
@@ -4151,18 +4155,39 @@ IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
       IsBrowserSideNavigationEnabled()
           ? root->render_manager()->speculative_frame_host()->render_view_host()
           : root->render_manager()->pending_render_view_host();
-  EXPECT_EQ(rvh->GetSiteInstance(), pending_rvh->GetSiteInstance());
-  EXPECT_NE(rvh, pending_rvh);
+  EXPECT_EQ(site_instance, pending_rvh->GetSiteInstance());
+  EXPECT_NE(rvh_routing_id, pending_rvh->GetRoutingID());
 
-  // Simulate that the dropped SwapOut ACK message arrives now on the original
-  // RenderFrameHost, which should now get deleted.
-  rfh->OnSwappedOut();
+  // TODO(alexmos, creis): Once https://crbug.com/535246 is fixed and the
+  // process_exit_observer is not needed above, we'll need to simulate that the
+  // dropped SwapOut ACK message arrives now on the original RenderFrameHost,
+  // causing it to be deleted.
   EXPECT_TRUE(deleted_observer.deleted());
 
   // Make sure the last navigation finishes without crashing.
   navigation_observer.Wait();
 }
 
+// Test for https://crbug.com/591478, where navigating to a cross-site page with
+// a subframe on the old site could cause the old RenderViewHost (now pending
+// deletion) to be reused.
+IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
+                       DontReusePendingDeleteRenderViewHostForSubframe) {
+  GURL main_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
+  EXPECT_TRUE(NavigateToURL(shell(), main_url));
+
+  std::string script =
+      "window.onunload = function() { "
+      "  var start = Date.now();"
+      "  while (Date.now() - start < 1000);"
+      "}";
+  EXPECT_TRUE(ExecuteScript(shell()->web_contents(), script));
+
+  GURL second_url(embedded_test_server()->GetURL(
+      "b.com", "/cross_site_iframe_factory.html?b(a)"));
+  EXPECT_TRUE(NavigateToURL(shell(), second_url));

[alexmos, 2016/03/29 18:43:02]

There's a chance that the script will finish before we get to create the
subframe and we won't be testing what we want to test, but I guess this is ok --
it won't cause the test to fail and navigations such as this should take way
less than 1 second on the bots, right?

Another thought is we could compare the new and old RVH routing IDs and make
sure they are different, similarly to the test above.  Probably ok either way,
given that we already have similar coverage above.

[Charlie Reis, 2016/03/29 20:29:03]

Yeah, there's a chance it could be a flaky failure if a regression happened, but
it should be pretty likely to fail in that case.  We can't use while(1) since
then the subframe will never commit (given that it ends up in the same process,
but a different RVH).  I think I'm ok with it as is.
Good idea.  Done.

+}
+
 // Check that when a cross-process frame acquires focus, the old focused frame
 // loses focus and fires blur events.  Starting on a page with a cross-site
 // subframe, simulate mouse clicks to switch focus from root frame to subframe