Refactor loading out of ExtensionService.

chrome/browser/extensions/extension_service.h

Index: chrome/browser/extensions/extension_service.h
diff --git a/chrome/browser/extensions/extension_service.h b/chrome/browser/extensions/extension_service.h
index b0720f39bb73a965bf888ad7f9bb67c29a68f1e5..d5ebf96ba199f12bbe8407fcdc060bc0a82a8b2d 100644
--- a/chrome/browser/extensions/extension_service.h
+++ b/chrome/browser/extensions/extension_service.h
@@ -45,6 +45,7 @@
 #include "content/public/browser/notification_registrar.h"
 
 class AppNotificationManager;
+class ComponentLoader;
 class BookmarkExtensionEventRouter;
 class CrxInstaller;
 class ExtensionBrowserEventRouter;
@@ -53,19 +54,20 @@ class ExtensionCookiesEventRouter;
 class ExtensionDownloadsEventRouter;
 class ExtensionFileBrowserEventRouter;
 class ExtensionGlobalError;
-class HistoryExtensionEventRouter;
 class ExtensionInstallUI;
 class ExtensionManagementEventRouter;
 class ExtensionPreferenceEventRouter;
-class ExtensionServiceBackend;
 class ExtensionSyncData;
 class ExtensionToolbarModel;
 class ExtensionUpdater;
 class ExtensionWebNavigationEventRouter;
+class HistoryExtensionEventRouter;
+class InstalledExtensionLoader;
 class GURL;
 class PendingExtensionManager;
 class Profile;
 class SyncData;
