Group renderer processes by privilige when we hit the max process count.

BUG=12128
TEST=Create a bunch of tabs (40+) and make sure New Tab pages are always
grouped in a process with other chrome internal pages, extensions are
always grouped together, and regular web pages are never in a process with
extensions or New Tab pages.
Review URL: http://codereview.chromium.org/126002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@18428 0039d316-1c4b-4281-b951-d872f2087c98

6 files changed, 144 insertions, 28 deletions

diff --git a/chrome/browser/renderer_host/mock_render_process_host.cc b/chrome/browser/renderer_host/mock_render_process_host.cc
index 2e6e61e..8cf0e10 100644
--- a/chrome/browser/renderer_host/mock_render_process_host.cc
+++ b/chrome/browser/renderer_host/mock_render_process_host.cc
@@ -12,6 +12,7 @@ MockRenderProcessHost::MockRenderProcessHost(Profile* profile)
 }
 
 MockRenderProcessHost::~MockRenderProcessHost() {
+  RemoveFromList();
   delete transport_dib_;
 }
 
diff --git a/chrome/browser/renderer_host/render_process_host.cc b/chrome/browser/renderer_host/render_process_host.cc
index 349f8e2..49e70da 100644
--- a/chrome/browser/renderer_host/render_process_host.cc
+++ b/chrome/browser/renderer_host/render_process_host.cc
@@ -6,12 +6,13 @@
 
 #include "base/rand_util.h"
 #include "base/sys_info.h"
