summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorerikkay@chromium.org <erikkay@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-07-11 17:14:48 +0000
committererikkay@chromium.org <erikkay@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2010-07-11 17:14:48 +0000
commit08c8ec7e386525210a0ff931e8a3e8e9678db398 (patch)
tree2cb862942569dcacfcec87fe1214df4a2beaffee
parent64fb9acba969a51fc806df5ae8e3e60e05994b78 (diff)
downloadchromium_src-08c8ec7e386525210a0ff931e8a3e8e9678db398.zip
chromium_src-08c8ec7e386525210a0ff931e8a3e8e9678db398.tar.gz
chromium_src-08c8ec7e386525210a0ff931e8a3e8e9678db398.tar.bz2
Add unit test and supporting code to test process overflow case.
This test is disabled for now since we don't have a fix yet. The point of checkin in this CL is to make it easier for us to test alternate approaches to fixing. BUG=43448 TEST=RenderProcessHostTest.ProcessOverflow Review URL: http://codereview.chromium.org/2928004 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52059 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--chrome/browser/renderer_host/render_process_host.cc11
-rw-r--r--chrome/browser/renderer_host/render_process_host.h5
-rw-r--r--chrome/browser/renderer_host/site_instance.cc28
-rw-r--r--chrome/browser/renderer_host/site_instance.h3
-rw-r--r--chrome/browser/renderer_host/test/render_process_host_browsertest.cc115
-rw-r--r--chrome/chrome_tests.gypi1
6 files changed, 157 insertions, 6 deletions
diff --git a/chrome/browser/renderer_host/render_process_host.cc b/chrome/browser/renderer_host/render_process_host.cc
index 5f9363e..959252e 100644
--- a/chrome/browser/renderer_host/render_process_host.cc
+++ b/chrome/browser/renderer_host/render_process_host.cc
@@ -13,7 +13,12 @@
namespace {
+size_t max_renderer_count_override = 0;
+
size_t GetMaxRendererProcessCount() {
+ if (max_renderer_count_override)
+ return max_renderer_count_override;
+
// Defines the maximum number of renderer processes according to the
// amount of installed memory as reported by the OS. The table
// values are calculated by assuming that you want the renderers to
@@ -74,8 +79,14 @@ IDMap<RenderProcessHost> all_hosts;
} // namespace
+// static
bool RenderProcessHost::run_renderer_in_process_ = false;
+// static
+void RenderProcessHost::SetMaxRendererProcessCount(size_t count) {
+ max_renderer_count_override = count;
+}
+
RenderProcessHost::RenderProcessHost(Profile* profile)
: max_page_id_(-1),
fast_shutdown_started_(false),
diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h
index 1ceeb42..ed8473b 100644
--- a/chrome/browser/renderer_host/render_process_host.h
+++ b/chrome/browser/renderer_host/render_process_host.h
@@ -257,6 +257,11 @@ class RenderProcessHost : public IPC::Channel::Sender,
// the caller is free to create a new renderer.
static RenderProcessHost* GetExistingProcessHost(Profile* profile, Type type);
+ // Overrides the default heuristic for limiting the max renderer process
+ // count. This is useful for unit testing process limit behaviors.
+ // A value of zero means to use the default heuristic.
+ static void SetMaxRendererProcessCount(size_t count);
+
protected:
// A proxy for our IPC::Channel that lives on the IO thread (see
// browser_process.h)
diff --git a/chrome/browser/renderer_host/site_instance.cc b/chrome/browser/renderer_host/site_instance.cc
index cd409d9d..b0f4386 100644
--- a/chrome/browser/renderer_host/site_instance.cc
+++ b/chrome/browser/renderer_host/site_instance.cc
@@ -48,6 +48,13 @@ bool SiteInstance::HasProcess() const {
}
RenderProcessHost* SiteInstance::GetProcess() {
+ // TODO(erikkay) It would be nice to ensure that the renderer type had been
+ // properly set before we get here. The default tab creation case winds up
+ // with no site set at this point, so it will default to TYPE_NORMAL. This
+ // may not be correct, so we'll wind up potentially creating a process that
+ // we then throw away, or worse sharing a process with the wrong process type.
+ // See crbug.com/43448.
+
// Create a new process if ours went away or was reused.
if (!process_) {
// See if we should reuse an old process
@@ -194,21 +201,30 @@ GURL SiteInstance::GetEffectiveURL(Profile* profile, const GURL& url) {
}
}
-RenderProcessHost::Type SiteInstance::GetRendererType() {
- // We may not have a site at this point, which generally means this is a
- // normal navigation.
- if (!has_site_ || !site_.is_valid())
+/*static*/
+RenderProcessHost::Type SiteInstance::RendererTypeForURL(const GURL& url) {
+ if (!url.is_valid())
return RenderProcessHost::TYPE_NORMAL;
- if (site_.SchemeIs(chrome::kExtensionScheme))
+ if (url.SchemeIs(chrome::kExtensionScheme))
return RenderProcessHost::TYPE_EXTENSION;
- if (DOMUIFactory::HasDOMUIScheme(site_))
+ // TODO(erikkay) creis recommends using UseDOMUIForURL instead.
+ if (DOMUIFactory::HasDOMUIScheme(url))
return RenderProcessHost::TYPE_DOMUI;
return RenderProcessHost::TYPE_NORMAL;
}
+RenderProcessHost::Type SiteInstance::GetRendererType() {
+ // We may not have a site at this point, which generally means this is a
+ // normal navigation.
+ if (!has_site_)
+ return RenderProcessHost::TYPE_NORMAL;
+
+ return RendererTypeForURL(site_);
+}
+
void SiteInstance::Observe(NotificationType type,
const NotificationSource& source,
const NotificationDetails& details) {
diff --git a/chrome/browser/renderer_host/site_instance.h b/chrome/browser/renderer_host/site_instance.h
index 98807d0..3be920c 100644
--- a/chrome/browser/renderer_host/site_instance.h
+++ b/chrome/browser/renderer_host/site_instance.h
@@ -132,6 +132,9 @@ class SiteInstance : public base::RefCounted<SiteInstance>,
static bool IsSameWebSite(Profile* profile,
const GURL& url1, const GURL& url2);
+ // Returns the renderer type for this URL.
+ static RenderProcessHost::Type RendererTypeForURL(const GURL& url);
+
protected:
friend class base::RefCounted<SiteInstance>;
friend class BrowsingInstance;
diff --git a/chrome/browser/renderer_host/test/render_process_host_browsertest.cc b/chrome/browser/renderer_host/test/render_process_host_browsertest.cc
new file mode 100644
index 0000000..537b03a
--- /dev/null
+++ b/chrome/browser/renderer_host/test/render_process_host_browsertest.cc
@@ -0,0 +1,115 @@
+// Copyright (c) 2010 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 "chrome/browser/browser.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/renderer_host/site_instance.h"
+#include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/browser/renderer_host/render_process_host.h"
+#include "chrome/common/url_constants.h"
+#include "chrome/test/in_process_browser_test.h"
+#include "chrome/test/ui_test_utils.h"
+
+class RenderProcessHostTest : public InProcessBrowserTest {
+ public:
+ RenderProcessHostTest() {
+ EnableDOMAutomation();
+ }
+
+ int RenderProcessHostCount() {
+ RenderProcessHost::iterator hosts = RenderProcessHost::AllHostsIterator();
+ int count = 0;
+ while (!hosts.IsAtEnd()) {
+ if (hosts.GetCurrentValue()->HasConnection())
+ count++;
+ hosts.Advance();
+ }
+ return count;
+ }
+};
+
+// When we hit the max number of renderers, verify that the way we do process
+// sharing behaves correctly. In particular, this test is verifying that even
+// when we hit the max process limit, that renderers of each type will wind up
+// in a process of that type, even if that means creating a new process.
+// TODO(erikkay) crbug.com/43448 - disabled for now until we can get a
+// reasonable implementation in place.
+IN_PROC_BROWSER_TEST_F(RenderProcessHostTest, DISABLED_ProcessOverflow) {
+ // Set max renderers to 1 to force running out of processes.
+ RenderProcessHost::SetMaxRendererProcessCount(1);
+
+ int tab_count = 1;
+ int host_count = 1;
+ TabContents* tab1 = NULL;
+ TabContents* tab2 = NULL;
+ RenderProcessHost* rph1 = NULL;
+ RenderProcessHost* rph2 = NULL;
+ RenderProcessHost* rph3 = NULL;
+
+ // Change the first tab to be the new tab page (TYPE_DOMUI).
+ GURL newtab(chrome::kChromeUINewTabURL);
+ ui_test_utils::NavigateToURL(browser(), newtab);
+ EXPECT_EQ(tab_count, browser()->tab_count());
+ tab1 = browser()->GetTabContentsAt(tab_count - 1);
+ rph1 = tab1->GetRenderProcessHost();
+ EXPECT_EQ(tab1->GetURL(), newtab);
+ EXPECT_EQ(host_count, RenderProcessHostCount());
+
+ // Create a new TYPE_NORMAL tab. It should be in its own process.
+ GURL page1("data:text/html,hello world1");
+ browser()->ShowSingletonTab(page1);
+ if (browser()->tab_count() == tab_count)
+ ui_test_utils::WaitForNewTab(browser());
+ tab_count++;
+ host_count++;
+ EXPECT_EQ(tab_count, browser()->tab_count());
+ tab1 = browser()->GetTabContentsAt(tab_count - 1);
+ rph2 = tab1->GetRenderProcessHost();
+ EXPECT_EQ(tab1->GetURL(), page1);
+ EXPECT_EQ(host_count, RenderProcessHostCount());
+ EXPECT_NE(rph1, rph2);
+
+ // Create another TYPE_NORMAL tab. It should share the previous process.
+ GURL page2("data:text/html,hello world2");
+ browser()->ShowSingletonTab(page2);
+ if (browser()->tab_count() == tab_count)
+ ui_test_utils::WaitForNewTab(browser());
+ tab_count++;
+ EXPECT_EQ(tab_count, browser()->tab_count());
+ tab2 = browser()->GetTabContentsAt(tab_count - 1);
+ EXPECT_EQ(tab2->GetURL(), page2);
+ EXPECT_EQ(host_count, RenderProcessHostCount());
+ EXPECT_EQ(tab2->GetRenderProcessHost(), rph2);
+
+ // Create another TYPE_DOMUI tab. It should share the process with newtab.
+ // Note: intentionally create this tab after the TYPE_NORMAL tabs to exercise
+ // bug 43448 where extension and DOMUI tabs could get combined into normal
+ // renderers.
+ GURL history(chrome::kChromeUIHistoryURL);
+ browser()->ShowSingletonTab(history);
+ if (browser()->tab_count() == tab_count)
+ ui_test_utils::WaitForNewTab(browser());
+ tab_count++;
+ EXPECT_EQ(tab_count, browser()->tab_count());
+ tab2 = browser()->GetTabContentsAt(tab_count - 1);
+ EXPECT_EQ(tab2->GetURL(), history);
+ EXPECT_EQ(host_count, RenderProcessHostCount());
+ EXPECT_EQ(tab2->GetRenderProcessHost(), rph1);
+
+ // Create a TYPE_EXTENSION tab. It should be in its own process.
+ // (the bookmark manager is implemented as an extension)
+ GURL bookmarks(chrome::kChromeUIBookmarksURL);
+ browser()->ShowSingletonTab(bookmarks);
+ if (browser()->tab_count() == tab_count)
+ ui_test_utils::WaitForNewTab(browser());
+ tab_count++;
+ host_count++;
+ EXPECT_EQ(tab_count, browser()->tab_count());
+ tab1 = browser()->GetTabContentsAt(tab_count - 1);
+ rph3 = tab1->GetRenderProcessHost();
+ EXPECT_EQ(tab1->GetURL(), bookmarks);
+ EXPECT_EQ(host_count, RenderProcessHostCount());
+ EXPECT_NE(rph1, rph3);
+ EXPECT_NE(rph2, rph3);
+}
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 72a39af..83b7b27 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -1447,6 +1447,7 @@
'browser/net/ftp_browsertest.cc',
'browser/printing/print_dialog_cloud_uitest.cc',
'browser/renderer_host/test/web_cache_manager_browsertest.cc',
+ 'browser/renderer_host/test/render_process_host_browsertest.cc',
'browser/renderer_host/test/render_view_host_manager_browsertest.cc',
'browser/sessions/session_restore_browsertest.cc',
'browser/ssl/ssl_browser_tests.cc',