Bug 1363925: Part 8d - Move updateAddonDisabledState to XPIDatabase. r=aswan
authorKris Maglione <maglione.k@gmail.com>
Sun, 22 Apr 2018 15:00:08 -0700
changeset 471299 c948486ee8e1faed6cd9f6200771ae40a9232e6a
parent 471298 1646b51d323f8d98211fb48436d6204b22c6044e
child 471300 3ad2e050c66a73c748761d451d2c7a5339501773
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1363925
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1363925: Part 8d - Move updateAddonDisabledState to XPIDatabase. r=aswan This code is large and complex, and can only be called when we have an AddonInternal object from XPIDatabase.jsm. It should live with that code. MozReview-Commit-ID: 3ssV5aH9NUJ
toolkit/mozapps/extensions/internal/XPIDatabase.jsm
toolkit/mozapps/extensions/internal/XPIInstall.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
--- a/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIDatabase.jsm
@@ -1,14 +1,22 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
+/**
+ * This file contains most of the logic required to maintain the
+ * extensions database, including querying and modifying extension
+ * metadata. In general, we try to avoid loading it during startup when
+ * at all possible. Please keep that in mind when deciding whether to
+ * add code here or elsewhere.
+ */
+
 /* eslint "valid-jsdoc": [2, {requireReturn: false, requireReturnDescription: false, prefer: {return: "returns"}}] */
 
 var EXPORTED_SYMBOLS = ["AddonInternal", "XPIDatabase", "XPIDatabaseReconcile"];
 
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
 XPCOMUtils.defineLazyModuleGetters(this, {
   AddonManager: "resource://gre/modules/AddonManager.jsm",
@@ -467,17 +475,17 @@ AddonInternal.prototype = {
           softDisabled = !this.userDisabled;
         }
       } else {
         softDisabled = false;
       }
     }
 
     if (this.inDatabase && updateDatabase) {
-      XPIProvider.updateAddonDisabledState(this, userDisabled, softDisabled);
+      XPIDatabase.updateAddonDisabledState(this, userDisabled, softDisabled);
       XPIDatabase.saveChanges();
     } else {
       this.appDisabled = !XPIDatabase.isUsableAddon(this);
       if (userDisabled !== undefined) {
         this.userDisabled = userDisabled;
       }
       if (softDisabled !== undefined) {
         this.softDisabled = softDisabled;
@@ -883,17 +891,17 @@ AddonWrapper.prototype = {
     }
 
     if (addon.inDatabase) {
       // hidden and system add-ons should not be user disabled,
       // as there is no UI to re-enable them.
       if (this.hidden) {
         throw new Error(`Cannot disable hidden add-on ${addon.id}`);
       }
-      XPIProvider.updateAddonDisabledState(addon, val);
+      XPIDatabase.updateAddonDisabledState(addon, val);
     } else {
       addon.userDisabled = val;
       // When enabling remove the softDisabled flag
       if (!val)
         addon.softDisabled = false;
     }
 
     return val;
@@ -903,19 +911,19 @@ AddonWrapper.prototype = {
     let addon = addonFor(this);
     if (val == addon.softDisabled)
       return val;
 
     if (addon.inDatabase) {
       // When softDisabling a theme just enable the active theme
       if (isTheme(addon.type) && val && !addon.userDisabled) {
         if (isWebExtension(addon.type))
-          XPIProvider.updateAddonDisabledState(addon, undefined, val);
+          XPIDatabase.updateAddonDisabledState(addon, undefined, val);
       } else {
-        XPIProvider.updateAddonDisabledState(addon, undefined, val);
+        XPIDatabase.updateAddonDisabledState(addon, undefined, val);
       }
     } else if (!addon.userDisabled) {
       // Only set softDisabled if not already disabled
       addon.softDisabled = val;
     }
 
     return val;
   },
@@ -1023,19 +1031,19 @@ AddonWrapper.prototype = {
   reload() {
     return new Promise((resolve) => {
       const addon = addonFor(this);
 
       logger.debug(`reloading add-on ${addon.id}`);
 
       if (!this.temporarilyInstalled) {
         let addonFile = addon.getResourceURI;
-        XPIProvider.updateAddonDisabledState(addon, true);
+        XPIDatabase.updateAddonDisabledState(addon, true);
         Services.obs.notifyObservers(addonFile, "flush-cache-entry");
-        XPIProvider.updateAddonDisabledState(addon, false);
+        XPIDatabase.updateAddonDisabledState(addon, false);
         resolve();
       } else {
         // This function supports re-installing an existing add-on.
         resolve(AddonManager.installTemporaryAddon(addon._sourceBundle));
       }
     });
   },
 
@@ -1248,17 +1256,17 @@ Object.assign(DBAddonInternal.prototype,
           aTargetApp.minVersion = aUpdateTarget.minVersion;
           aTargetApp.maxVersion = aUpdateTarget.maxVersion;
           XPIDatabase.saveChanges();
         }
       });
     });
 
     if (wasCompatible != this.isCompatible)
