Take extensions out of Profile into a profile-keyed service, ExtensionSystem.

chrome/browser/profiles/profile_impl.cc

Index: chrome/browser/profiles/profile_impl.cc
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 32cc7316531b2cb00cd404febc7b83f9e53a32c2..589ccdfe04dabc487714377d5d63cb1391018afd 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -30,18 +30,14 @@
 #include "chrome/browser/defaults.h"
 #include "chrome/browser/download/download_service.h"
 #include "chrome/browser/download/download_service_factory.h"
-#include "chrome/browser/extensions/component_loader.h"
-#include "chrome/browser/extensions/extension_devtools_manager.h"
-#include "chrome/browser/extensions/extension_error_reporter.h"
 #include "chrome/browser/extensions/extension_event_router.h"
-#include "chrome/browser/extensions/extension_info_map.h"
-#include "chrome/browser/extensions/extension_message_service.h"
-#include "chrome/browser/extensions/extension_navigation_observer.h"
 #include "chrome/browser/extensions/extension_pref_store.h"
+#include "chrome/browser/extensions/extension_pref_value_map.h"
 #include "chrome/browser/extensions/extension_process_manager.h"
 #include "chrome/browser/extensions/extension_service.h"
 #include "chrome/browser/extensions/extension_special_storage_policy.h"
-#include "chrome/browser/extensions/unpacked_installer.h"
+#include "chrome/browser/extensions/extension_system.h"
+#include "chrome/browser/extensions/extension_system_factory.h"
 #include "chrome/browser/extensions/user_script_master.h"
 #include "chrome/browser/favicon/favicon_service.h"
 #include "chrome/browser/geolocation/chrome_geolocation_permission_context.h"
@@ -249,7 +245,6 @@ ProfileImpl::ProfileImpl(const FilePath& path,
       ALLOW_THIS_IN_INITIALIZER_LIST(visited_link_event_listener_(
           new VisitedLinkEventListener(this))),
       ALLOW_THIS_IN_INITIALIZER_LIST(io_data_(this)),
-      extension_devtools_manager_(NULL),
       host_content_settings_map_(NULL),
       host_zoom_map_(NULL),
       history_service_created_(false),
@@ -421,103 +416,6 @@ void ProfileImpl::DoFinalInit() {
       content::NotificationService::NoDetails());
 }
 
