diff options
author | erikkay@chromium.org <erikkay@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-11 17:14:48 +0000 |
---|---|---|
committer | erikkay@chromium.org <erikkay@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2010-07-11 17:14:48 +0000 |
commit | 08c8ec7e386525210a0ff931e8a3e8e9678db398 (patch) | |
tree | 2cb862942569dcacfcec87fe1214df4a2beaffee | |
parent | 64fb9acba969a51fc806df5ae8e3e60e05994b78 (diff) | |
download | chromium_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.cc | 11 | ||||
-rw-r--r-- | chrome/browser/renderer_host/render_process_host.h | 5 | ||||
-rw-r--r-- | chrome/browser/renderer_host/site_instance.cc | 28 | ||||
-rw-r--r-- | chrome/browser/renderer_host/site_instance.h | 3 | ||||
-rw-r--r-- | chrome/browser/renderer_host/test/render_process_host_browsertest.cc | 115 | ||||
-rw-r--r-- | chrome/chrome_tests.gypi | 1 |
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', |