Auto-install/uninstall shared module dependencies for extensions.

chrome/browser/extensions/extension_service.h

 // Copyright (c) 2012 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_SERVICE_H_
 #define CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_
 
 #include <map>
 #include <set>
 #include <string>
@@ skipping 48 matching lines @@
 class BrowserEventRouter;
 class ComponentLoader;
 class ContentSettingsStore;
 class CrxInstaller;
 class ExtensionActionStorageManager;
 class ExtensionSyncData;
 class ExtensionSystem;
 class ExtensionUpdater;
 class PendingExtensionManager;
 class SettingsFrontend;
-}
+}  // namespace extensions
 
 namespace syncer {
 class SyncErrorFactory;
 }
 
 // This is an interface class to encapsulate the dependencies that
 // various classes have on ExtensionService. This allows easy mocking.
 class ExtensionServiceInterface : public syncer::SyncableService {
  public:
   virtual ~ExtensionServiceInterface() {}
@@ skipping 323 matching lines @@
   // Adds |extension| to this ExtensionService and notifies observers that the
   // extensions have been loaded.
   virtual void AddExtension(const extensions::Extension* extension) OVERRIDE;
 
   // Check if we have preferences for the component extension and, if not or if
   // the stored version differs, install the extension (without requirements
   // checking) before calling AddExtension.
   virtual void AddComponentExtension(const extensions::Extension* extension)
       OVERRIDE;
 
+  // Checks an extension's shared module imports to see if they are satisfied.
+  // If they are not, this function adds the dependencies to the pending install
+  // list if |extension| came from the webstore.  If the import conditions can
+  // never be satisfied (e.g., the importer is not from the web store or the
+  // imported extension is not a shared module), |unrecoverable_error| is set
+  // true.
+  bool CheckImports(const extensions::Extension* extension,
+                    bool* unrecoverable_error);

[asargent_no_longer_on_chrome, 2013/06/13 05:47:16]

Huh, it's pretty unusual to see a function returning two bools, one as the
normal return value and one as an out param. If I'm understanding your comment
right, it seems like this one function might be serving 3 different purposes:

1) checking whether imports are already satisfied
2) checking if imports cannot ever be satisfied (e.g. it depends on another
extension, but isn't served from the webstore and that other extension isn't
already installed)
3) beginning the process of fetching dependencies

Would it make sense to break this into 3 separate functions for these 3 uses?

[elijahtaylor1, 2013/06/13 23:32:09]

Ok you caught me writing bad code for sure.  I would really say this function
does two things: your numbers 1+2 combined, and 3.  I've changed the return
value to an enum to capture state, and restructured the code in CheckImports to
more clearly separate 1+2 and 3 functionality.  I think it makes sense to keep
it all together rather than duplicate code or factor out the code to get the
pending list of extensions.  PTAL and let me know if this is ok with you.

+
+  // Returns a set of extensions that import a given extension.
+  scoped_ptr<const ExtensionSet> GetSharedModuleImporters(

[asargent_no_longer_on_chrome, 2013/06/13 05:47:16]

optional naming suggestion: GetDependents / GetDependentExtensions  might be
slightly better

[elijahtaylor1, 2013/06/13 23:32:09]

Done.

+      const extensions::Extension* extension);
+
+  // Checks an extension's shared module imports to see if they would still be
+  // referenced by other extensions when a particular extension is uninstalled.
+  // Uninstalls shared modules that are no longer referenced.

[asargent_no_longer_on_chrome, 2013/06/13 05:47:16]

nit: The first sentence in this comment is sort of confusing. Consider removing
it, and just making the second sentence something like "Uninstalls shared
modules that are only referenced by |extension|."

[elijahtaylor1, 2013/06/13 23:32:09]

Done.

+  void PruneSharedModulesOnUninstall(const extensions::Extension* extension);
+
   // Informs the service that an extension's files are in place for loading.
   //
   // Please make sure the Blacklist is checked some time before calling this
   // method.
   void OnExtensionInstalled(
       const extensions::Extension* extension,
       const syncer::StringOrdinal& page_ordinal,
       bool has_requirement_errors,
       bool wait_for_idle);
 
+  // Checks for delayed installation for all pending installs.
+  void MaybeFinishDelayedInstallations();
+
   // Similar to FinishInstallation, but first checks if there still is an update
   // pending for the extension, and makes sure the extension is still idle.
   void MaybeFinishDelayedInstallation(const std::string& extension_id);
 
   // Finishes installation of an update for an extension with the specified id,
   // when installation of that extension was previously delayed because the
   // extension was in use.
   virtual void FinishDelayedInstallation(
      const std::string& extension_id) OVERRIDE;
 
@@ skipping 361 matching lines @@
 
   // The list of installed extensions that have been terminated.
   ExtensionSet terminated_extensions_;
 
   // The list of installed extensions that have been blacklisted. Generally
   // these shouldn't be considered as installed by the extension platform: we
   // only keep them around so that if extensions are blacklisted by mistake
   // they can easily be un-blacklisted.
   ExtensionSet blacklisted_extensions_;
 
-  // The list of extension updates that have had their installs delayed because
-  // they are waiting for idle.
-  ExtensionSet delayed_updates_for_idle_;
-
-  // The list of extension installs delayed by |installs_delayed_|.
-  // This is a disjoint set from |delayed_updates_for_idle_|. Extensions in
-  // the |delayed_installs_| do not need to wait for idle.
+  // The list of extension installs delayed for various reasons.  The reason
+  // for delayed install is stored in ExtensionPrefs.
   ExtensionSet delayed_installs_;
 
   // Hold the set of pending extensions.
   extensions::PendingExtensionManager pending_extension_manager_;
 
   // The map of extension IDs to their runtime data.
   ExtensionRuntimeDataMap extension_runtime_data_;
 
   // The full path to the directory where extensions are installed.
   base::FilePath install_directory_;
@@ skipping 95 matching lines @@
   ObserverList<extensions::UpdateObserver, true> update_observers_;
 
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            InstallAppsWithUnlimtedStorage);
   FRIEND_TEST_ALL_PREFIXES(ExtensionServiceTest,
                            InstallAppsAndCheckStorageProtection);
   DISALLOW_COPY_AND_ASSIGN(ExtensionService);
 };
 
 #endif  // CHROME_BROWSER_EXTENSIONS_EXTENSION_SERVICE_H_