-void ProfileImpl::InitExtensions(bool extensions_enabled) {
-  if (user_script_master_ || extension_service_.get())
-    return;  // Already initialized.
-
-  const CommandLine* command_line = CommandLine::ForCurrentProcess();
-  if (command_line->HasSwitch(
-      switches::kEnableExtensionTimelineApi)) {
-    extension_devtools_manager_ = new ExtensionDevToolsManager(this);
-  }
-
-  // The ExtensionInfoMap needs to be created before the
-  // ExtensionProcessManager.
-  extension_info_map_ = new ExtensionInfoMap();
-  extension_process_manager_.reset(ExtensionProcessManager::Create(this));
-  extension_event_router_.reset(new ExtensionEventRouter(this));
-  extension_message_service_ = new ExtensionMessageService(this);
-  extension_navigation_observer_.reset(new ExtensionNavigationObserver(this));
-
-  ExtensionErrorReporter::Init(true);  // allow noisy errors.
-
-  user_script_master_ = new UserScriptMaster(this);
-
-  bool autoupdate_enabled = true;
-#if defined(OS_CHROMEOS)
-  if (!extensions_enabled)
-    autoupdate_enabled = false;
-  else
-    autoupdate_enabled = !command_line->HasSwitch(switches::kGuestSession);
-#endif
-  extension_service_.reset(new ExtensionService(
-      this,
-      CommandLine::ForCurrentProcess(),
-      GetPath().AppendASCII(ExtensionService::kInstallDirectoryName),
-      extension_prefs_.get(),
-      autoupdate_enabled,
-      extensions_enabled));
-
-  extension_service_->component_loader()->AddDefaultComponentExtensions();
-  if (command_line->HasSwitch(switches::kLoadComponentExtension)) {
-    CommandLine::StringType path_list = command_line->GetSwitchValueNative(
-        switches::kLoadComponentExtension);
-    StringTokenizerT<CommandLine::StringType,
-        CommandLine::StringType::const_iterator> t(path_list,
-                                                   FILE_PATH_LITERAL(","));
-    while (t.GetNext()) {
-      // Load the component extension manifest synchronously.
-      // Blocking the UI thread is acceptable here since
-      // this flag designated for developers.
-      base::ThreadRestrictions::ScopedAllowIO allow_io;
-      extension_service_->component_loader()->AddOrReplace(
-          FilePath(t.token()));
-    }
-  }
-  extension_service_->Init();
-
-  if (extensions_enabled) {
-    // Load any extensions specified with --load-extension.
-    // TODO(yoz): Seems like this should move into ExtensionService::Init.
-    if (command_line->HasSwitch(switches::kLoadExtension)) {
-      CommandLine::StringType path_list = command_line->GetSwitchValueNative(
-          switches::kLoadExtension);
-      StringTokenizerT<CommandLine::StringType,
-          CommandLine::StringType::const_iterator> t(path_list,
-                                                     FILE_PATH_LITERAL(","));
-      scoped_refptr<extensions::UnpackedInstaller> installer =
-          extensions::UnpackedInstaller::Create(extension_service_.get());
-      while (t.GetNext()) {
-        installer->LoadFromCommandLine(FilePath(t.token()));
-      }
-    }
-  }
-
-  // Make the chrome://extension-icon/ resource available.
-  GetChromeURLDataManager()->AddDataSource(new ExtensionIconSource(this));
-
-  // Initialize extension event routers. Note that on Chrome OS, this will
-  // not succeed if the user has not logged in yet, in which case the
-  // event routers are initialized in LoginUtilsImpl::CompleteLogin instead.
-  // The InitEventRouters call used to be in BrowserMain, because when bookmark
-  // import happened on first run, the bookmark bar was not being correctly
-  // initialized (see issue 40144). Now that bookmarks aren't imported and
-  // the event routers need to be initialized for every profile individually,
-  // initialize them with the extension service.
-  // If this profile is being created as part of the import process, never
-  // initialize the event routers. If import is going to run in a separate
-  // process (the profile itself is on the main process), wait for import to
-  // finish before initializing the routers.
-  if (!command_line->HasSwitch(switches::kImport) &&
-      !command_line->HasSwitch(switches::kImportFromFile)) {
-    if (g_browser_process->profile_manager()->will_import()) {
-      extension_service_->InitEventRoutersAfterImport();
-    } else {
-      extension_service_->InitEventRouters();
-    }
-  }
-}
-
 void ProfileImpl::InitPromoResources() {
   if (promo_resource_service_)
     return;
@@ -627,9 +525,6 @@ ProfileImpl::~ProfileImpl() {
   // HistoryService first.
   favicon_service_.reset();
 
-  if (extension_message_service_)
-    extension_message_service_->DestroyingProfile();
-
   if (pref_proxy_config_tracker_.get())
     pref_proxy_config_tracker_->DetachFromPrefService();
 
@@ -704,27 +599,19 @@ VisitedLinkMaster* ProfileImpl::GetVisitedLinkMaster() {
 }
 
 ExtensionService* ProfileImpl::GetExtensionService() {

[Miranda Callahan, 2012/02/09 22:10:04]

Will it eventually be possible to remove this method, and the three below? I
just did a cursory glance through the code, and it looks like this will be a
ridiculous monster patch; but it would be nice to remove the factory entirely
from the Profile, to greatly increase its modularity.

And maybe in the meantime add big warnings that these functions are deprecated,
and users should directly pass the ExtensionSystemFactory a profile pointer to
get an ExtensionService*?

[Yoyo Zhou, 2012/02/16 21:49:20]

Right, this would an even more enormous change if I tried to remove all of these
getters at once. I've got TODOs in profile.h, but I don't know where the
warnings are most appropriate; I could stick them there too.

-  return extension_service_.get();
+  return ExtensionSystemFactory::GetForProfile(this)->extension_service();
 }
 
 UserScriptMaster* ProfileImpl::GetUserScriptMaster() {
-  return user_script_master_.get();
-}
-
-ExtensionDevToolsManager* ProfileImpl::GetExtensionDevToolsManager() {
-  return extension_devtools_manager_.get();
+  return ExtensionSystemFactory::GetForProfile(this)->user_script_master();
 }
 
 ExtensionProcessManager* ProfileImpl::GetExtensionProcessManager() {
-  return extension_process_manager_.get();
-}
-
-ExtensionMessageService* ProfileImpl::GetExtensionMessageService() {
-  return extension_message_service_.get();
+  return ExtensionSystemFactory::GetForProfile(this)->process_manager();
 }
 
 ExtensionEventRouter* ProfileImpl::GetExtensionEventRouter() {
-  return extension_event_router_.get();
+  return ExtensionSystemFactory::GetForProfile(this)->event_router();
 }
 
 ExtensionSpecialStoragePolicy*
@@ -775,12 +662,9 @@ void ProfileImpl::OnPrefsLoaded(bool success) {
   ProfileDependencyManager::GetInstance()->CreateProfileServices(this, false);
 
   // Ensure that preferences set by extensions are restored in the profile
-  // as early as possible. The constructor takes care of that.
-  extension_prefs_.reset(new ExtensionPrefs(
-      prefs_.get(),
-      GetPath().AppendASCII(ExtensionService::kInstallDirectoryName),
-      GetExtensionPrefValueMap()));
-  extension_prefs_->Init(extensions_disabled);
+  // as early as possible. The ExtensionPrefs constructor takes care of that.
+  ExtensionSystemFactory::GetForProfile(this)->InitPrefs(
+      extensions_disabled, GetExtensionPrefValueMap());

[Elliot Glaysher, 2012/02/09 20:10:24]

In both the OTRProfileImpl and here, you have this block of extension
initialization. Can this be moved into the factory? A large motivation for the
factorization (other than shutdown crashing) was moving per-service
initialization out of the profile implementations so it would be easier for, ie,
Chrome for Android to just not compile the ExtensionService in without ifdefing
the Profile code.

[Yoyo Zhou, 2012/02/16 21:49:20]

Good point. I think this should be possible. But I think
GetExtensionPrefValueMap would have to be public, right? Is there a workaround
for that?

 
   DCHECK(!net_pref_observer_.get());
   net_pref_observer_.reset(new NetPrefObserver(
@@ -826,8 +710,10 @@ net::URLRequestContextGetter* ProfileImpl::GetRequestContext() {
 
 net::URLRequestContextGetter* ProfileImpl::GetRequestContextForRenderProcess(
     int renderer_child_id) {
-  if (extension_service_.get()) {
-    const Extension* installed_app = extension_service_->
+  ExtensionService* extension_service =
+      ExtensionSystemFactory::GetForProfile(this)->extension_service();
+  if (extension_service) {
+    const Extension* installed_app = extension_service->
         GetInstalledAppForRenderer(renderer_child_id);
     if (installed_app != NULL && installed_app->is_storage_isolated() &&
         installed_app->HasAPIPermission(
@@ -863,31 +749,6 @@ net::URLRequestContextGetter* ProfileImpl::GetRequestContextForIsolatedApp(
   return io_data_.GetIsolatedAppRequestContextGetter(app_id);
 }
 
-void ProfileImpl::RegisterExtensionWithRequestContexts(
-    const Extension* extension) {
-  base::Time install_time;
-  if (extension->location() != Extension::COMPONENT) {
-    install_time = GetExtensionService()->extension_prefs()->
-        GetInstallTime(extension->id());
-  }
-  bool incognito_enabled =
-      GetExtensionService()->IsIncognitoEnabled(extension->id());
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&ExtensionInfoMap::AddExtension, extension_info_map_.get(),
-                 make_scoped_refptr(extension), install_time,
-                 incognito_enabled));
-}
-
-void ProfileImpl::UnregisterExtensionWithRequestContexts(
-    const std::string& extension_id,
-    const extension_misc::UnloadedExtensionReason reason) {
-  BrowserThread::PostTask(
-      BrowserThread::IO, FROM_HERE,
-      base::Bind(&ExtensionInfoMap::RemoveExtension, extension_info_map_.get(),
-                 extension_id, reason));
-}
-
 net::SSLConfigService* ProfileImpl::GetSSLConfigService() {
   return ssl_config_service_manager_->Get();
 }
@@ -1362,10 +1223,6 @@ ChromeBlobStorageContext* ProfileImpl::GetBlobStorageContext() {
   return blob_storage_context_;
 }
 
-ExtensionInfoMap* ProfileImpl::GetExtensionInfoMap() {
-  return extension_info_map_.get();
-}
-
 ChromeURLDataManager* ProfileImpl::GetChromeURLDataManager() {
   if (!chrome_url_data_manager_.get())
     chrome_url_data_manager_.reset(new ChromeURLDataManager(