-      XPIProvider.updateAddonDisabledState(this);
+      XPIDatabase.updateAddonDisabledState(this);
   },
 
   toJSON() {
     return copyProperties(this, PROP_JSON_FIELDS);
   },
 
   get inDatabase() {
     return true;
@@ -2207,16 +2215,151 @@ this.XPIDatabase = {
       if (newActive != addon.active) {
         addon.active = newActive;
         this.saveChanges();
       }
     }
   },
 
   /**
+   * Updates the disabled state for an add-on. Its appDisabled property will be
+   * calculated and if the add-on is changed the database will be saved and
+   * appropriate notifications will be sent out to the registered AddonListeners.
+   *
+   * @param {DBAddonInternal} aAddon
+   *        The DBAddonInternal to update
+   * @param {boolean?} [aUserDisabled]
+   *        Value for the userDisabled property. If undefined the value will
+   *        not change
+   * @param {boolean?} [aSoftDisabled]
+   *        Value for the softDisabled property. If undefined the value will
+   *        not change. If true this will force userDisabled to be true
+   * @param {boolean?} [aBecauseSelecting]
+   *        True if we're disabling this add-on because we're selecting
+   *        another.
+   * @returns {boolean?}
+   *       A tri-state indicating the action taken for the add-on:
+   *           - undefined: The add-on did not change state
+   *           - true: The add-on because disabled
+   *           - false: The add-on became enabled
+   * @throws if addon is not a DBAddonInternal
+   */
+  updateAddonDisabledState(aAddon, aUserDisabled, aSoftDisabled, aBecauseSelecting) {
+    if (!(aAddon.inDatabase))
+      throw new Error("Can only update addon states for installed addons.");
+    if (aUserDisabled !== undefined && aSoftDisabled !== undefined) {
+      throw new Error("Cannot change userDisabled and softDisabled at the " +
+                      "same time");
+    }
+
+    if (aUserDisabled === undefined) {
+      aUserDisabled = aAddon.userDisabled;
+    } else if (!aUserDisabled) {
+      // If enabling the add-on then remove softDisabled
+      aSoftDisabled = false;
+    }
+
+    // If not changing softDisabled or the add-on is already userDisabled then
+    // use the existing value for softDisabled
+    if (aSoftDisabled === undefined || aUserDisabled)
+      aSoftDisabled = aAddon.softDisabled;
+
+    let appDisabled = !this.isUsableAddon(aAddon);
+    // No change means nothing to do here
+    if (aAddon.userDisabled == aUserDisabled &&
+        aAddon.appDisabled == appDisabled &&
+        aAddon.softDisabled == aSoftDisabled)
+      return undefined;
+
+    let wasDisabled = aAddon.disabled;
+    let isDisabled = aUserDisabled || aSoftDisabled || appDisabled;
+
+    // If appDisabled changes but addon.disabled doesn't,
+    // no onDisabling/onEnabling is sent - so send a onPropertyChanged.
+    let appDisabledChanged = aAddon.appDisabled != appDisabled;
+
+    // Update the properties in the database.
+    this.setAddonProperties(aAddon, {
+      userDisabled: aUserDisabled,
+      appDisabled,
+      softDisabled: aSoftDisabled
+    });
+
+    let wrapper = aAddon.wrapper;
+
+    if (appDisabledChanged) {
+      AddonManagerPrivate.callAddonListeners("onPropertyChanged",
+                                             wrapper,
+                                             ["appDisabled"]);
+    }
+
+    // If the add-on is not visible or the add-on is not changing state then
+    // there is no need to do anything else
+    if (!aAddon.visible || (wasDisabled == isDisabled))
+      return undefined;
+
+    // Flag that active states in the database need to be updated on shutdown
+    Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
+
+    // Sync with XPIStates.
+    let xpiState = XPIStates.getAddon(aAddon.location, aAddon.id);
+    if (xpiState) {
+      xpiState.syncWithDB(aAddon);
+      XPIStates.save();
+    } else {
+      // There should always be an xpiState
+      logger.warn("No XPIState for ${id} in ${location}", aAddon);
+    }
+
+    // Have we just gone back to the current state?
+    if (isDisabled != aAddon.active) {
+      AddonManagerPrivate.callAddonListeners("onOperationCancelled", wrapper);
+    } else {
+      if (isDisabled) {
+        AddonManagerPrivate.callAddonListeners("onDisabling", wrapper, false);
+      } else {
+        AddonManagerPrivate.callAddonListeners("onEnabling", wrapper, false);
+      }
+
+      this.updateAddonActive(aAddon, !isDisabled);
+
+      if (isDisabled) {
+        if (aAddon.bootstrap && XPIProvider.activeAddons.has(aAddon.id)) {
+          XPIProvider.callBootstrapMethod(aAddon, aAddon._sourceBundle, "shutdown",
+                                          BOOTSTRAP_REASONS.ADDON_DISABLE);
+          XPIProvider.unloadBootstrapScope(aAddon.id);
+        }
+        AddonManagerPrivate.callAddonListeners("onDisabled", wrapper);
+      } else {
+        if (aAddon.bootstrap) {
+          XPIProvider.callBootstrapMethod(aAddon, aAddon._sourceBundle, "startup",
+                                          BOOTSTRAP_REASONS.ADDON_ENABLE);
+        }
+        AddonManagerPrivate.callAddonListeners("onEnabled", wrapper);
+      }
+    }
+
+    // Notify any other providers that a new theme has been enabled
+    if (isTheme(aAddon.type)) {
+      if (!isDisabled) {
+        AddonManagerPrivate.notifyAddonChanged(aAddon.id, aAddon.type);
+
+        if (xpiState) {
+          xpiState.syncWithDB(aAddon);
+          XPIStates.save();
+        }
+      } else if (isDisabled && !aBecauseSelecting) {
+        AddonManagerPrivate.notifyAddonChanged(null, "theme");
+      }
+    }
+
+    return isDisabled;
+  },
+
+  /**
    * Record a bit of per-addon telemetry.
    *
    * Yes, this description is extremely helpful. How dare you question its
    * utility?
    *
    * @param {AddonInternal} aAddon
    *        The addon to record
    */
