Resurrect ExtensionProcessManager. Move the code for starting extension

instances from ExtensionsService to the manager.

Unlike ExtensionsService, EPM is not shared between an incognito Profile and
its parent.
Review URL: http://codereview.chromium.org/109044

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

8 files changed, 200 insertions, 100 deletions

diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
new file mode 100644
index 0000000..79b2e0f
--- /dev/null
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -0,0 +1,71 @@
+// Copyright (c) 2009 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/extensions/extension_process_manager.h"
+
+#include "chrome/browser/browsing_instance.h"
+#include "chrome/browser/extensions/extension.h"
+#include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_view.h"
+#include "chrome/browser/extensions/extensions_service.h"
+#include "chrome/browser/profile.h"
+#include "chrome/browser/tab_contents/site_instance.h"
+#include "chrome/common/notification_service.h"
+
+static void CreateBackgroundHosts(
+    ExtensionProcessManager* manager, const ExtensionList* extensions) {
+  for (ExtensionList::const_iterator extension = extensions->begin();
+       extension != extensions->end(); ++extension) {
+    // Start the process for the master page, if it exists.
+    if ((*extension)->background_url().is_valid()) {
+      manager->CreateBackgroundHost(*extension, (*extension)->background_url());
+    }
+  }
+}
+
+ExtensionProcessManager::ExtensionProcessManager(Profile* profile)
+    : browsing_instance_(new BrowsingInstance(profile)) {
+  // TODO: register notification, and load any hosts for existing exts.
+  NotificationService::current()->AddObserver(this,
+      NotificationType::EXTENSIONS_LOADED,
+      NotificationService::AllSources());
+
+  if (profile->GetExtensionsService()) {
+    CreateBackgroundHosts(this, profile->GetExtensionsService()->extensions());
+  }
+}
+
+ExtensionProcessManager::~ExtensionProcessManager() {
+  for (ExtensionHostList::iterator iter = background_hosts_.begin();
+      iter != background_hosts_.end(); ++iter) {
+    delete *iter;
+  }
+}
+
+ExtensionView* ExtensionProcessManager::CreateView(Extension* extension,
+                                                   const GURL& url,
+                                                   Browser* browser) {
+  return new ExtensionView(
+      new ExtensionHost(extension, GetSiteInstanceForURL(url)), browser, url);
+}
+
+void ExtensionProcessManager::CreateBackgroundHost(Extension* extension,
+                                                   const GURL& url) {
+  ExtensionHost* host =
+      new ExtensionHost(extension, GetSiteInstanceForURL(url));
+  host->CreateRenderView(url, NULL);  // create a RenderViewHost with no view
+  background_hosts_.push_back(host);
+}
+
+SiteInstance* ExtensionProcessManager::GetSiteInstanceForURL(const GURL& url) {
+  return browsing_instance_->GetSiteInstanceForURL(url);
+}
+
+void ExtensionProcessManager::Observe(NotificationType type,
+                                      const NotificationSource& source,
+                                      const NotificationDetails& details) {
+  DCHECK(type == NotificationType::EXTENSIONS_LOADED);
+  const ExtensionList* extensions = Details<ExtensionList>(details).ptr();
+  CreateBackgroundHosts(this, extensions);
+}
diff --git a/chrome/browser/extensions/extension_process_manager.h b/chrome/browser/extensions/extension_process_manager.h
new file mode 100644
index 0000000..73be45d
--- /dev/null
+++ b/chrome/browser/extensions/extension_process_manager.h
@@ -0,0 +1,59 @@
+// Copyright (c) 2009 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.
+
+#ifndef CHROME_BROWSER_EXTENSIONS_EXTENSION_PROCESS_MANAGER_H_
+#define CHROME_BROWSER_EXTENSIONS_EXTENSION_PROCESS_MANAGER_H_
+
+#include <list>
+
+#include "base/ref_counted.h"
+#include "chrome/common/notification_observer.h"
+
+class Browser;
+class BrowsingInstance;
+class Extension;
+class ExtensionHost;
+class ExtensionView;
+class GURL;
+class Profile;
+class SiteInstance;
+
+// Manages dynamic state of running Chromium extensions.  There is one instance
+// of this class per Profile (including OTR).
+class ExtensionProcessManager : public NotificationObserver {
+ public:
+  ExtensionProcessManager(Profile* profile);
+  ~ExtensionProcessManager();
+
+  // Creates a new ExtensionView, grouping it in the appropriate SiteInstance
+  // (and therefore process) based on the URL and profile.
+  ExtensionView* CreateView(Extension* extension,
+                            const GURL& url,
+                            Browser* browser);
+
+  // Creates a new UI-less extension instance.  Like CreateView, but not
+  // displayed anywhere.
+  void CreateBackgroundHost(Extension* extension, const GURL& url);
+
+  // Returns the SiteInstance that the given URL belongs to.
+  SiteInstance* GetSiteInstanceForURL(const GURL& url);
+
+  // NotificationObserver:
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
+
+ private:
+  // The list of running viewless background extensions.
+  typedef std::list<ExtensionHost*> ExtensionHostList;
+  ExtensionHostList background_hosts_;
+
+  // The BrowsingInstance shared by all extensions in this profile.  This
+  // controls process grouping.
+  scoped_refptr<BrowsingInstance> browsing_instance_;
+
+  DISALLOW_COPY_AND_ASSIGN(ExtensionProcessManager);
+};
+
+#endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_PROCESS_MANAGER_H_
diff --git a/chrome/browser/extensions/extension_process_manager_unittest.cc b/chrome/browser/extensions/extension_process_manager_unittest.cc
new file mode 100644
index 0000000..a72ac26
--- /dev/null
+++ b/chrome/browser/extensions/extension_process_manager_unittest.cc
@@ -0,0 +1,61 @@
+// Copyright (c) 2009 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/extensions/extension_error_reporter.h"
+#include "chrome/browser/extensions/extension_process_manager.h"
+#include "chrome/browser/tab_contents/site_instance.h"
+#include "chrome/test/testing_profile.h"
+#include "testing/gtest/include/gtest/gtest.h"
+#include "testing/platform_test.h"
+
+namespace {
+
+// make the test a PlatformTest to setup autorelease pools properly on mac
+class ExtensionProcessManagerTest : public testing::Test {
+ public:
+  static void SetUpTestCase() {
+    ExtensionErrorReporter::Init(false);  // no noisy errors
+  }
+
+  virtual void SetUp() {
+    ExtensionErrorReporter::GetInstance()->ClearErrors();
+  }
+};
+
+}
+
+// Test that extensions get grouped in the right SiteInstance (and therefore
+// process) based on their URLs.
+TEST_F(ExtensionProcessManagerTest, ProcessGrouping) {
+  // Extensions in different profiles should always be different SiteInstances.
+  // Note: we don't initialize these, since we're not testing that
+  // functionality.  This means we can get away with a NULL UserScriptMaster.
+  TestingProfile profile1(1);
+  scoped_ptr<ExtensionProcessManager> manager1(
+      new ExtensionProcessManager(&profile1));
+
+  TestingProfile profile2(2);
+  scoped_ptr<ExtensionProcessManager> manager2(
+      new ExtensionProcessManager(&profile2));
+
+  // Extensions with common origins ("scheme://id/") should be grouped in the
+  // same SiteInstance.
+  GURL ext1_url1("chrome-extensions://ext1_id/index.html");
+  GURL ext1_url2("chrome-extensions://ext1_id/toolstrips/toolstrip.html");
+  GURL ext2_url1("chrome-extensions://ext2_id/index.html");
+
+  scoped_refptr<SiteInstance> site11 =
+      manager1->GetSiteInstanceForURL(ext1_url1);
+  scoped_refptr<SiteInstance> site12 =
+      manager1->GetSiteInstanceForURL(ext1_url2);
+  EXPECT_EQ(site11, site12);
+
+  scoped_refptr<SiteInstance> site21 =
+      manager1->GetSiteInstanceForURL(ext2_url1);
+  EXPECT_NE(site11, site21);
+
+  scoped_refptr<SiteInstance> other_profile_site =
+      manager2->GetSiteInstanceForURL(ext1_url1);
+  EXPECT_NE(site11, other_profile_site);
+}
diff --git a/chrome/browser/extensions/extension_shelf.cc b/chrome/browser/extensions/extension_shelf.cc
index 1668252..f32d9a7 100644
--- a/chrome/browser/extensions/extension_shelf.cc
+++ b/chrome/browser/extensions/extension_shelf.cc
@@ -7,6 +7,7 @@
 #include "base/logging.h"
 #include "chrome/browser/browser.h"
 #include "chrome/browser/extensions/extension.h"
