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..64de389bcad54b8c1996d87b84100b1070901118 100644
--- a/content/browser/site_per_process_browsertest.cc
+++ b/content/browser/site_per_process_browsertest.cc
@@ -4115,10 +4115,13 @@ 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();

[Charlie Reis, 2016/03/28 19:06:37]

Have to grab these in advance to avoid a UaF below.

   RenderFrameDeletedObserver deleted_observer(rfh);
 
   // Install a BrowserMessageFilter to drop SwapOut ACK messages in A's
   // process.
+  // TODO(creis): This will be flaky, since there's still a 1 second timeout.

[Charlie Reis, 2016/03/28 19:06:37]

Oops, I forgot to take this TODO out.  I fixed this on line 4136.

[Charlie Reis, 2016/03/29 04:19:01]

Done.

   scoped_refptr<SwapoutACKMessageFilter> filter = new SwapoutACKMessageFilter();
   rfh->GetProcess()->AddFilter(filter.get());
 
@@ -4129,12 +4132,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.

[Charlie Reis, 2016/03/28 19:06:37]

I'd like to remove this as well, but I think this CL tackles enough bugs
already.  :)

Given my fix to clear the pending delete RFHs when the process dies, it means
that rfh is gone after line 4149 and we can't interact with it below.  We can
clean that up as part of the 535246 bug fix.

@@ -4151,18 +4156,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));
+}
+
 // 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