Bug 1330349 - Part 3 - make sure only one theme may be selected at the same time. r?Mossop draft
authorMike de Boer <mdeboer@mozilla.com>
Thu, 02 Mar 2017 14:22:20 +0100
changeset 491925 9d7554d6ea4faadfb42ae2dc140613d253a9bbc6
parent 491924 81d730d4ede3c934b256ab3c8d1f353c6001d3d6
child 491926 87370044cee487a820e097fc9fc82b2b97522cb0
push id47458
push usermdeboer@mozilla.com
push dateThu, 02 Mar 2017 13:47:01 +0000
reviewersMossop
bugs1330349
milestone54.0a1
Bug 1330349 - Part 3 - make sure only one theme may be selected at the same time. r?Mossop MozReview-Commit-ID: JeanO5g4KtA
toolkit/mozapps/extensions/LightweightThemeManager.jsm
toolkit/mozapps/extensions/internal/XPIProvider.jsm
toolkit/mozapps/extensions/internal/XPIProviderUtils.js
--- a/toolkit/mozapps/extensions/LightweightThemeManager.jsm
+++ b/toolkit/mozapps/extensions/LightweightThemeManager.jsm
@@ -14,16 +14,17 @@ Components.utils.import("resource://gre/
 /* globals AddonManagerPrivate*/
 Components.utils.import("resource://gre/modules/Services.jsm");
 
 const ID_SUFFIX              = "@personas.mozilla.org";
 const PREF_LWTHEME_TO_SELECT = "extensions.lwThemeToSelect";
 const PREF_GENERAL_SKINS_SELECTEDSKIN = "general.skins.selectedSkin";
 const PREF_EM_DSS_ENABLED    = "extensions.dss.enabled";
 const ADDON_TYPE             = "theme";
+const ADDON_TYPE_WEBEXT      = "webextension-theme";
 
 const URI_EXTENSION_STRINGS  = "chrome://mozapps/locale/extensions/extensions.properties";
 
 const STRING_TYPE_NAME       = "type.%ID%.name";
 
 const DEFAULT_MAX_USED_THEMES_COUNT = 30;
 
 const MAX_PREVIEW_SECONDS = 30;
@@ -349,17 +350,17 @@ this.LightweightThemeManager = {
    *         The ID of the newly enabled add-on
    * @param  aType
    *         The type of the newly enabled add-on
    * @param  aPendingRestart
    *         true if the newly enabled add-on will only become enabled after a
    *         restart
    */
   addonChanged(aId, aType, aPendingRestart) {
-    if (aType != ADDON_TYPE)
+    if (aType != ADDON_TYPE && aType != ADDON_TYPE_WEBEXT)
       return;
 
     let id = _getInternalID(aId);
     let current = this.currentTheme;
 
     try {
       let next = Services.prefs.getCharPref(PREF_LWTHEME_TO_SELECT);
       if (id == next && aPendingRestart)
--- a/toolkit/mozapps/extensions/internal/XPIProvider.jsm
+++ b/toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ -409,16 +409,30 @@ function addonMap(addons) {
  *
  * @param  {String} type
  * @return {Boolean}
  */
 function isWebExtension(type) {
   return type == "webextension" || type == "webextension-theme";
 }
 
+var gThemeAliases = null;
+/**
+ * Helper function that determines whether an addon of a certain type is a
+ * theme.
+ *
+ * @param  {String} type
+ * @return {Boolean}
+ */
+function isTheme(type) {
+  if (!gThemeAliases)
+    gThemeAliases = getAllAliasesForTypes(["theme"]);
+  return gThemeAliases.includes(type);
+}
+
 /**
  * Sets permissions on a file
  *
  * @param  aFile
  *         The file or directory to operate on.
  * @param  aPermissions
  *         The permisions to set
  */
@@ -1043,17 +1057,18 @@ var loadManifestFromWebManifest = Task.a
 
   addon.targetApplications = [{
     id: TOOLKIT_ID,
     minVersion: bss.strict_min_version,
     maxVersion: bss.strict_max_version,
   }];
 
   addon.targetPlatforms = [];
-  addon.userDisabled = false;
+  // Themes are disabled by default, except when they're installed from a web page.
+  addon.userDisabled = theme;
   addon.softDisabled = addon.blocklistState == Blocklist.STATE_SOFTBLOCKED;
 
   return addon;
 });
 
 /**
  * Reads an AddonInternal object from an RDF stream.
  *
@@ -1305,17 +1320,17 @@ let loadManifestFromRDF = Task.async(fun
     }
 
     addon.targetPlatforms.push(platform);
   }
 
   // A theme's userDisabled value is true if the theme is not the selected skin
   // or if there is an active lightweight theme. We ignore whether softblocking
   // is in effect since it would change the active theme.
-  if (addon.type == "theme") {
+  if (isTheme(addon.type)) {
     addon.userDisabled = !!LightweightThemeManager.currentTheme ||
                          addon.internalName != XPIProvider.selectedSkin;
   } else if (addon.type == "experiment") {
     // Experiments are disabled by default. It is up to the Experiments Manager
     // to enable them (it drives installation).
     addon.userDisabled = true;
   } else {
     addon.userDisabled = false;
@@ -4105,16 +4120,20 @@ this.XPIProvider = {
     XPIProvider.callBootstrapMethod(addon, file, "startup",
                                     BOOTSTRAP_REASONS.ADDON_INSTALL);
     AddonManagerPrivate.callInstallListeners("onExternalInstall",
                                              null, addon.wrapper,
                                              oldAddon ? oldAddon.wrapper : null,
                                              false);
     AddonManagerPrivate.callAddonListeners("onInstalled", addon.wrapper);
 
+    // Notify providers that a new theme has been enabled.
+    if (isTheme(addon.type))
+      AddonManagerPrivate.notifyAddonChanged(addon.id, addon.type, false);
+
     return addon.wrapper;
   }),
 
   /**
    * Returns an Addon corresponding to an instance ID.
    * @param aInstanceID
    *        An Addon Instance ID
    * @return {Promise}
@@ -4268,37 +4287,42 @@ this.XPIProvider = {
    * @param  aType
    *         The type of the newly enabled add-on
    * @param  aPendingRestart
    *         true if the newly enabled add-on will only become enabled after a
    *         restart
    */
   addonChanged(aId, aType, aPendingRestart) {
     // We only care about themes in this provider
-    if (aType != "theme")
+    if (!isTheme(aType))
       return;
 
     if (!aId) {
       // Fallback to the default theme when no theme was enabled
       this.enableDefaultTheme();
       return;
     }
 
     // Look for the previously enabled theme and find the internalName of the
     // currently selected theme
     let previousTheme = null;
     let newSkin = this.defaultSkin;
-    let addons = XPIDatabase.getAddonsByType("theme");
+    let addons = XPIDatabase.getAddonsByType("theme", "webextension-theme");
     for (let theme of addons) {
       if (!theme.visible)
         return;
-      if (theme.id == aId)
+      let isChangedAddon = (theme.id == aId);
+      if (isWebExtension(theme.type)) {
+        if (!isChangedAddon)
+          this.updateAddonDisabledState(theme, true, undefined, aPendingRestart);
+      } else if (isChangedAddon) {
         newSkin = theme.internalName;
-      else if (theme.userDisabled == false && !theme.pendingUninstall)
+      } else if (theme.userDisabled == false && !theme.pendingUninstall) {
         previousTheme = theme;
+      }
     }
 
     if (aPendingRestart) {
       Services.prefs.setBoolPref(PREF_DSS_SWITCHPENDING, true);
       Services.prefs.setCharPref(PREF_DSS_SKIN_TO_SELECT, newSkin);
     } else if (newSkin == this.currentSkin) {
       try {
         Services.prefs.clearUserPref(PREF_DSS_SWITCHPENDING);
@@ -4314,17 +4338,17 @@ this.XPIProvider = {
 
     // Flush the preferences to disk so they don't get out of sync with the
     // database
     Services.prefs.savePrefFile(null);
 
     // Mark the previous theme as disabled. This won't cause recursion since
     // only enabled calls notifyAddonChanged.
     if (previousTheme)
-      this.updateAddonDisabledState(previousTheme, true);
+      this.updateAddonDisabledState(previousTheme, true, undefined, aPendingRestart);
   },
 
   /**
    * Update the appDisabled property for all add-ons.
    */
   updateAddonAppDisabledStates() {
     let addons = XPIDatabase.getAddons();
     for (let addon of addons) {
@@ -4524,17 +4548,23 @@ this.XPIProvider = {
     // restarting
     if (Services.appinfo.inSafeMode)
       return false;
 
     // Anything that is active is already enabled
     if (aAddon.active)
       return false;
 
-    if (aAddon.type == "theme") {
+    if (isTheme(aAddon.type)) {
+      if (isWebExtension(aAddon.type)) {
+        // Enabling a WebExtension type theme requires a restart ONLY when the
+        // theme-to-be-disabled requires a restart.
+        let theme = XPIDatabase.getVisibleAddonForInternalName(this.currentSkin);
+        return !theme || this.disableRequiresRestart(theme);
+      }
       // If dynamic theme switching is enabled then switching themes does not
       // require a restart
       if (Preferences.get(PREF_EM_DSS_ENABLED))
         return false;
 
       // If the theme is already the theme in use then no restart is necessary.
       // This covers the case where the default theme is in use but a
       // lightweight theme is considered active.
@@ -4953,23 +4983,26 @@ this.XPIProvider = {
    * @param  aAddon
    *         The DBAddonInternal to update
    * @param  aUserDisabled
    *         Value for the userDisabled property. If undefined the value will
    *         not change
    * @param  aSoftDisabled
    *         Value for the softDisabled property. If undefined the value will
    *         not change. If true this will force userDisabled to be true
+   * @param  aPendingRestart
+   *         If the addon is updated whilst the disabled state of another non-
+   *         restartless addon is also set, we need to carry that forward.
    * @return 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) {
+  updateAddonDisabledState(aAddon, aUserDisabled, aSoftDisabled, aPendingRestart = false) {
     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) {
@@ -5021,17 +5054,17 @@ this.XPIProvider = {
     // Flag that active states in the database need to be updated on shutdown
     Services.prefs.setBoolPref(PREF_PENDING_OPERATIONS, true);
 
     // Have we just gone back to the current state?
     if (isDisabled != aAddon.active) {
       AddonManagerPrivate.callAddonListeners("onOperationCancelled", wrapper);
     } else {
       if (isDisabled) {
-        var needsRestart = this.disableRequiresRestart(aAddon);
+        var needsRestart = aPendingRestart || this.disableRequiresRestart(aAddon);
         AddonManagerPrivate.callAddonListeners("onDisabling", wrapper,
                                                needsRestart);
       } else {
         needsRestart = this.enableRequiresRestart(aAddon);
         AddonManagerPrivate.callAddonListeners("onEnabling", wrapper,
                                                needsRestart);
       }
 
@@ -5078,17 +5111,17 @@ this.XPIProvider = {
       xpiState.syncWithDB(aAddon);
       XPIStates.save();
     } else {
       // There should always be an xpiState
       logger.warn("No XPIState for ${id} in ${location}", aAddon);
     }
 
     // Notify any other providers that a new theme has been enabled
-    if (aAddon.type == "theme" && !isDisabled)
+    if (isTheme(aAddon.type) && !isDisabled)
       AddonManagerPrivate.notifyAddonChanged(aAddon.id, aAddon.type, needsRestart);
 
     return isDisabled;
   },
 
   /**
    * Uninstalls an add-on, immediately if possible or marks it as pending
    * uninstall if not.
@@ -5226,17 +5259,17 @@ this.XPIProvider = {
     } else if (aAddon.bootstrap && aAddon.active && !this.disableRequiresRestart(aAddon)) {
       this.callBootstrapMethod(aAddon, aAddon._sourceBundle, "shutdown",
                                BOOTSTRAP_REASONS.ADDON_UNINSTALL);
       this.unloadBootstrapScope(aAddon.id);
       XPIDatabase.updateAddonActive(aAddon, false);
     }
 
     // Notify any other providers that a new theme has been enabled
-    if (aAddon.type == "theme" && aAddon.active)
+    if (isTheme(aAddon.type) && aAddon.active)
       AddonManagerPrivate.notifyAddonChanged(null, aAddon.type, requiresRestart);
   },
 
   /**
    * Cancels the pending uninstall of an add-on.
    *
    * @param  aAddon
    *         The DBAddonInternal to cancel uninstall for
@@ -5265,17 +5298,17 @@ this.XPIProvider = {
 
     if (aAddon.bootstrap && !aAddon.disabled && !this.enableRequiresRestart(aAddon)) {
       this.callBootstrapMethod(aAddon, aAddon._sourceBundle, "startup",
                                BOOTSTRAP_REASONS.ADDON_INSTALL);
       XPIDatabase.updateAddonActive(aAddon, true);
     }
 
     // Notify any other providers that this theme is now enabled again.
-    if (aAddon.type == "theme" && aAddon.active)
+    if (isTheme(aAddon.type) && aAddon.active)
       AddonManagerPrivate.notifyAddonChanged(aAddon.id, aAddon.type, false);
   }
 };
 
 function getHashStringForCrypto(aCrypto) {
   // return the two-digit hexadecimal code for a byte
   let toHexString = charCode => ("0" + charCode.toString(16)).slice(-2);
 
@@ -5831,16 +5864,20 @@ class AddonInstall {
           } else {
             // XXX this makes it dangerous to do some things in onInstallEnded
             // listeners because important cleanup hasn't been done yet
             XPIProvider.unloadBootstrapScope(this.addon.id);
           }
         }
         XPIProvider.setTelemetry(this.addon.id, "unpacked", installedUnpacked);
         recordAddonTelemetry(this.addon);
+
+        // Notify providers that a new theme has been enabled.
+        if (isTheme(this.addon.type) && this.addon.active)
+          AddonManagerPrivate.notifyAddonChanged(this.addon.id, this.addon.type, requiresRestart);
       }
     }).bind(this)).then(null, (e) => {
       logger.warn(`Failed to install ${this.file.path} from ${this.sourceURI.spec} to ${stagedAddon.path}`, e);
 
       if (stagedAddon.exists())
         recursiveRemove(stagedAddon);
       this.state = AddonManager.STATE_INSTALL_FAILED;
       this.error = AddonManager.ERROR_FILE_ACCESS;
@@ -7346,17 +7383,17 @@ AddonWrapper.prototype = {
     let addon = addonFor(this);
     let repositoryAddon = addon._repositoryAddon;
     if (repositoryAddon && ("screenshots" in repositoryAddon)) {
       let repositoryScreenshots = repositoryAddon.screenshots;
       if (repositoryScreenshots && repositoryScreenshots.length > 0)
         return repositoryScreenshots;
     }
 
-    if (addon.type == "theme" && this.hasResource("preview.png")) {
+    if (isTheme(addon.type) && this.hasResource("preview.png")) {
       let url = this.getResourceURI("preview.png").spec;
       return [new AddonManagerPrivate.AddonScreenshot(url)];
     }
 
     return null;
   },
 
   get applyBackgroundUpdates() {
@@ -7486,21 +7523,23 @@ AddonWrapper.prototype = {
   },
   set userDisabled(val) {
     let addon = addonFor(this);
     if (val == this.userDisabled) {
       return val;
     }
 
     if (addon.inDatabase) {
-      if (addon.type == "theme" && val) {
+      let theme = isTheme(addon.type)
+      if (theme && val) {
         if (addon.internalName == XPIProvider.defaultSkin)
           throw new Error("Cannot disable the default theme");
         XPIProvider.enableDefaultTheme();
-      } else {
+      }
+      if (!(theme && val) || isWebExtension(addon.type)) {
         // hidden and system add-ons should not be user disasbled,
         // 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);
       }
     } else {
@@ -7515,20 +7554,22 @@ AddonWrapper.prototype = {
 
   set softDisabled(val) {
     let addon = addonFor(this);
     if (val == addon.softDisabled)
       return val;
 
     if (addon.inDatabase) {
       // When softDisabling a theme just enable the active theme
-      if (addon.type == "theme" && val && !addon.userDisabled) {
+      if (isTheme(addon.type) && val && !addon.userDisabled) {
         if (addon.internalName == XPIProvider.defaultSkin)
           throw new Error("Cannot disable the default theme");
         XPIProvider.enableDefaultTheme();
+        if (isWebExtension(addon.type))
+          XPIProvider.updateAddonDisabledState(addon, undefined, val);
       } else {
         XPIProvider.updateAddonDisabledState(addon, undefined, val);
       }
     } else if (!addon.userDisabled) {
       // Only set softDisabled if not already disabled
       addon.softDisabled = val;
     }
 
--- a/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
+++ b/toolkit/mozapps/extensions/internal/XPIProviderUtils.js
@@ -1128,32 +1128,34 @@ this.XPIDatabase = {
   getVisibleAddons(aTypes, aCallback) {
     this.getAddonList(aAddon => (aAddon.visible &&
                                  (!aTypes || (aTypes.length == 0) ||
                                   (aTypes.indexOf(aAddon.type) > -1))),
                       aCallback);
   },
 
   /**
-   * Synchronously gets all add-ons of a particular type.
+   * Synchronously gets all add-ons of a particular type(s).
    *
-   * @param  aType
-   *         The type of add-on to retrieve
+   * @param  aType, aType2, ...
+   *         The type(s) of add-on to retrieve
    * @return an array of DBAddonInternals
    */
-  getAddonsByType(aType) {
+  getAddonsByType(...aTypes) {
     if (!this.addonDB) {
       // jank-tastic! Must synchronously load DB if the theme switches from
       // an XPI theme to a lightweight theme before the DB has loaded,
       // because we're called from sync XPIProvider.addonChanged
-      logger.warn("Synchronous load of XPI database due to getAddonsByType(" + aType + ")");
+      logger.warn("Synchronous load of XPI database due to getAddonsByType([" +
+        aTypes.join(", ") + "])");
       AddonManagerPrivate.recordSimpleMeasure("XPIDB_lateOpen_byType", XPIProvider.runPhase);
       this.syncLoadDB(true);
     }
-    return _filterDB(this.addonDB, aAddon => (aAddon.type == aType));
+
+    return _filterDB(this.addonDB, aAddon => aTypes.includes(aAddon.type));
   },
 
   /**
    * Synchronously gets an add-on with a particular internalName.
    *
    * @param  aInternalName
    *         The internalName of the add-on to retrieve
    * @return a DBAddonInternal