Index: content/browser/site_instance_impl_unittest.cc |
diff --git a/content/browser/site_instance_impl_unittest.cc b/content/browser/site_instance_impl_unittest.cc |
index fa4ea5732f3ef4434064032d69ccd519b911ec47..261d16c82f1a29b6ae9ac0f5039d512821367a29 100644 |
--- a/content/browser/site_instance_impl_unittest.cc |
+++ b/content/browser/site_instance_impl_unittest.cc |
@@ -2,6 +2,7 @@ |
// Use of this source code is governed by a BSD-style license that can be |
// found in the LICENSE file. |
+#include "base/command_line.h" |
#include "base/compiler_specific.h" |
#include "base/stl_util.h" |
#include "base/string16.h" |
@@ -17,6 +18,7 @@ |
#include "content/public/browser/web_ui_controller_factory.h" |
#include "content/public/common/content_client.h" |
#include "content/public/common/content_constants.h" |
+#include "content/public/common/content_switches.h" |
#include "content/public/common/url_constants.h" |
#include "content/public/test/mock_render_process_host.h" |
#include "content/public/test/test_browser_context.h" |
@@ -92,11 +94,6 @@ class SiteInstanceTestBrowserClient : |
return &factory_; |
} |
- virtual bool ShouldUseProcessPerSite(BrowserContext* browser_context, |
- const GURL& effective_url) OVERRIDE { |
- return false; |
- } |
- |
virtual bool IsSuitableHost(content::RenderProcessHost* process_host, |
const GURL& site_url) OVERRIDE { |
return (privileged_process_id_ == process_host->GetID()) == |
@@ -133,6 +130,9 @@ class SiteInstanceTest : public testing::Test { |
} |
virtual void TearDown() { |
+ // Ensure that no RenderProcessHosts are left over after the tests. |
+ EXPECT_TRUE(content::RenderProcessHost::AllHostsIterator().IsAtEnd()); |
+ |
content::GetContentClient()->set_browser_for_testing(old_browser_client_); |
content::SetContentClient(old_client_); |
MessageLoop::current()->RunAllPending(); |
@@ -155,26 +155,16 @@ class SiteInstanceTest : public testing::Test { |
content::ContentBrowserClient* old_browser_client_; |
}; |
+// Subclass of BrowsingInstance that updates a counter when deleted and |
+// returns TestSiteInstances from GetSiteInstanceForURL. |
class TestBrowsingInstance : public BrowsingInstance { |
public: |
TestBrowsingInstance(BrowserContext* browser_context, int* delete_counter) |
: BrowsingInstance(browser_context), |
- use_process_per_site_(false), |
delete_counter_(delete_counter) { |
} |
- // Overrides BrowsingInstance::ShouldUseProcessPerSite so that we can test |
- // both alternatives without using command-line switches. |
- bool ShouldUseProcessPerSite(const GURL& url) { |
- return use_process_per_site_; |
- } |
- |
- void set_use_process_per_site(bool use_process_per_site) { |
- use_process_per_site_ = use_process_per_site; |
- } |
- |
// Make a few methods public for tests. |
- using BrowsingInstance::ShouldUseProcessPerSite; |
using BrowsingInstance::browser_context; |
using BrowsingInstance::HasSiteInstance; |
using BrowsingInstance::GetSiteInstanceForURL; |
@@ -186,12 +176,10 @@ class TestBrowsingInstance : public BrowsingInstance { |
(*delete_counter_)++; |
} |
- // Set by individual tests. |
- bool use_process_per_site_; |
- |
int* delete_counter_; |
}; |
+// Subclass of SiteInstanceImpl that updates a counter when deleted. |
class TestSiteInstance : public SiteInstanceImpl { |
public: |
static TestSiteInstance* CreateTestSiteInstance( |
@@ -410,10 +398,13 @@ TEST_F(SiteInstanceTest, IsSameWebSite) { |
// Test to ensure that there is only one SiteInstance per site in a given |
// BrowsingInstance, when process-per-site is not in use. |
TEST_F(SiteInstanceTest, OneSiteInstancePerSite) { |
+ ASSERT_FALSE(CommandLine::ForCurrentProcess()->HasSwitch( |
+ switches::kProcessPerSite)); |
int delete_counter = 0; |
+ scoped_ptr<content::TestBrowserContext> browser_context( |
+ new content::TestBrowserContext()); |
TestBrowsingInstance* browsing_instance = |
- new TestBrowsingInstance(NULL, &delete_counter); |
- browsing_instance->set_use_process_per_site(false); |
+ new TestBrowsingInstance(browser_context.get(), &delete_counter); |
const GURL url_a1("http://www.google.com/1.html"); |
scoped_refptr<SiteInstanceImpl> site_instance_a1( |
@@ -444,8 +435,7 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSite) { |
// A visit to the original site in a new BrowsingInstance (same or different |
// browser context) should return a different SiteInstance. |
TestBrowsingInstance* browsing_instance2 = |
- new TestBrowsingInstance(NULL, &delete_counter); |
- browsing_instance2->set_use_process_per_site(false); |
+ new TestBrowsingInstance(browser_context.get(), &delete_counter); |
// Ensure the new SiteInstance is ref counted so that it gets deleted. |
scoped_refptr<SiteInstanceImpl> site_instance_a2_2( |
static_cast<SiteInstanceImpl*>( |
@@ -453,6 +443,14 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSite) { |
EXPECT_NE(site_instance_a1.get(), site_instance_a2_2.get()); |
EXPECT_FALSE(site_instance_a1->IsRelatedSiteInstance(site_instance_a2_2)); |
+ // The two SiteInstances for http://google.com should not use the same process |
+ // if process-per-site is not enabled. |
+ scoped_ptr<content::RenderProcessHost> process_a1( |
+ site_instance_a1->GetProcess()); |
+ scoped_ptr<content::RenderProcessHost> process_a2_2( |
+ site_instance_a2_2->GetProcess()); |
+ EXPECT_NE(process_a1.get(), process_a2_2.get()); |
+ |
// Should be able to see that we do have SiteInstances. |
EXPECT_TRUE(browsing_instance->HasSiteInstance( |
GURL("http://mail.google.com"))); |
@@ -467,22 +465,28 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSite) { |
EXPECT_FALSE(browsing_instance2->HasSiteInstance( |
GURL("http://www.yahoo.com"))); |
- // browsing_instances will be deleted when their SiteInstances are deleted |
+ // browsing_instances will be deleted when their SiteInstances are deleted. |
+ // The processes will be unregistered when the RPH scoped_ptrs go away. |
} |
-// Test to ensure that there is only one SiteInstance per site for an entire |
-// BrowserContext, if process-per-site is in use. |
+// Test to ensure that there is only one RenderProcessHost per site for an |
+// entire BrowserContext, if process-per-site is in use. |
TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInBrowserContext) { |
+ CommandLine::ForCurrentProcess()->AppendSwitch( |
+ switches::kProcessPerSite); |
int delete_counter = 0; |
+ scoped_ptr<content::TestBrowserContext> browser_context( |
+ new content::TestBrowserContext()); |
TestBrowsingInstance* browsing_instance = |
- new TestBrowsingInstance(NULL, &delete_counter); |
- browsing_instance->set_use_process_per_site(true); |
+ new TestBrowsingInstance(browser_context.get(), &delete_counter); |
const GURL url_a1("http://www.google.com/1.html"); |
scoped_refptr<SiteInstanceImpl> site_instance_a1( |
static_cast<SiteInstanceImpl*>( |
browsing_instance->GetSiteInstanceForURL(url_a1))); |
EXPECT_TRUE(site_instance_a1.get() != NULL); |
+ scoped_ptr<content::RenderProcessHost> process_a1( |
+ site_instance_a1->GetProcess()); |
// A separate site should create a separate SiteInstance. |
const GURL url_b1("http://www.yahoo.com/"); |
@@ -505,28 +509,30 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInBrowserContext) { |
site_instance_a1->GetRelatedSiteInstance(url_a2)); |
// A visit to the original site in a new BrowsingInstance (same browser |
- // context) should also return the same SiteInstance. |
- // This BrowsingInstance doesn't get its own SiteInstance within the test, so |
- // it won't be deleted by its children. Thus, we'll keep a ref count to it |
- // to make sure it gets deleted. |
- scoped_refptr<TestBrowsingInstance> browsing_instance2( |
- new TestBrowsingInstance(NULL, &delete_counter)); |
- browsing_instance2->set_use_process_per_site(true); |
- EXPECT_EQ(site_instance_a1.get(), |
- browsing_instance2->GetSiteInstanceForURL(url_a2)); |
+ // context) should return a different SiteInstance with the same process. |
+ TestBrowsingInstance* browsing_instance2 = |
+ new TestBrowsingInstance(browser_context.get(), &delete_counter); |
+ scoped_refptr<SiteInstanceImpl> site_instance_a1_2( |
+ static_cast<SiteInstanceImpl*>( |
+ browsing_instance2->GetSiteInstanceForURL(url_a1))); |
+ EXPECT_TRUE(site_instance_a1.get() != NULL); |
+ EXPECT_NE(site_instance_a1.get(), site_instance_a1_2.get()); |
+ EXPECT_EQ(process_a1.get(), site_instance_a1_2->GetProcess()); |
// A visit to the original site in a new BrowsingInstance (different browser |
- // context) should return a different SiteInstance. |
- scoped_ptr<content::TestBrowserContext> browser_context( |
+ // context) should return a different SiteInstance with a different process. |
+ scoped_ptr<content::TestBrowserContext> browser_context2( |
new content::TestBrowserContext()); |
TestBrowsingInstance* browsing_instance3 = |
- new TestBrowsingInstance(browser_context.get(), &delete_counter); |
- browsing_instance3->set_use_process_per_site(true); |
- // Ensure the new SiteInstance is ref counted so that it gets deleted. |
+ new TestBrowsingInstance(browser_context2.get(), &delete_counter); |
scoped_refptr<SiteInstanceImpl> site_instance_a2_3( |
static_cast<SiteInstanceImpl*>( |
browsing_instance3->GetSiteInstanceForURL(url_a2))); |
+ EXPECT_TRUE(site_instance_a2_3.get() != NULL); |
+ scoped_ptr<content::RenderProcessHost> process_a2_3( |
+ site_instance_a2_3->GetProcess()); |
EXPECT_NE(site_instance_a1.get(), site_instance_a2_3.get()); |
+ EXPECT_NE(process_a1.get(), process_a2_3.get()); |
// Should be able to see that we do have SiteInstances. |
EXPECT_TRUE(browsing_instance->HasSiteInstance( |
@@ -535,23 +541,26 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInBrowserContext) { |
GURL("http://mail.google.com"))); // visited before |
EXPECT_TRUE(browsing_instance->HasSiteInstance( |
GURL("http://mail.yahoo.com"))); // visited before |
- EXPECT_TRUE(browsing_instance2->HasSiteInstance( |
- GURL("http://www.yahoo.com"))); // different BI, but same browser context |
// Should be able to see that we don't have SiteInstances. |
+ EXPECT_FALSE(browsing_instance2->HasSiteInstance( |
+ GURL("http://www.yahoo.com"))); // different BI, same browser context |
EXPECT_FALSE(browsing_instance->HasSiteInstance( |
GURL("https://www.google.com"))); // not visited before |
EXPECT_FALSE(browsing_instance3->HasSiteInstance( |
GURL("http://www.yahoo.com"))); // different BI, different context |
- // browsing_instances will be deleted when their SiteInstances are deleted |
+ // browsing_instances will be deleted when their SiteInstances are deleted. |
+ // The processes will be unregistered when the RPH scoped_ptrs go away. |
} |
static SiteInstanceImpl* CreateSiteInstance( |
- content::RenderProcessHostFactory* factory, const GURL& url) { |
+ content::BrowserContext* browser_context, |
+ content::RenderProcessHostFactory* factory, |
+ const GURL& url) { |
SiteInstanceImpl* instance = |
reinterpret_cast<SiteInstanceImpl*>( |
- SiteInstance::CreateForURL(NULL, url)); |
+ SiteInstance::CreateForURL(browser_context, url)); |
instance->set_render_process_host_factory(factory); |
return instance; |
} |
@@ -564,18 +573,20 @@ TEST_F(SiteInstanceTest, ProcessSharingByType) { |
ChildProcessSecurityPolicyImpl::GetInstance(); |
// Make a bunch of mock renderers so that we hit the limit. |
+ scoped_ptr<content::TestBrowserContext> browser_context( |
+ new content::TestBrowserContext()); |
std::vector<MockRenderProcessHost*> hosts; |
for (size_t i = 0; i < content::kMaxRendererProcessCount; ++i) |
- hosts.push_back(new MockRenderProcessHost(NULL)); |
+ hosts.push_back(new MockRenderProcessHost(browser_context.get())); |
// Create some extension instances and make sure they share a process. |
scoped_refptr<SiteInstanceImpl> extension1_instance( |
- CreateSiteInstance(&rph_factory, |
+ CreateSiteInstance(browser_context.get(), &rph_factory, |
GURL(kPrivilegedScheme + std::string("://foo/bar")))); |
set_privileged_process_id(extension1_instance->GetProcess()->GetID()); |
scoped_refptr<SiteInstanceImpl> extension2_instance( |
- CreateSiteInstance(&rph_factory, |
+ CreateSiteInstance(browser_context.get(), &rph_factory, |
GURL(kPrivilegedScheme + std::string("://baz/bar")))); |
scoped_ptr<content::RenderProcessHost> extension_host( |
@@ -585,12 +596,12 @@ TEST_F(SiteInstanceTest, ProcessSharingByType) { |
// Create some WebUI instances and make sure they share a process. |
scoped_refptr<SiteInstanceImpl> webui1_instance(CreateSiteInstance( |
- &rph_factory, |
+ browser_context.get(), &rph_factory, |
GURL(chrome::kChromeUIScheme + std::string("://newtab")))); |
policy->GrantWebUIBindings(webui1_instance->GetProcess()->GetID()); |
scoped_refptr<SiteInstanceImpl> webui2_instance(CreateSiteInstance( |
- &rph_factory, |
+ browser_context.get(), &rph_factory, |
GURL(chrome::kChromeUIScheme + std::string("://history")))); |
scoped_ptr<content::RenderProcessHost> dom_host( |