+class UnpackedInstaller;
 class Version;
 
 namespace chromeos {
@@ -118,24 +120,6 @@ class ExtensionService
       public ExternalExtensionProviderInterface::VisitorInterface,
       public content::NotificationObserver {
  public:
-  // Information about a registered component extension.
-  struct ComponentExtensionInfo {
-    ComponentExtensionInfo(const std::string& manifest,
-                           const FilePath& root_directory)
-        : manifest(manifest),
-          root_directory(root_directory) {
-    }
-
-    bool Equals(const ComponentExtensionInfo& other) const;
-
-    // The extension's manifest. This is required for component extensions so
-    // that ExtensionService doesn't need to go to disk to load them.
-    std::string manifest;
-
-    // Directory where the extension is stored.
-    FilePath root_directory;
-  };
-
   // The name of the directory inside the profile where extensions are
   // installed to.
   static const char* kInstallDirectoryName;
@@ -190,14 +174,6 @@ class ExtensionService
   // Gets the object managing the set of pending extensions.
   virtual PendingExtensionManager* pending_extension_manager() OVERRIDE;
 
-  // Registers an extension to be loaded as a component extension.
-  void register_component_extension(const ComponentExtensionInfo& info) {
-    component_extension_manifests_.push_back(info);
-  }
-
-  // Unregisters a component extension from the list of extensions to be loaded
-  void UnregisterComponentExtension(const ComponentExtensionInfo& info);
-
   const FilePath& install_directory() const { return install_directory_; }
 
   AppsPromo* apps_promo() { return &apps_promo_; }
@@ -315,28 +291,6 @@ class ExtensionService
   void UpdateActivePermissions(const Extension* extension,
                                const ExtensionPermissionSet* permissions);
 
-  // Loads the extension from the directory |extension_path|.
-  void LoadExtension(const FilePath& extension_path);
-
-  // Loads the extension from the directory |extension_path|.
-  // This version of this method is intended for testing only.
-  void LoadExtension(const FilePath& extension_path, bool prompt_for_plugins);
-
-  // Same as above, but for use with command line switch --load-extension=path.
-  void LoadExtensionFromCommandLine(const FilePath& extension_path);
-
-  // Loads any component extensions.
-  void LoadComponentExtensions();
-
-  // Loads particular component extension.
-  const Extension* LoadComponentExtension(const ComponentExtensionInfo& info);
-
-  // Unloads particular component extension.
-  void UnloadComponentExtension(const ComponentExtensionInfo& info);
-
-  // Loads all known extensions (used by startup and testing code).
-  void LoadAllExtensions();
-
   // Check for updates (or potentially new extensions from external providers)
   void CheckForExternalUpdates();
 
@@ -391,10 +345,6 @@ class ExtensionService
   // been loaded from a file and installed.
   void AddExtension(const Extension* extension);
 
-  // Called by the backend when an unpacked extension has been loaded.
-  void OnLoadSingleExtension(const Extension* extension,
-                             bool prompt_for_plugins);
-
   // Called by the backend when an extension has been installed.
   void OnExtensionInstalled(
       const Extension* extension, bool from_webstore, int page_index);
@@ -451,6 +401,8 @@ class ExtensionService
   // TODO(skerner): Get rid of this method.  crbug.com/63756
   bool is_ready() { return ready_; }
 
+  ComponentLoader* component_loader() { return component_loader_.get(); }
+
   // Note that this may return NULL if autoupdate is not turned on.
   ExtensionUpdater* updater();
 
@@ -482,7 +434,7 @@ class ExtensionService
 #endif
 
   // Notify the frontend that there was an error loading an extension.
-  // This method is public because ExtensionServiceBackend can post to here.
+  // This method is public because ExtensionLoadTask can post to here.

[Aaron Boodman, 2011/10/28 22:21:58]

TODO(aa): DIEDIEDIE

   void ReportExtensionLoadError(const FilePath& extension_path,
                                 const std::string& error,
                                 bool be_noisy);
@@ -558,6 +510,10 @@ class ExtensionService
   // |client| can be NULL for a silent install.
   scoped_refptr<CrxInstaller> MakeCrxInstaller(ExtensionInstallUI* client);
 
+  scoped_refptr<UnpackedInstaller> MakeUnpackedInstaller();

[Aaron Boodman, 2011/10/28 22:21:58]

It seems a little unfortunate to me to have to have these methods in here.

I'm not super familiar with WeakPtr yet, but I think that if you have ES inherit
SupportsWeakPtr, you can just do:

new InstalledExtensionLoader(extension_service_->AsWeakPtr())

at the callsite. This would be a little more nicely decoupled IMO.

[Yoyo Zhou, 2011/10/31 21:58:15]

Heh, I went back and forth on this myself. I did look into weak_ptr.h and it
works as you suggest. Originally I thought I'd have to create an object outside
the ES, and I'd have no choice but to use SupportsWeakPtr. But then I didn't.

I then found akalin's change where he added WeakPtrFactory to ES, and he wrote
this:

"Yeah, I thought of making ExtensionService inherit from base::SupportsWeakPtr
(which adds an AsWeakPtr() method) but ultimately decided against it.  The only
things that really need the weakptr are ExtensionBackend and CrxInstaller;
everything else is tied to the profile's lifetime.  It seems like an
implementation detail that the service passes itself as a weak ptr to
CrxInstaller so I thought it better to hide it behind a function."

Of course, now that we have 3, we should perhaps reassess.

[Aaron Boodman, 2011/10/31 22:33:25]

Can you try it? I'm really fired up about the idea of not having these depend on
ExtensionService long-term.

[Yoyo Zhou, 2011/11/01 21:50:57]

Got rid of these two:
1. changed ES::MakeUnpackedInstaller to UnpackedInstaller::Create
2. made InstalledLoader just a regular object (it lives only one one thread); it
looks a little odd though

If this looks ok (see ExtensionService::ReloadExtension for usage example), I'll
do the same for CrxInstaller.

+
+  scoped_refptr<InstalledExtensionLoader> MakeInstalledExtensionLoader();
+
 #if defined(UNIT_TEST)
   void TrackTerminatedExtensionForTest(const Extension* extension) {
     TrackTerminatedExtension(extension);
@@ -669,9 +625,6 @@ class ExtensionService
   // Helper that updates the active extension list used for crash reporting.
   void UpdateActiveExtensionsInCrashReporter();
 
-  // Helper method. Loads extension from prefs.
-  void LoadInstalledExtension(const ExtensionInfo& info, bool write_to_prefs);
-
   // We implement some Pepper plug-ins using NaCl to take advantage of NaCl's
   // strong sandbox. Typically, these NaCl modules are stored in extensions
   // and registered here. Not all NaCl modules need to register for a MIME
@@ -687,11 +640,6 @@ class ExtensionService
 
   NaClModuleInfoList::iterator FindNaClModule(const GURL& url);
 
-  // Returns the flags that should be used with Extension::Create() for an
-  // extension that is already installed.
-  int GetExtensionCreateFlagsForInstalledExtension(
-      const ExtensionInfo* info);
-
   base::WeakPtrFactory<ExtensionService> weak_ptr_factory_;
 
   // The profile this ExtensionService is part of.
@@ -736,8 +684,8 @@ class ExtensionService
   // Whether to notify users when they attempt to install an extension.
   bool show_extensions_prompts_;
 
-  // The backend that will do IO on behalf of this instance.
-  scoped_refptr<ExtensionServiceBackend> backend_;
+  // For loading and unloading component extensions.
+  scoped_ptr<ComponentLoader> component_loader_;
 
   // Used by dispatchers to limit API quota for individual extensions.
   ExtensionsQuotaService quota_service_;
@@ -785,10 +733,6 @@ class ExtensionService
   ExtensionIconManager omnibox_icon_manager_;
   ExtensionIconManager omnibox_popup_icon_manager_;
 
-  // List of registered component extensions (see Extension::Location).
-  typedef std::vector<ComponentExtensionInfo> RegisteredComponentExtensions;
-  RegisteredComponentExtensions component_extension_manifests_;
-
   // Manages the promotion of the web store.
   AppsPromo apps_promo_;