--- a/toolkit/mozapps/extensions/internal/XPIInstall.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIInstall.jsm
@@ -1,14 +1,21 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
+/**
+ * This file contains most of the logic required to install extensions.
+ * In general, we try to avoid loading it until extension installation
+ * or update is required. Please keep that in mind when deciding whether
+ * to add code here or elsewhere.
+ */
+
 /* eslint "valid-jsdoc": [2, {requireReturn: false, requireReturnDescription: false, prefer: {return: "returns"}}] */
 
 var EXPORTED_SYMBOLS = [
   "UpdateChecker",
   "XPIInstall",
   "verifyBundleSignedState",
 ];
 
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -1,14 +1,21 @@
  /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
+/**
+ * This file contains most of the logic required to load and run
+ * extensions at startup. Anything which is not required immediately at
+ * startup should go in XPIInstall.jsm or XPIDatabase.jsm if at all
+ * possible, in order to minimize the impact on startup performance.
+ */
+
 /* eslint "valid-jsdoc": [2, {requireReturn: false, requireReturnDescription: false, prefer: {return: "returns"}}] */
 
 var EXPORTED_SYMBOLS = ["XPIProvider", "XPIInternal"];
 
 /* globals WebExtensionPolicy */
 
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
@@ -1972,17 +1979,17 @@ var XPIProvider = {
 
         if (signedState != addon.signedState) {
           addon.signedState = signedState;
           AddonManagerPrivate.callAddonListeners("onPropertyChanged",
                                                  addon.wrapper,
                                                  ["signedState"]);
         }
 
-        let disabled = XPIProvider.updateAddonDisabledState(addon);
+        let disabled = XPIDatabase.updateAddonDisabledState(addon);
         if (disabled !== undefined)
           changes[disabled ? "disabled" : "enabled"].push(addon.id);
       }
 
       XPIDatabase.saveChanges();
 
       Services.obs.notifyObservers(null, "xpi-signature-changed", JSON.stringify(changes));
     } catch (err) {
@@ -2562,17 +2569,17 @@ var XPIProvider = {
   addonChanged(aId, aType) {
     // We only care about themes in this provider
     if (!isTheme(aType))
       return;
 
     let addons = XPIDatabase.getAddonsByType("webextension-theme");
     for (let theme of addons) {
       if (theme.visible && theme.id != aId)
-        this.updateAddonDisabledState(theme, true, undefined, true);
+        XPIDatabase.updateAddonDisabledState(theme, true, undefined, true);
     }
 
     if (!aId && (!LightweightThemeManager.currentTheme ||
                  LightweightThemeManager.currentTheme !== DEFAULT_THEME_ID)) {
       let theme = LightweightThemeManager.getUsedTheme(DEFAULT_THEME_ID);
       // This can only ever be null in tests.
       // This can all go away once lightweight themes are gone.
       if (theme) {
@@ -2582,33 +2589,33 @@ var XPIProvider = {
   },
 
   /**
    * Update the appDisabled property for all add-ons.
    */
   updateAddonAppDisabledStates() {
     let addons = XPIDatabase.getAddons();
     for (let addon of addons) {
-      this.updateAddonDisabledState(addon);
+      XPIDatabase.updateAddonDisabledState(addon);
     }
   },
 
   /**
    * Update the repositoryAddon property for all add-ons.
    */
   async updateAddonRepositoryData() {
     let addons = await XPIDatabase.getVisibleAddons(null);
     logger.debug("updateAddonRepositoryData found " + addons.length + " visible add-ons");
 
     await Promise.all(addons.map(addon =>
       AddonRepository.getCachedAddonByID(addon.id).then(aRepoAddon => {
         if (aRepoAddon || AddonRepository.getCompatibilityOverridesSync(addon.id)) {
           logger.debug("updateAddonRepositoryData got info for " + addon.id);
           addon._repositoryAddon = aRepoAddon;
-          this.updateAddonDisabledState(addon);
+          XPIDatabase.updateAddonDisabledState(addon);
         }
       })));
   },
 
   onDebugConnectionChange({what, connection}) {
     if (what != "opened")
       return;
 
@@ -2823,17 +2830,17 @@ var XPIProvider = {
       } else if (aMethod == "shutdown") {
         activeAddon.started = false;
 
         // Extensions are automatically deinitialized in the correct order at shutdown.
         if (aReason != BOOTSTRAP_REASONS.APP_SHUTDOWN) {
           activeAddon.disable = true;
           for (let addon of this.getDependentAddons(aAddon)) {
             if (addon.active)
-              this.updateAddonDisabledState(addon);
+              XPIDatabase.updateAddonDisabledState(addon);
           }
         }
       }
 
       let installLocation = aAddon._installLocation || null;
       let params = {
         id: aAddon.id,
         version: aAddon.version,
@@ -2885,161 +2892,26 @@ var XPIProvider = {
           activeAddon.startupPromise = Promise.resolve(result);
           activeAddon.startupPromise.catch(Cu.reportError);
         }
       }
     } finally {
       // Extensions are automatically initialized in the correct order at startup.
       if (aMethod == "startup" && aReason != BOOTSTRAP_REASONS.APP_STARTUP) {
         for (let addon of this.getDependentAddons(aAddon))
-          this.updateAddonDisabledState(addon);
+          XPIDatabase.updateAddonDisabledState(addon);
       }
 
       if (CHROME_TYPES.has(aAddon.type) && aMethod == "shutdown" && aReason != BOOTSTRAP_REASONS.APP_SHUTDOWN) {
         logger.debug("Removing manifest for " + aFile.path);
         Components.manager.removeBootstrappedManifestLocation(aFile);
       }
       this.setTelemetry(aAddon.id, aMethod + "_MS", new Date() - timeStart);
     }
   },
-
-  /**
-   * Updates the disabled state for an add-on. Its appDisabled property will be
-   * calculated and if the add-on is changed the database will be saved and
-   * appropriate notifications will be sent out to the registered AddonListeners.
-   *
-   * @param {DBAddonInternal} aAddon
-   *        The DBAddonInternal to update
-   * @param {boolean?} [aUserDisabled]
-   *        Value for the userDisabled property. If undefined the value will
-   *        not change
-   * @param {boolean?} [aSoftDisabled]
-   *        Value for the softDisabled property. If undefined the value will
-   *        not change. If true this will force userDisabled to be true
-   * @param {boolean?} [aBecauseSelecting]
-   *        True if we're disabling this add-on because we're selecting
-   *        another.
-   * @returns {boolean?}
-   *       A tri-state indicating the action taken for the add-on:
-   *           - undefined: The add-on did not change state
-   *           - true: The add-on because disabled
-   *           - false: The add-on became enabled
-   * @throws if addon is not a DBAddonInternal
-   */
-  updateAddonDisabledState(aAddon, aUserDisabled, aSoftDisabled, aBecauseSelecting) {
-    if (!(aAddon.inDatabase))
-      throw new Error("Can only update addon states for installed addons.");
-    if (aUserDisabled !== undefined && aSoftDisabled !== undefined) {
-      throw new Error("Cannot change userDisabled and softDisabled at the " +
-                      "same time");
-    }
-
-    if (aUserDisabled === undefined) {
-      aUserDisabled = aAddon.userDisabled;
-    } else if (!aUserDisabled) {
-      // If enabling the add-on then remove softDisabled
-      aSoftDisabled = false;
-    }
-
-    // If not changing softDisabled or the add-on is already userDisabled then
-    // use the existing value for softDisabled
-    if (aSoftDisabled === undefined || aUserDisabled)
-      aSoftDisabled = aAddon.softDisabled;
-
-    let appDisabled = !XPIDatabase.isUsableAddon(aAddon);
-    // No change means nothing to do here
-    if (aAddon.userDisabled == aUserDisabled &&
-        aAddon.appDisabled == appDisabled &&
-        aAddon.softDisabled == aSoftDisabled)
-      return undefined;
-
-    let wasDisabled = aAddon.disabled;
-    let isDisabled = aUserDisabled || aSoftDisabled || appDisabled;
-
-    // If appDisabled changes but addon.disabled doesn't,
-    // no onDisabling/onEnabling is sent - so send a onPropertyChanged.
-    let appDisabledChanged = aAddon.appDisabled != appDisabled;
-
-    // Update the properties in the database.
-    XPIDatabase.setAddonProperties(aAddon, {
-      userDisabled: aUserDisabled,
-      appDisabled,
-      softDisabled: aSoftDisabled
-    });
-
-    let wrapper = aAddon.wrapper;
-
-    if (appDisabledChanged) {
-      AddonManagerPrivate.callAddonListeners("onPropertyChanged",
-                                             wrapper,
-                                             ["appDisabled"]);
-    }
-
-    // If the add-on is not visible or the add-on is not changing state then
-    // there is no need to do anything else
-    if (!aAddon.visible || (wasDisabled == isDisabled))
-      return undefined;
-
-    // Flag that active states in the database need to be updated on shutdown
-    Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
-
-    // Sync with XPIStates.
-    let xpiState = XPIStates.getAddon(aAddon.location, aAddon.id);
-    if (xpiState) {
-      xpiState.syncWithDB(aAddon);
-      XPIStates.save();
-    } else {
-      // There should always be an xpiState
-      logger.warn("No XPIState for ${id} in ${location}", aAddon);
-    }
-
-    // Have we just gone back to the current state?
-    if (isDisabled != aAddon.active) {
-      AddonManagerPrivate.callAddonListeners("onOperationCancelled", wrapper);
-    } else {
-      if (isDisabled) {
-        AddonManagerPrivate.callAddonListeners("onDisabling", wrapper, false);
-      } else {
-        AddonManagerPrivate.callAddonListeners("onEnabling", wrapper, false);
-      }
-
-      XPIDatabase.updateAddonActive(aAddon, !isDisabled);
-
-      if (isDisabled) {
-        if (aAddon.bootstrap && this.activeAddons.has(aAddon.id)) {
-          this.callBootstrapMethod(aAddon, aAddon._sourceBundle, "shutdown",
-                                   BOOTSTRAP_REASONS.ADDON_DISABLE);
-          this.unloadBootstrapScope(aAddon.id);
-        }
-        AddonManagerPrivate.callAddonListeners("onDisabled", wrapper);
-      } else {
-        if (aAddon.bootstrap) {
-          this.callBootstrapMethod(aAddon, aAddon._sourceBundle, "startup",
-                                   BOOTSTRAP_REASONS.ADDON_ENABLE);
-        }
-        AddonManagerPrivate.callAddonListeners("onEnabled", wrapper);
-      }
-    }
-
-    // Notify any other providers that a new theme has been enabled
-    if (isTheme(aAddon.type)) {
-      if (!isDisabled) {
-        AddonManagerPrivate.notifyAddonChanged(aAddon.id, aAddon.type);
-
-        if (xpiState) {
-          xpiState.syncWithDB(aAddon);
-          XPIStates.save();
-        }
-      } else if (isDisabled && !aBecauseSelecting) {
-        AddonManagerPrivate.notifyAddonChanged(null, "theme");
-      }
-    }
-
-    return isDisabled;
-  },
 };
 
 for (let meth of ["cancelUninstallAddon", "getInstallForFile",
                   "getInstallForURL", "installAddonFromLocation",
                   "installAddonFromSources", "installTemporaryAddon",
                   "isInstallAllowed", "uninstallAddon", "updateSystemAddons"]) {
   XPIProvider[meth] = function() {
     return XPIInstall[meth](...arguments);