Fix 3 crashes related to navigations after a process dies.

content/browser/site_per_process_browsertest.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/site_per_process_browsertest.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 4082 matching lines @@
   EXPECT_EQ(expected_title, title_watcher.WaitAndGetTitle());
 }
 
 // Test for https://crbug.com/515302.  Perform two navigations, A->B->A, and
 // delay the SwapOut ACK from the A->B navigation, so that the second B->A
 // navigation is initiated before the first page receives the SwapOut ACK.
 // Ensure that the RVH(A) that's pending deletion is not reused in that case.
 // crbug.com/554825
 #if defined(THREAD_SANITIZER)
 #define MAYBE_RenderViewHostPendingDeletionIsNotReused \
         DISABLED_RenderViewHostPendingDeletionIsNotReused

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

There's a chance I fixed the problem in https://crbug.com/554825.  I'll try
re-enabling it after this CL lands.

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

Agreed that you may have fixed it -- and sounds good to try re-enabling
afterward.  Thanks!

 #else
 #define MAYBE_RenderViewHostPendingDeletionIsNotReused \
         RenderViewHostPendingDeletionIsNotReused
 #endif
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
                        MAYBE_RenderViewHostPendingDeletionIsNotReused) {
   GURL a_url(embedded_test_server()->GetURL("a.com", "/title1.html"));
   NavigateToURL(shell(), a_url);
 
   FrameTreeNode* root = static_cast<WebContentsImpl*>(shell()->web_contents())
                             ->GetFrameTree()
                             ->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());
 
   // Navigate to B.  This must wait for DidCommitProvisionalLoad, as opposed to
   // DidStopLoading, since otherwise the SwapOut timer might call OnSwappedOut
   // and destroy |rvh| before its pending deletion status is checked.
   GURL b_url(embedded_test_server()->GetURL("b.com", "/title2.html"));
   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.

   RenderProcessHostWatcher process_exit_observer(
       rvh->GetProcess(),
       RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
   process_exit_observer.Wait();
 
   // Start a navigation back to A and check that the RenderViewHost wasn't
   // reused.
   TestNavigationObserver navigation_observer(shell()->web_contents());
   shell()->LoadURL(a_url);
   RenderViewHostImpl* pending_rvh =
       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
 // and then back to root frame.
 IN_PROC_BROWSER_TEST_F(SitePerProcessBrowserTest,
                        CrossProcessFocusChangeFiresBlurEvents) {
   GURL main_url(
       embedded_test_server()->GetURL("a.com", "/page_with_input_field.html"));
   EXPECT_TRUE(NavigateToURL(shell(), main_url));
 
@@ skipping 2080 matching lines @@
   EXPECT_EQ(b_url, root->current_url());
 
   // Verify that the same RenderViewHost is preserved and that it is no longer
   // in swapped out state.
   EXPECT_EQ(rvh, contents->GetFrameTree()->GetRenderViewHost(
                      root->current_frame_host()->GetSiteInstance()));
   EXPECT_FALSE(rvh->is_swapped_out_);
 }
 
 }  // namespace content