+#include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_view.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/profile.h"
@@ -150,14 +151,15 @@ void ExtensionShelf::Observe(NotificationType type,
 
 bool ExtensionShelf::AddExtensionViews(const ExtensionList* extensions) {
   bool added_toolstrip = false;
-  ExtensionsService* service = browser_->profile()->GetExtensionsService();
+  ExtensionProcessManager* manager =
+      browser_->profile()->GetExtensionProcessManager();
   for (ExtensionList::const_iterator extension = extensions->begin();
        extension != extensions->end(); ++extension) {
     for (std::vector<std::string>::const_iterator toolstrip_path =
          (*extension)->toolstrips().begin();
          toolstrip_path != (*extension)->toolstrips().end(); ++toolstrip_path) {
       ExtensionView* toolstrip =
-          service->CreateView(*extension,
+          manager->CreateView(*extension,
                               (*extension)->GetResourceURL(*toolstrip_path),
                               browser_);
       if (!background_.empty())
diff --git a/chrome/browser/extensions/extension_view_unittest.cc b/chrome/browser/extensions/extension_view_unittest.cc
index ca1643a..0efffa5 100755
--- a/chrome/browser/extensions/extension_view_unittest.cc
+++ b/chrome/browser/extensions/extension_view_unittest.cc
@@ -8,6 +8,7 @@
 #include "chrome/browser/extensions/extension_shelf.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_host.h"
+#include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extensions_service.h"
 #include "chrome/browser/extensions/test_extension_loader.h"
 #include "chrome/browser/tab_contents/site_instance.h"
@@ -103,7 +104,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionViewTest, Index) {
 
   // Start the extension process and wait for it to show a javascript alert.
   MockExtensionHost host(extension, url,
-      browser()->profile()->GetExtensionsService()->GetSiteInstanceForURL(url));
+      browser()->profile()->GetExtensionProcessManager()->
+          GetSiteInstanceForURL(url));
   EXPECT_TRUE(host.got_message());
 }
 
diff --git a/chrome/browser/extensions/extensions_service.cc b/chrome/browser/extensions/extensions_service.cc
index 0af48343..da5db0f1 100644
--- a/chrome/browser/extensions/extensions_service.cc
+++ b/chrome/browser/extensions/extensions_service.cc
@@ -16,16 +16,13 @@
 #include "chrome/browser/browser.h"
 #include "chrome/browser/browser_list.h"
 #include "chrome/browser/browser_process.h"
-#include "chrome/browser/browsing_instance.h"
 #include "chrome/browser/extensions/extension.h"
 #include "chrome/browser/extensions/extension_browser_event_router.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
-#include "chrome/browser/extensions/extension_host.h"
-#include "chrome/browser/extensions/extension_view.h"
+#include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/user_script_master.h"
 #include "chrome/browser/plugin_service.h"
 #include "chrome/browser/profile.h"
-#include "chrome/browser/tab_contents/site_instance.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/notification_service.h"
 #include "chrome/common/unzip.h"
@@ -81,16 +78,10 @@ ExtensionsService::ExtensionsService(Profile* profile,
     : message_loop_(MessageLoop::current()),
       install_directory_(profile->GetPath().AppendASCII(kInstallDirectoryName)),
       backend_(new ExtensionsServiceBackend(install_directory_)),
-      user_script_master_(user_script_master),
-      browsing_instance_(new BrowsingInstance(profile)) {
+      user_script_master_(user_script_master) {
 }
 
 ExtensionsService::~ExtensionsService() {
-  for (ExtensionHostList::iterator iter = background_hosts_.begin();
-      iter != background_hosts_.end(); ++iter) {
-    delete *iter;
-  }
-
   for (ExtensionList::iterator iter = extensions_.begin();
        iter != extensions_.end(); ++iter) {
     delete *iter;
@@ -172,11 +163,6 @@ void ExtensionsService::OnExtensionsLoaded(ExtensionList* new_extensions) {
          script != scripts.end(); ++script) {
       user_script_master_->AddLoneScript(*script);
     }
-
-    // Start the process for the master page, if it exists.
-    if ((*extension)->background_url().is_valid()) {
-      CreateBackgroundHost(*extension, (*extension)->background_url());
-    }
   }
 
   // Since user scripts may have changed, tell UserScriptMaster to kick off
@@ -208,25 +194,6 @@ Extension* ExtensionsService::GetExtensionByID(std::string id) {
   return NULL;
 }
 
-ExtensionView* ExtensionsService::CreateView(Extension* extension,
-                                             const GURL& url,
-                                             Browser* browser) {
-  return new ExtensionView(
-      new ExtensionHost(extension, GetSiteInstanceForURL(url)), browser, url);
-}
-
-void ExtensionsService::CreateBackgroundHost(Extension* extension,
-                                           const GURL& url) {
-  ExtensionHost* host =
-      new ExtensionHost(extension, GetSiteInstanceForURL(url));
-  host->CreateRenderView(url, NULL);  // create a RenderViewHost with no view
-  background_hosts_.push_back(host);
-}
-
-SiteInstance* ExtensionsService::GetSiteInstanceForURL(const GURL& url) {
-  return browsing_instance_->GetSiteInstanceForURL(url);
-}
-
 
 // ExtensionsServicesBackend
 
diff --git a/chrome/browser/extensions/extensions_service.h b/chrome/browser/extensions/extensions_service.h
index 67c8c4e..9a83c99 100644
--- a/chrome/browser/extensions/extensions_service.h
+++ b/chrome/browser/extensions/extensions_service.h
@@ -7,7 +7,6 @@
 
 #include <string>
 #include <vector>
-#include <list>
 
 #include "base/file_path.h"
 #include "base/message_loop.h"
@@ -16,10 +15,7 @@
 #include "base/values.h"
 
 class Browser;
-class BrowsingInstance;
 class Extension;
-class ExtensionHost;
-class ExtensionView;
 class ExtensionsServiceBackend;
 class GURL;
 class Profile;
@@ -82,19 +78,6 @@ class ExtensionsService : public ExtensionsServiceFrontendInterface {
   virtual void OnExtensionInstalled(Extension* extension, bool is_update);
   virtual Extension* GetExtensionByID(std::string id);
 
-  // Creates a new ExtensionView, grouping it in the appropriate SiteInstance
-  // (and therefore process) based on the URL and profile.
-  ExtensionView* CreateView(Extension* extension,
-                            const GURL& url,
-                            Browser* browser);
-
-  // Creates a new UI-less extension instance.  Like CreateView, but not
-  // displayed anywhere.
-  void CreateBackgroundHost(Extension* extension, const GURL& url);
-
-  // Returns the SiteInstance that the given URL belongs to.
-  SiteInstance* GetSiteInstanceForURL(const GURL& url);
-
   // The name of the file that the current active version number is stored in.
   static const char* kCurrentVersionFileName;
 
@@ -118,14 +101,6 @@ class ExtensionsService : public ExtensionsServiceFrontendInterface {
   // The user script master for this profile.
   scoped_refptr<UserScriptMaster> user_script_master_;
 
-  // The BrowsingInstance shared by all extensions in this profile.  This
-  // controls process grouping.
-  scoped_refptr<BrowsingInstance> browsing_instance_;
-
-  // The list of running viewless background extensions.
-  typedef std::list<ExtensionHost*> ExtensionHostList;
-  ExtensionHostList background_hosts_;
-
   DISALLOW_COPY_AND_ASSIGN(ExtensionsService);
 };
 
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index 6e1e614..e6c27df 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -14,11 +14,9 @@
 #include "chrome/browser/extensions/extension.h"
 #include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extensions_service.h"
-#include "chrome/browser/tab_contents/site_instance.h"
 #include "chrome/common/extensions/url_pattern.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/json_value_serializer.h"
-#include "chrome/test/testing_profile.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "testing/platform_test.h"
 
@@ -358,38 +356,3 @@ TEST_F(ExtensionsServiceTest, GenerateID) {
   ASSERT_EQ("chrome-extension://0000000000000000000000000000000000000001/",
             frontend->extensions()->at(1)->url().spec());
 }
-
-// Test that extensions get grouped in the right SiteInstance (and therefore
-// process) based on their URLs.
-TEST_F(ExtensionsServiceTest, ProcessGrouping) {
-  // Extensions in different profiles should always be different SiteInstances.
-  // Note: we don't initialize these, since we're not testing that
-  // functionality.  This means we can get away with a NULL UserScriptMaster.
-  TestingProfile profile1(1);
-  scoped_refptr<ExtensionsService> frontend1(
-      new ExtensionsService(&profile1, NULL));
-
-  TestingProfile profile2(2);
-  scoped_refptr<ExtensionsService> frontend2(
-      new ExtensionsService(&profile2, NULL));
-
-  // Extensions with common origins ("scheme://id/") should be grouped in the
-  // same SiteInstance.
-  GURL ext1_url1("chrome-extensions://ext1_id/index.html");
-  GURL ext1_url2("chrome-extensions://ext1_id/toolstrips/toolstrip.html");
-  GURL ext2_url1("chrome-extensions://ext2_id/index.html");
-
-  scoped_refptr<SiteInstance> site11 =
-      frontend1->GetSiteInstanceForURL(ext1_url1);
-  scoped_refptr<SiteInstance> site12 =
-      frontend1->GetSiteInstanceForURL(ext1_url2);
-  EXPECT_EQ(site11, site12);
-
-  scoped_refptr<SiteInstance> site21 =
-      frontend1->GetSiteInstanceForURL(ext2_url1);
-  EXPECT_NE(site11, site21);
-
-  scoped_refptr<SiteInstance> other_profile_site =
-      frontend2->GetSiteInstanceForURL(ext1_url1);
-  EXPECT_NE(site11, other_profile_site);
-}