Introducing ExtensionProcessManager. This manages the ExtensionViews to

ensure there is only 1 process per extension.

I also changed ExtensionMessageService from singleton to one instance per
Profile.  This means messages can only be passed to extensions and scripts
within the same profile.
Review URL: http://codereview.chromium.org/62132

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

8 files changed, 188 insertions, 21 deletions

diff --git a/chrome/browser/extensions/extension_message_service.cc b/chrome/browser/extensions/extension_message_service.cc
index 87469f9..ef2530a 100755
--- a/chrome/browser/extensions/extension_message_service.cc
+++ b/chrome/browser/extensions/extension_message_service.cc
@@ -12,19 +12,41 @@
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/browser/renderer_host/resource_message_filter.h"
 #include "chrome/common/render_messages.h"
+#include "chrome/common/stl_util-inl.h"
 
 // Since we have 2 ports for every channel, we just index channels by half the
 // port ID.
-#define GET_CHANNEL_ID(port_id) (port_id / 2)
+#define GET_CHANNEL_ID(port_id) ((port_id) / 2)
 
 // Port1 is always even, port2 is always odd.
-#define IS_PORT1_ID(port_id) ((port_id & 1) == 0)
+#define IS_PORT1_ID(port_id) (((port_id) & 1) == 0)
 
 // Change even to odd and vice versa, to get the other side of a given channel.
-#define GET_OPPOSITE_PORT_ID(source_port_id) (source_port_id ^ 1)
+#define GET_OPPOSITE_PORT_ID(source_port_id) ((source_port_id) ^ 1)
 
