Properly recreate swapped out RenderView.

content/browser/frame_host/render_frame_host_manager_browsertest.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 <set>
 
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "base/location.h"
 #include "base/memory/ref_counted.h"
@@ skipping 2145 matching lines @@
   // The test must wait for a process to exit, but if the IPC message is
   // properly ignored, there will be no crash. Therefore, navigate the
   // original window to the same site as the popup, which will just exit the
   // process cleanly.
   EXPECT_TRUE(NavigateToURL(
       shell(), embedded_test_server()->GetURL("b.com", "/title3.html")));
   watcher.Wait();
   EXPECT_TRUE(watcher.did_exit_normally());
 }
 
+// Tests navigating cross-process and reusing an existing RenderViewHost,

[Charlie Reis, 2015/10/16 17:02:23]

nit: Tests that

[nasko, 2015/10/16 22:17:38]

Done.

+// which process has been killed/crashed, doesn't result in creating

[Charlie Reis, 2015/10/16 17:02:23]

nit: Put the killed/crash phrase in parentheses, not commas.
nit: s/which/whose/

[nasko, 2015/10/16 22:17:38]

Done.

+// a RenderView with no valid RenderFrame or RenderFrameProxy.

[Charlie Reis, 2015/10/16 17:02:24]

nit: Mention https://crbug.com/543222 here.

[nasko, 2015/10/16 22:17:38]

I ended up filing a master bug (https://crbug.com/544271) and explains the root
cause of the bug much better than the actual crashes.

+IN_PROC_BROWSER_TEST_F(RenderFrameHostManagerTest,
+                       RenderViewInitAfterProcessKill) {
+  StartEmbeddedServer();
+  NavigateToURL(shell(),
+                embedded_test_server()->GetURL("a.com", "/title1.html"));
+
+  // Open a popup to navigate.
+  Shell* new_shell =
+      OpenPopup(shell()->web_contents(), GURL(url::kAboutBlankURL), "foo");
+  EXPECT_EQ(shell()->web_contents()->GetSiteInstance(),
+            new_shell->web_contents()->GetSiteInstance());
+  WebContentsImpl* new_web_contents =
+      static_cast<WebContentsImpl*>(new_shell->web_contents());
+  FrameTreeNode* popup_root = new_web_contents->GetFrameTree()->root();
+  RenderViewHostImpl* rvh = new_web_contents->GetRenderViewHost();
+
+  // Navigate the popup to a different site.
+  NavigateToURL(new_shell,
+                embedded_test_server()->GetURL("b.com", "/title2.html"));
+  EXPECT_NE(shell()->web_contents()->GetSiteInstance(),
+            new_shell->web_contents()->GetSiteInstance());
+  EXPECT_EQ(rvh, popup_root->render_manager()->GetSwappedOutRenderViewHost(
+                     shell()->web_contents()->GetSiteInstance()));

[Charlie Reis, 2015/10/16 17:02:24]

I'm not sure I understand the bug, but it seems like the part we care about is
that shell() has a swapped out RVH for b.com (rather than the RVH for a.com in
the popup).  According to the comments, that's the one that causes the crash,
right?

[nasko, 2015/10/16 22:17:38]

Yes, you are correct. This RVH was a copy/paste from another test. Removed.

+
+  // Kill the process for the main tab.
+  RenderProcessHost* process = popup_root->current_frame_host()->GetProcess();

[Charlie Reis, 2015/10/16 17:02:23]

Wait, are the comments wrong?  This is killing the popup's process, not the main
tab.

[nasko, 2015/10/16 22:17:38]

Yes, fixed.

+  RenderProcessHostWatcher crash_observer(
+      process, RenderProcessHostWatcher::WATCH_FOR_PROCESS_EXIT);
+  process->Shutdown(0, false);
+  crash_observer.Wait();
+  EXPECT_FALSE(popup_root->current_frame_host()->IsRenderFrameLive());
+  EXPECT_FALSE(
+      popup_root->current_frame_host()->render_view_host()->IsRenderViewLive());
+
+  // Navigate the main tab to the site of the popup. This will cause the
+  // RenderView for b.com in the main tab to be created.

[Charlie Reis, 2015/10/16 17:02:23]

Can you add some explanation of what this is verifying?  Maybe mention that this
used to crash because the RenderView was created without a frame or a proxy, if
that's what was happening.

Also, does this actually fail without your fix?  I know a lot of tests pass even
if the renderer crashes.  If it does fail, mention how.

[nasko, 2015/10/16 22:17:38]

The process crashes, so we don't complete the navigation, therefore
NavigateToURL fails. I'll double check again.

+  NavigateToURL(shell(),
+                embedded_test_server()->GetURL("b.com", "/title3.html"));
+}
+
 }  // namespace content