Fixing a problem, where a hung renderer process is not killed when navigating away

content/browser/renderer_host/render_process_host_browsertest.cc

Index: content/browser/renderer_host/render_process_host_browsertest.cc
diff --git a/content/browser/renderer_host/render_process_host_browsertest.cc b/content/browser/renderer_host/render_process_host_browsertest.cc
index 50de34073b0031c3496facd661518a1230b6af2c..e6195617e5f434bf1cac01bb8346bd6ed9b17dbe 100644
--- a/content/browser/renderer_host/render_process_host_browsertest.cc
+++ b/content/browser/renderer_host/render_process_host_browsertest.cc
@@ -15,6 +15,7 @@
 #include "content/common/test_url_constants.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
+#include "net/test/test_server.h"
 
 using content::WebContents;
 
@@ -252,3 +253,59 @@ IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, ProcessOverflow) {
 IN_PROC_BROWSER_TEST_F(RenderProcessHostTestWithCommandLine, ProcessOverflow) {
   TestProcessOverflow();
 }
+
+IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, UnresponsiveCrossSiteNavigation) {
+  WebContents* tab = NULL;
+  WebContents* tab2 = NULL;
+  content::RenderProcessHost* rph = NULL;
+  base::ProcessHandle ph = NULL;

[Charlie Reis, 2012/03/01 21:08:17]

nit: Rename to process.

[nasko, 2012/03/01 22:50:15]

Done.

+
+  // Start two servers to enable cross-site navigations.
+  ASSERT_TRUE(test_server()->Start());
+  net::TestServer https_server(
+      net::TestServer::TYPE_HTTPS,
+      net::TestServer::kLocalhost,
+      FilePath(FILE_PATH_LITERAL("chrome/test/data")));
+  ASSERT_TRUE(https_server.Start());
+
+  GURL infinite_url(test_server()->GetURL("files/infinite_beforeunload.html"));

[Charlie Reis, 2012/03/01 21:08:17]

Why did you choose to hang in beforeunload instead of unload?  If there's a
reason, might be good to have a comment.

[nasko, 2012/03/01 22:50:15]

No real reason. After the latest discussions, it might actually be better to
hang on the unload handler, so I made the second navigation use the
beforeunload.

+  GURL same_process_url(test_server()->GetURL("files/english_page.html"));
+  GURL regular_url(https_server.GetURL("files/english_page.html"));

[Charlie Reis, 2012/03/01 21:08:17]

What does regular refer to?  Maybe new_process_url?

[nasko, 2012/03/01 22:50:15]

Done.

+
+  // Navigate the tab to the page which will lock up the process.

[Charlie Reis, 2012/03/01 21:08:17]

nit: ...when we navigate away.

[nasko, 2012/03/01 22:50:15]

Done.

+  ui_test_utils::NavigateToURL(browser(), infinite_url);
+  tab = browser()->GetWebContentsAt(0);
+  rph = tab->GetRenderProcessHost();
+  EXPECT_EQ(tab->GetURL(), infinite_url);
+
+  // Remember the process prior to navigation, as we expect it to get killed.
+  ph = rph->GetHandle();
+  ASSERT_TRUE(ph);
+
+  ui_test_utils::NavigateToURL(browser(), regular_url);
+  EXPECT_FALSE(base::KillProcess(ph, 1, false));

[Charlie Reis, 2012/03/01 21:08:17]

Add a comment that this should fail because the process should already be
killed, since it's sort of an indirect way to check that it's gone.

[nasko, 2012/03/01 22:50:15]

Done.

+
+  // Now, let's load the unresponsive page in one tab, then open another tab
+  // which will use the same process.
+  ui_test_utils::NavigateToURL(browser(), infinite_url);
+  tab = browser()->GetWebContentsAt(0);
+  rph = tab->GetRenderProcessHost();
+  EXPECT_EQ(tab->GetURL(), infinite_url);
+
+  ui_test_utils::NavigateToURLWithDisposition(browser(), same_process_url,
+      NEW_BACKGROUND_TAB, ui_test_utils::BROWSER_TEST_WAIT_FOR_NAVIGATION);
+  EXPECT_EQ(browser()->tab_count(), 2);
+  tab2 = browser()->GetWebContentsAt(1);
+  ASSERT_TRUE(tab2 != NULL);
+  EXPECT_EQ(tab2->GetURL(), same_process_url);
+  EXPECT_EQ(rph, tab2->GetRenderProcessHost());
+
+  ph = rph->GetHandle();
+  ASSERT_TRUE(ph);
+
+  // Navigating to the cross site URL will not kill the process, since it will
+  // have more than one tab using it. Kill it to confirm that it is still there,
+  // as well as finish the test faster.
+  ui_test_utils::NavigateToURL(browser(), regular_url);
+  EXPECT_TRUE(base::KillProcess(ph, 1, false));
+}