+#include "chrome/browser/child_process_security_policy.h"
 #include "chrome/common/chrome_constants.h"
 #include "chrome/common/notification_service.h"
 
 namespace {
 
-unsigned int GetMaxRendererProcessCount() {
+size_t GetMaxRendererProcessCount() {
   // 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
@@ -21,7 +22,7 @@ unsigned int GetMaxRendererProcessCount() {
   // If you modify this table you need to adjust browser\browser_uitest.cc
   // to match the expected number of processes.
 
-  static const int kMaxRenderersByRamTier[] = {
+  static const size_t kMaxRenderersByRamTier[] = {
     3,                        // less than 256MB
     6,                        //  256MB
     9,                        //  512MB
@@ -39,7 +40,7 @@ unsigned int GetMaxRendererProcessCount() {
     40                        // 3584MB
   };
 
-  static unsigned int max_count = 0;
+  static size_t max_count = 0;
   if (!max_count) {
     size_t memory_tier = base::SysInfo::AmountOfPhysicalMemoryMB() / 256;
     if (memory_tier >= arraysize(kMaxRenderersByRamTier))
@@ -52,8 +53,24 @@ unsigned int GetMaxRendererProcessCount() {
 
 // Returns true if the given host is suitable for launching a new view
 // associated with the given profile.
-static bool IsSuitableHost(Profile* profile, RenderProcessHost* host) {
-  return host->profile() == profile;
+static bool IsSuitableHost(RenderProcessHost* host, Profile* profile,
+                           RenderProcessHost::Type type) {
+  // If the host doesn't have a PID yet, we don't know what it will be used
+  // for, so just say it's unsuitable to be safe.
+  if (host->pid() == -1)
+    return false;
+
+  if (host->profile() != profile)
+    return false;
+
+  RenderProcessHost::Type host_type = RenderProcessHost::TYPE_NORMAL;
+  if (ChildProcessSecurityPolicy::GetInstance()->HasDOMUIBindings(host->pid()))
+    host_type = RenderProcessHost::TYPE_DOMUI;
+  if (ChildProcessSecurityPolicy::GetInstance()->
+        HasExtensionBindings(host->pid()))
+    host_type = RenderProcessHost::TYPE_EXTENSION;
+
+  return host_type == type;
 }
 
 // the global list of all renderer processes
@@ -127,8 +144,7 @@ RenderProcessHost* RenderProcessHost::FromID(int render_process_id) {
 
 // static
 bool RenderProcessHost::ShouldTryToUseExistingProcessHost() {
-  unsigned int renderer_process_count =
-      static_cast<unsigned int>(all_hosts.size());
+  size_t renderer_process_count = all_hosts.size();
 
   // NOTE: Sometimes it's necessary to create more render processes than
   //       GetMaxRendererProcessCount(), for instance when we want to create
@@ -141,13 +157,15 @@ bool RenderProcessHost::ShouldTryToUseExistingProcessHost() {
 }
 
 // static
-RenderProcessHost* RenderProcessHost::GetExistingProcessHost(Profile* profile) {
+RenderProcessHost* RenderProcessHost::GetExistingProcessHost(Profile* profile,
+                                                             Type type) {
   // First figure out which existing renderers we can use.
   std::vector<RenderProcessHost*> suitable_renderers;
   suitable_renderers.reserve(size());
 
   for (iterator iter = begin(); iter != end(); ++iter) {
-    if (IsSuitableHost(profile, iter->second))
+    if (run_renderer_in_process() ||
+        IsSuitableHost(iter->second, profile, type))
       suitable_renderers.push_back(iter->second);
   }
 
@@ -170,3 +188,8 @@ void RenderProcessHost::SetProcessID(int pid) {
   pid_ = pid;
   all_hosts.AddWithID(this, pid);
 }
+
+void RenderProcessHost::RemoveFromList() {
+  if (all_hosts.Lookup(pid_))
+    all_hosts.Remove(pid_);
+}
diff --git a/chrome/browser/renderer_host/render_process_host.h b/chrome/browser/renderer_host/render_process_host.h
index 44990d7..ef79711 100644
--- a/chrome/browser/renderer_host/render_process_host.h
+++ b/chrome/browser/renderer_host/render_process_host.h
@@ -28,6 +28,16 @@ class RenderProcessHost : public IPC::Channel::Sender,
  public:
   typedef IDMap<RenderProcessHost>::const_iterator iterator;
 
+  // We classify renderers according to their highest privilege, and try
+  // to group pages into renderers with similar privileges.
+  // Note: it may be possible for a renderer to have multiple privileges,
+  // in which case we call it an "extension" renderer.
+  enum Type {
+    TYPE_NORMAL,     // Normal renderer, no extra privileges.
+    TYPE_DOMUI,      // Renderer with DOMUI privileges, like New Tab.
+    TYPE_EXTENSION,  // Renderer with extension privileges.
+  };
+
   RenderProcessHost(Profile* profile);
   virtual ~RenderProcessHost();
 
@@ -174,15 +184,19 @@ class RenderProcessHost : public IPC::Channel::Sender,
   static bool ShouldTryToUseExistingProcessHost();
 
   // Get an existing RenderProcessHost associated with the given profile, if
-  // possible.  The renderer process is chosen randomly from the
-  // processes associated with the given profile.
-  // Returns NULL if no suitable renderer process is available.
-  static RenderProcessHost* GetExistingProcessHost(Profile* profile);
+  // possible.  The renderer process is chosen randomly from suitable renderers
+  // that share the same profile and type.
+  // Returns NULL if no suitable renderer process is available, in which case
+  // the caller is free to create a new renderer.
+  static RenderProcessHost* GetExistingProcessHost(Profile* profile, Type type);
 
  protected:
-  // Sets the process  of this object, so that others access it using FromID.
+  // Sets the process of this object, so that others access it using FromID.
   void SetProcessID(int pid);
 
+  // For testing.  Removes this host from the list of hosts.
+  void RemoveFromList();
+
   base::Process process_;
 
   // A proxy for our IPC::Channel that lives on the IO thread (see
diff --git a/chrome/browser/tab_contents/site_instance.cc b/chrome/browser/tab_contents/site_instance.cc
index 0dbe744..8ea7baf 100644
--- a/chrome/browser/tab_contents/site_instance.cc
+++ b/chrome/browser/tab_contents/site_instance.cc
@@ -4,6 +4,7 @@
 
 #include "chrome/browser/tab_contents/site_instance.h"
 
+#include "chrome/browser/dom_ui/dom_ui_factory.h"
 #include "chrome/browser/renderer_host/browser_render_process_host.h"
 #include "chrome/common/url_constants.h"
 #include "chrome/common/notification_service.h"
@@ -35,7 +36,7 @@ RenderProcessHost* SiteInstance::GetProcess() {
     // See if we should reuse an old process
     if (RenderProcessHost::ShouldTryToUseExistingProcessHost())
       process_ = RenderProcessHost::GetExistingProcessHost(
-          browsing_instance_->profile());
+          browsing_instance_->profile(), GetRendererType());
 
     // Otherwise (or if that fails), create a new one.
     if (!process_) {
@@ -166,6 +167,21 @@ bool SiteInstance::IsSameWebSite(const GURL& url1, const GURL& url2) {
   return net::RegistryControlledDomainService::SameDomainOrHost(url1, url2);
 }
 
+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())
+    return RenderProcessHost::TYPE_NORMAL;
+
+  if (site_.SchemeIs(chrome::kExtensionScheme))
+    return RenderProcessHost::TYPE_EXTENSION;
+
+  if (DOMUIFactory::HasDOMUIScheme(site_))
+    return RenderProcessHost::TYPE_DOMUI;
+
+  return RenderProcessHost::TYPE_NORMAL;
+}
+
 void SiteInstance::Observe(NotificationType type,
                            const NotificationSource& source,
                            const NotificationDetails& details) {
diff --git a/chrome/browser/tab_contents/site_instance.h b/chrome/browser/tab_contents/site_instance.h
index 5c14dcc..81c182d 100644
--- a/chrome/browser/tab_contents/site_instance.h
+++ b/chrome/browser/tab_contents/site_instance.h
@@ -138,6 +138,10 @@ class SiteInstance : public base::RefCounted<SiteInstance>,
   // GetRelatedSiteInstance instead.
   SiteInstance(BrowsingInstance* browsing_instance);
 
+  // Returns the type of renderer process this instance belongs in, for grouping
+  // purposes.
+  RenderProcessHost::Type GetRendererType();
+
  private:
   // NotificationObserver implementation.
   void Observe(NotificationType type,
diff --git a/chrome/browser/tab_contents/site_instance_unittest.cc b/chrome/browser/tab_contents/site_instance_unittest.cc
index d0068d6..12bb9f9 100644
--- a/chrome/browser/tab_contents/site_instance_unittest.cc
+++ b/chrome/browser/tab_contents/site_instance_unittest.cc
@@ -3,11 +3,13 @@
 // found in the LICENSE file.
 
 #include "base/string16.h"
+#include "chrome/browser/child_process_security_policy.h"
 #include "chrome/browser/renderer_host/browser_render_process_host.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/renderer_host/test_render_view_host.h"
 #include "chrome/browser/tab_contents/navigation_entry.h"
 #include "chrome/browser/tab_contents/tab_contents.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/render_messages.h"
 #include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
@@ -170,12 +172,12 @@ TEST_F(SiteInstanceTest, CloneNavigationEntry) {
 // Test to ensure UpdateMaxPageID is working properly.
 TEST_F(SiteInstanceTest, UpdateMaxPageID) {
   scoped_refptr<SiteInstance> instance(SiteInstance::CreateSiteInstance(NULL));
-  EXPECT_EQ(-1, instance.get()->max_page_id());
+  EXPECT_EQ(-1, instance->max_page_id());
 
   // Make sure max_page_id_ is monotonically increasing.
-  instance.get()->UpdateMaxPageID(3);
-  instance.get()->UpdateMaxPageID(1);
-  EXPECT_EQ(3, instance.get()->max_page_id());
+  instance->UpdateMaxPageID(3);
+  instance->UpdateMaxPageID(1);
+  EXPECT_EQ(3, instance->max_page_id());
 }
 
 // Test to ensure GetProcess returns and creates processes correctly.
@@ -185,13 +187,13 @@ TEST_F(SiteInstanceTest, GetProcess) {
   scoped_ptr<RenderProcessHost> host1;
   scoped_refptr<SiteInstance> instance(
       SiteInstance::CreateSiteInstance(profile.get()));
-  host1.reset(instance.get()->GetProcess());
+  host1.reset(instance->GetProcess());
   EXPECT_TRUE(host1.get() != NULL);
 
   // Ensure that GetProcess creates a new process.
   scoped_refptr<SiteInstance> instance2(
       SiteInstance::CreateSiteInstance(profile.get()));
-  scoped_ptr<RenderProcessHost> host2(instance2.get()->GetProcess());
+  scoped_ptr<RenderProcessHost> host2(instance2->GetProcess());
   EXPECT_TRUE(host2.get() != NULL);
   EXPECT_NE(host1.get(), host2.get());
 }
@@ -200,10 +202,10 @@ TEST_F(SiteInstanceTest, GetProcess) {
 TEST_F(SiteInstanceTest, SetSite) {
   scoped_refptr<SiteInstance> instance(SiteInstance::CreateSiteInstance(NULL));
   EXPECT_FALSE(instance->has_site());
-  EXPECT_TRUE(instance.get()->site().is_empty());
+  EXPECT_TRUE(instance->site().is_empty());
 
-  instance.get()->SetSite(GURL("http://www.google.com/index.html"));
-  EXPECT_EQ(GURL("http://google.com"), instance.get()->site());
+  instance->SetSite(GURL("http://www.google.com/index.html"));
+  EXPECT_EQ(GURL("http://google.com"), instance->site());
 
   EXPECT_TRUE(instance->has_site());
 }
@@ -291,14 +293,14 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSite) {
   // Getting the new SiteInstance from the BrowsingInstance and from another
   // SiteInstance in the BrowsingInstance should give the same result.
   EXPECT_EQ(site_instance_b1.get(),
-            site_instance_a1.get()->GetRelatedSiteInstance(url_b1));
+            site_instance_a1->GetRelatedSiteInstance(url_b1));
 
   // A second visit to the original site should return the same SiteInstance.
   const GURL url_a2("http://www.google.com/2.html");
   EXPECT_EQ(site_instance_a1.get(),
             browsing_instance->GetSiteInstanceForURL(url_a2));
   EXPECT_EQ(site_instance_a1.get(),
-            site_instance_a1.get()->GetRelatedSiteInstance(url_a2));
+            site_instance_a1->GetRelatedSiteInstance(url_a2));
 
   // A visit to the original site in a new BrowsingInstance (same or different
   // profile) should return a different SiteInstance.
@@ -349,14 +351,14 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInProfile) {
   // Getting the new SiteInstance from the BrowsingInstance and from another
   // SiteInstance in the BrowsingInstance should give the same result.
   EXPECT_EQ(site_instance_b1.get(),
-            site_instance_a1.get()->GetRelatedSiteInstance(url_b1));
+            site_instance_a1->GetRelatedSiteInstance(url_b1));
 
   // A second visit to the original site should return the same SiteInstance.
   const GURL url_a2("http://www.google.com/2.html");
   EXPECT_EQ(site_instance_a1.get(),
             browsing_instance->GetSiteInstanceForURL(url_a2));
   EXPECT_EQ(site_instance_a1.get(),
-            site_instance_a1.get()->GetRelatedSiteInstance(url_a2));
+            site_instance_a1->GetRelatedSiteInstance(url_a2));
 
   // A visit to the original site in a new BrowsingInstance (same profile)
   // should also return the same SiteInstance.
@@ -398,3 +400,59 @@ TEST_F(SiteInstanceTest, OneSiteInstancePerSiteInProfile) {
 
   // browsing_instances will be deleted when their SiteInstances are deleted
 }
+
+static SiteInstance* CreateSiteInstance(RenderProcessHostFactory* factory,
+                                        const GURL& url) {
+  SiteInstance* instance = SiteInstance::CreateSiteInstanceForURL(NULL, url);
+  instance->set_render_process_host_factory(factory);
+  return instance;
+}
+
+// Test to ensure that pages that require certain privileges are grouped
+// in processes with similar pages.
+TEST_F(SiteInstanceTest, ProcessSharingByType) {
+  MockRenderProcessHostFactory rph_factory;
+  ChildProcessSecurityPolicy* policy =
+      ChildProcessSecurityPolicy::GetInstance();
+
+  // Make a bunch of mock renderers so that we hit the limit.
+  std::vector<MockRenderProcessHost*> hosts;
+  for (size_t i = 0; i < chrome::kMaxRendererProcessCount; ++i)
+    hosts.push_back(new MockRenderProcessHost(NULL));
+
+  // Create some extension instances and make sure they share a process.
+  scoped_refptr<SiteInstance> extension1_instance(
+      CreateSiteInstance(&rph_factory, GURL("chrome-extension://foo/bar")));
+  policy->Add(extension1_instance->GetProcess()->pid());
+  policy->GrantExtensionBindings(extension1_instance->GetProcess()->pid());
+
+  scoped_refptr<SiteInstance> extension2_instance(
+      CreateSiteInstance(&rph_factory, GURL("chrome-extension://baz/bar")));
+
+  scoped_ptr<RenderProcessHost> extension_host(
+      extension1_instance->GetProcess());
+  EXPECT_EQ(extension1_instance->GetProcess(),
+            extension2_instance->GetProcess());
+
+  // Create some DOMUI instances and make sure they share a process.
+  scoped_refptr<SiteInstance> dom1_instance(
+      CreateSiteInstance(&rph_factory, GURL("chrome://newtab")));
+  policy->Add(dom1_instance->GetProcess()->pid());
+  policy->GrantDOMUIBindings(dom1_instance->GetProcess()->pid());
+
+  scoped_refptr<SiteInstance> dom2_instance(
+      CreateSiteInstance(&rph_factory, GURL("chrome://history")));
+
+  scoped_ptr<RenderProcessHost> dom_host(dom1_instance->GetProcess());
+  EXPECT_EQ(dom1_instance->GetProcess(), dom2_instance->GetProcess());
+
+  // Make sure none of differing privilege processes are mixed.
+  EXPECT_NE(extension1_instance->GetProcess(), dom1_instance->GetProcess());
+
+  for (size_t i = 0; i < chrome::kMaxRendererProcessCount; ++i) {
+    EXPECT_NE(extension1_instance->GetProcess(), hosts[i]);
+    EXPECT_NE(dom1_instance->GetProcess(), hosts[i]);
+  }
+
+  STLDeleteContainerPointers(hosts.begin(), hosts.end());
+}