summaryrefslogtreecommitdiffstats
path: root/content
diff options
context:
space:
mode:
authorcreis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-12 20:12:58 +0000
committercreis@chromium.org <creis@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-03-12 20:12:58 +0000
commit8c0eb70b0f944127475e420765286ac082645dd4 (patch)
tree447fc228b9657e2c759a93b4b2e4c1ad42a2cf50 /content
parent95102f6e9af498512938d4853fe0a22426ab47b1 (diff)
downloadchromium_src-8c0eb70b0f944127475e420765286ac082645dd4.zip
chromium_src-8c0eb70b0f944127475e420765286ac082645dd4.tar.gz
chromium_src-8c0eb70b0f944127475e420765286ac082645dd4.tar.bz2
Fixing a problem, where a hung renderer process is not killed when navigating away
BUG=104346 TEST=Steps to reproduce listed in the bug. Review URL: https://chromiumcodereview.appspot.com/9514016 Patch from Nasko Oskov <nasko@chromium.org>. git-svn-id: svn://svn.chromium.org/chrome/trunk/src@126199 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
-rw-r--r--content/browser/renderer_host/render_process_host_browsertest.cc68
-rw-r--r--content/browser/renderer_host/render_view_host_impl.cc40
-rw-r--r--content/test/data/english_page.html7
-rw-r--r--content/test/data/infinite_beforeunload.html14
-rw-r--r--content/test/data/infinite_unload.html14
5 files changed, 143 insertions, 0 deletions
diff --git a/content/browser/renderer_host/render_process_host_browsertest.cc b/content/browser/renderer_host/render_process_host_browsertest.cc
index 50de340..4ea27e9 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,70 @@ 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 process;
+ FilePath doc_root;
+
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("content"));
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("test"));
+ doc_root = doc_root.Append(FILE_PATH_LITERAL("data"));
+
+ // Start two servers to enable cross-site navigations.
+ net::TestServer server(net::TestServer::TYPE_HTTP,
+ net::TestServer::kLocalhost, doc_root);
+ ASSERT_TRUE(server.Start());
+ net::TestServer https_server(net::TestServer::TYPE_HTTPS,
+ net::TestServer::kLocalhost, doc_root);
+ ASSERT_TRUE(https_server.Start());
+
+ GURL infinite_beforeunload_url(
+ server.GetURL("files/infinite_beforeunload.html"));
+ GURL infinite_unload_url(server.GetURL("files/infinite_unload.html"));
+ GURL same_process_url(server.GetURL("files/english_page.html"));
+ GURL new_process_url(https_server.GetURL("files/english_page.html"));
+
+ // Navigate the tab to the page which will lock up the process when we
+ // navigate away from it.
+ ui_test_utils::NavigateToURL(browser(), infinite_unload_url);
+ tab = browser()->GetWebContentsAt(0);
+ rph = tab->GetRenderProcessHost();
+ EXPECT_EQ(tab->GetURL(), infinite_unload_url);
+
+ // Remember the process prior to navigation, as we expect it to get killed.
+ process = rph->GetHandle();
+ ASSERT_TRUE(process);
+
+ ui_test_utils::NavigateToURL(browser(), new_process_url);
+ // This should fail, because the navigation to the new URL would result in
+ // the process getting killed. This is an indirect way to check for the
+ // process having been terminated.
+ EXPECT_FALSE(base::KillProcess(process, 1, false));
+
+ // 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_beforeunload_url);
+ tab = browser()->GetWebContentsAt(0);
+ rph = tab->GetRenderProcessHost();
+ EXPECT_EQ(tab->GetURL(), infinite_beforeunload_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());
+
+ process = rph->GetHandle();
+ ASSERT_TRUE(process);
+
+ // 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(), new_process_url);
+ EXPECT_TRUE(base::KillProcess(process, 1, false));
+}
diff --git a/content/browser/renderer_host/render_view_host_impl.cc b/content/browser/renderer_host/render_view_host_impl.cc
index daf17c7..cf2593f 100644
--- a/content/browser/renderer_host/render_view_host_impl.cc
+++ b/content/browser/renderer_host/render_view_host_impl.cc
@@ -41,6 +41,7 @@
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
+#include "content/public/browser/render_process_host.h"
#include "content/public/browser/render_view_host_delegate.h"
#include "content/public/browser/render_view_host_observer.h"
#include "content/public/browser/user_metrics.h"
@@ -418,6 +419,12 @@ void RenderViewHostImpl::WasSwappedOut() {
// Don't bother reporting hung state anymore.
StopHangMonitorTimeout();
+ // If we are still waiting on the unload handler to be run, we consider
+ // the process hung and we should terminate it if there are no other tabs
+ // using the process. If there are other views using this process, the
+ // unresponsive renderer timeout will catch it.
+ bool hung = is_waiting_for_unload_ack_;
+
// Now that we're no longer the active RVH in the tab, start filtering out
// most IPC messages. Usually the renderer will have stopped sending
// messages as of OnSwapOutACK. However, we may have timed out waiting
@@ -426,6 +433,39 @@ void RenderViewHostImpl::WasSwappedOut() {
// still allow synchronous messages through).
SetSwappedOut(true);
+ // If we are not running the renderer in process and no other tab is using
+ // the hung process, kill it, assuming it is a real process (unit tests don't
+ // have real processes).
+ if (hung) {
+ base::ProcessHandle process_handle = GetProcess()->GetHandle();
+ int views = 0;
+
+ // Count the number of listeners for the process, which is equivalent to
+ // views using the process as of this writing.
+ content::RenderProcessHost::listeners_iterator iter(
+ GetProcess()->ListenersIterator());
+ for (; !iter.IsAtEnd(); iter.Advance())
+ ++views;
+
+ if (!content::RenderProcessHost::run_renderer_in_process() &&
+ process_handle && views <= 1) {
+ // We expect the delegate for this RVH to be TabContents, as it is the
+ // only class that swaps out render view hosts on navigation.
+ DCHECK(delegate_->GetRenderViewType() == content::VIEW_TYPE_TAB_CONTENTS);
+
+ // Kill the process only if TabContents sets SuddenTerminationAllowed,
+ // which indicates that the timer has expired.
+ // This is not the case if we load data URLs or about:blank. The reason
+ // is that there is no network requests and this code is hit without
+ // setting the unresponsiveness timer. This allows a corner case where a
+ // navigation to a data URL will leave a process running, if the
+ // beforeunload handler completes fine, but the unload handler hangs.
+ // At this time, the complexity to solve this edge case is not worthwhile.
+ if (SuddenTerminationAllowed())
+ base::KillProcess(process_handle, content::RESULT_CODE_HUNG, false);
+ }
+ }
+
// Inform the renderer that it can exit if no one else is using it.
Send(new ViewMsg_WasSwappedOut(GetRoutingID()));
}
diff --git a/content/test/data/english_page.html b/content/test/data/english_page.html
new file mode 100644
index 0000000..30b0d42
--- /dev/null
+++ b/content/test/data/english_page.html
@@ -0,0 +1,7 @@
+<html>
+<head><title>This page is in English</title></head>
+<body>
+No joke! This is a page written in English. Awesome don't you think?
+It has to be more than 100 bytes long for the CLD to detect the language correctly.
+</body>
+</html>
diff --git a/content/test/data/infinite_beforeunload.html b/content/test/data/infinite_beforeunload.html
new file mode 100644
index 0000000..785ad0f
--- /dev/null
+++ b/content/test/data/infinite_beforeunload.html
@@ -0,0 +1,14 @@
+<html>
+<head>
+ <title>Infinite beforeunload</title>
+</head>
+
+<body>
+<script>
+ window.onbeforeunload = function(e) {
+ while(true){}
+ }
+</script>
+</body>
+
+</html>
diff --git a/content/test/data/infinite_unload.html b/content/test/data/infinite_unload.html
new file mode 100644
index 0000000..671250d
--- /dev/null
+++ b/content/test/data/infinite_unload.html
@@ -0,0 +1,14 @@
+<html>
+<head>
+ <title>Infinite unload</title>
+</head>
+
+<body>
+<script>
+ window.onunload = function(e) {
+ while(true){}
+ }
+</script>
+</body>
+
+</html>