-ExtensionMessageService* ExtensionMessageService::GetInstance() {
-  return Singleton<ExtensionMessageService>::get();
+namespace {
+typedef std::map<URLRequestContext*, ExtensionMessageService*> InstanceMap;
+struct SingletonData {
+  ~SingletonData() {
+    STLDeleteContainerPairSecondPointers(map.begin(), map.end());
+  }
+  Lock lock;
+  InstanceMap map;
+};
+}  // namespace
+
+// static
+ExtensionMessageService* ExtensionMessageService::GetInstance(
+    URLRequestContext* context) {
+  SingletonData* data = Singleton<SingletonData>::get();
+  AutoLock lock(data->lock);
+
+  ExtensionMessageService* instance = data->map[context];
+  if (!instance) {
+    instance = new ExtensionMessageService();
+    data->map[context] = instance;
+  }
+  return instance;
 }
 
 ExtensionMessageService::ExtensionMessageService()
@@ -37,7 +59,8 @@ void ExtensionMessageService::RegisterExtension(
   // TODO(mpcomplete): We need to ensure an extension always ends up in a single
   // process.  I think this means having an ExtensionProcessManager which holds
   // a BrowsingContext for each extension.
-  //DCHECK(process_ids_.find(extension_id) == process_ids_.end());
+  DCHECK(process_ids_.find(extension_id) == process_ids_.end() ||
+         process_ids_[extension_id] == render_process_id);
   process_ids_[extension_id] = render_process_id;
 }
 
diff --git a/chrome/browser/extensions/extension_message_service.h b/chrome/browser/extensions/extension_message_service.h
index 0020a30..a63837a 100755
--- a/chrome/browser/extensions/extension_message_service.h
+++ b/chrome/browser/extensions/extension_message_service.h
@@ -12,6 +12,7 @@
 
 class ExtensionView;
 class ResourceMessageFilter;
+class URLRequestContext;
 
 // This class manages message passing between renderer processes.  It maintains
 // a list of available extensions and which renderers each lives in, as well as
@@ -24,7 +25,9 @@ class ResourceMessageFilter;
 //   messages on the IO thread.
 class ExtensionMessageService {
  public:
-  static ExtensionMessageService* GetInstance();
+  // Returns the message service for the given context.  Messages can only
+  // be sent within a single context.
+  static ExtensionMessageService* GetInstance(URLRequestContext* context);
 
   ExtensionMessageService();
 
diff --git a/chrome/browser/extensions/extension_process_manager.cc b/chrome/browser/extensions/extension_process_manager.cc
new file mode 100755
index 0000000..245450c
--- /dev/null
+++ b/chrome/browser/extensions/extension_process_manager.cc
@@ -0,0 +1,42 @@
+// 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 "base/singleton.h"
+#include "chrome/browser/extensions/extension_view.h"
+#include "chrome/browser/tab_contents/site_instance.h"
+
+// static
+ExtensionProcessManager* ExtensionProcessManager::GetInstance() {
+  return Singleton<ExtensionProcessManager>::get();
+}
+
+ExtensionProcessManager::ExtensionProcessManager() {
+}
+
+ExtensionProcessManager::~ExtensionProcessManager() {
+}
+
+ExtensionView* ExtensionProcessManager::CreateView(Extension* extension,
+                                                   const GURL& url,
+                                                   Profile* profile) {
+  return new ExtensionView(extension, url, GetSiteInstanceForURL(url, profile));
+}
+
+SiteInstance* ExtensionProcessManager::GetSiteInstanceForURL(
+    const GURL& url, Profile* profile) {
+  BrowsingInstance* browsing_instance = GetBrowsingInstance(profile);
+  return browsing_instance->GetSiteInstanceForURL(url);
+}
+
+BrowsingInstance* ExtensionProcessManager::GetBrowsingInstance(
+    Profile* profile) {
+  BrowsingInstance* instance = browsing_instance_map_[profile];
+  if (!instance) {
+    instance = new BrowsingInstance(profile);
+    browsing_instance_map_[profile] = instance;
+  }
+  return instance;
+}
diff --git a/chrome/browser/extensions/extension_process_manager.h b/chrome/browser/extensions/extension_process_manager.h
new file mode 100755
index 0000000..0d4048b
--- /dev/null
+++ b/chrome/browser/extensions/extension_process_manager.h
@@ -0,0 +1,54 @@
+// 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 "base/ref_counted.h"
+
+#include <map>
+
+class BrowsingInstance;
+class Extension;
+class ExtensionView;
+class GURL;
+class Profile;
+class SiteInstance;
+
+// This class controls what process new extension instances use.  We use the
+// BrowsingInstance site grouping rules to group extensions.  Since all
+// resources in a given extension have the same origin, they will be grouped
+// into the same process.
+//
+// We separate further by Profile: each Profile has its own group of extension
+// processes that never overlap with other Profiles.
+class ExtensionProcessManager {
+ public:
+  static ExtensionProcessManager* GetInstance();
+
+  // These are public for use by Singleton.  Do not instantiate or delete
+  // manually.
+  ExtensionProcessManager();
+  ~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,
+                            Profile* profile);
+
+  // Returns the SiteInstance that the given URL belongs to in this profile.
+  SiteInstance* GetSiteInstanceForURL(const GURL& url, Profile* profile);
+ private:
+  // Returns our BrowsingInstance for the given profile.  Lazily created and
+  // cached.
+  BrowsingInstance* GetBrowsingInstance(Profile* profile);
+
+  // Cache of BrowsingInstances grouped by Profile.
+  typedef std::map<Profile*, scoped_refptr<BrowsingInstance> >
+      BrowsingInstanceMap;
+  BrowsingInstanceMap browsing_instance_map_;
+};
+
+#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 100755
index 0000000..ae43367
--- /dev/null
+++ b/chrome/browser/extensions/extension_process_manager_unittest.cc
@@ -0,0 +1,42 @@
+// 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 "testing/gtest/include/gtest/gtest.h"
+
+#include "chrome/test/testing_profile.h"
+#include "chrome/browser/extensions/extension_process_manager.h"
+#include "chrome/browser/tab_contents/site_instance.h"
+
+class ExtensionProcessManagerTest : public testing::Test {
+};
+
+// Test that extensions get grouped in the right SiteInstance (and therefore
+// process) based on their URLs.
+TEST(ExtensionProcessManagerTest, ProcessGrouping) {
+  ExtensionProcessManager* manager = ExtensionProcessManager::GetInstance();
+
+  // Extensions in different profiles should always be different SiteInstances.
+  TestingProfile profile1(1);
+  TestingProfile profile2(2);
+
+  // 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 =
+      manager->GetSiteInstanceForURL(ext1_url1, &profile1);
+  scoped_refptr<SiteInstance> site12 =
+      manager->GetSiteInstanceForURL(ext1_url2, &profile1);
+  EXPECT_EQ(site11, site12);
+
+  scoped_refptr<SiteInstance> site21 =
+      manager->GetSiteInstanceForURL(ext2_url1, &profile1);
+  EXPECT_NE(site11, site21);
+
+  scoped_refptr<SiteInstance> other_profile_site =
+      manager->GetSiteInstanceForURL(ext1_url1, &profile2);
+  EXPECT_NE(site11, other_profile_site);
+}
diff --git a/chrome/browser/extensions/extension_view.cc b/chrome/browser/extensions/extension_view.cc
index 11c33cf..969a046 100755
--- a/chrome/browser/extensions/extension_view.cc
+++ b/chrome/browser/extensions/extension_view.cc
@@ -6,6 +6,7 @@
 
 #include "chrome/browser/extensions/extension.h"
 #include "chrome/browser/extensions/extension_message_service.h"
+#include "chrome/browser/profile.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/renderer_host/render_process_host.h"
 #include "chrome/common/resource_bundle.h"
@@ -15,10 +16,9 @@
 
 ExtensionView::ExtensionView(Extension* extension,
                              const GURL& url,
-                             Profile* profile)
-    : HWNDHtmlView(url, this, false),
-      extension_(extension),
-      profile_(profile) {
+                             SiteInstance* instance)
+    : HWNDHtmlView(url, this, false, instance),
+      extension_(extension) {
   // Set the width initially to 0, so that the WebCore::Document can
   // correctly compute the minPrefWidth which is returned in
   // DidContentsChangeSize()
@@ -58,7 +58,8 @@ void ExtensionView::CreatingRenderer() {
 }
 
 void ExtensionView::RenderViewCreated(RenderViewHost* rvh) {
-  ExtensionMessageService::GetInstance()->RegisterExtension(
+  URLRequestContext* context = rvh->process()->profile()->GetRequestContext();
+  ExtensionMessageService::GetInstance(context)->RegisterExtension(
       extension_->id(), render_view_host()->process()->pid());
 }
 
diff --git a/chrome/browser/extensions/extension_view.h b/chrome/browser/extensions/extension_view.h
index 960a294..09c7788 100755
--- a/chrome/browser/extensions/extension_view.h
+++ b/chrome/browser/extensions/extension_view.h
@@ -15,7 +15,6 @@
 #endif
 
 class Extension;
-class Profile;
 struct WebPreferences;
 
 // This class is the browser component of an extension component's RenderView.
@@ -25,13 +24,14 @@ struct WebPreferences;
 class ExtensionView : public HWNDHtmlView,
                       public RenderViewHostDelegate {
  public:
-  ExtensionView(Extension* extension, const GURL& url, Profile* profile);
+  ExtensionView(Extension* extension, const GURL& url, SiteInstance* instance);
 
   // HWNDHtmlView
   virtual void CreatingRenderer();
 
   // RenderViewHostDelegate
-  virtual Profile* GetProfile() const { return profile_; }
+  // TODO(mpcomplete): GetProfile is unused.
+  virtual Profile* GetProfile() const { return NULL; }
   virtual void RenderViewCreated(RenderViewHost* render_view_host);
   virtual void DidContentsPreferredWidthChange(const int pref_width);
   virtual void DidStopLoading(RenderViewHost* render_view_host,
@@ -52,9 +52,6 @@ class ExtensionView : public HWNDHtmlView,
   // The extension that we're hosting in this view.
   Extension* extension_;
 
-  // The profile that owns this extension.
-  Profile* profile_;
-
   DISALLOW_COPY_AND_ASSIGN(ExtensionView);
 };
 
diff --git a/chrome/browser/extensions/extension_view_unittest.cc b/chrome/browser/extensions/extension_view_unittest.cc
index 69a1fb9..d3d52bf 100755
--- a/chrome/browser/extensions/extension_view_unittest.cc
+++ b/chrome/browser/extensions/extension_view_unittest.cc
@@ -6,9 +6,11 @@
 #include "chrome/browser/browser.h"
 #include "chrome/browser/renderer_host/render_view_host.h"
 #include "chrome/browser/extensions/extension_error_reporter.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/extensions/test_extension_loader.h"
+#include "chrome/browser/tab_contents/site_instance.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/test/in_process_browser_test.h"
 #include "chrome/test/ui_test_utils.h"
@@ -28,8 +30,9 @@ const char* kExtensionId = "00123456789abcdef0123456789abcdef0123456";
 // up a javascript alert.
 class MockExtensionView : public ExtensionView {
  public:
-  MockExtensionView(Extension* extension, const GURL& url, Profile* profile)
-      : ExtensionView(extension, url, profile), got_message_(false) {
+  MockExtensionView(Extension* extension, const GURL& url,
+                    SiteInstance* instance)
+      : ExtensionView(extension, url, instance), got_message_(false) {
     InitHidden();
     MessageLoop::current()->PostDelayedTask(FROM_HERE,
         new MessageLoop::QuitTask, kAlertTimeoutMs);
@@ -88,6 +91,8 @@ IN_PROC_BROWSER_TEST_F(ExtensionViewTest, Index) {
   GURL url = Extension::GetResourceURL(extension->url(), "toolstrip1.html");
 
   // Start the extension process and wait for it to show a javascript alert.
-  MockExtensionView view(extension, url, browser()->profile());
+  MockExtensionView view(
+      extension, url, ExtensionProcessManager::GetInstance()->
+          GetSiteInstanceForURL(url, browser()->profile()));
   EXPECT_TRUE(view.got_message());
 }