Bug 1220911: Remove most of the special casing around experiments. r=rhelmer

Experiments should differ from normal add-ons in a few ways:

* They can always be enabled regardless of compatibility info
* They default to disabled when installed
* They cannot be checked for updates
* They only stay enabled for the lifetime of the current process
* The UI doesn't give users the ability to enable/disable

This makes a few changes to keep these differences but remove much of the special casing code for experiments.

Being able to use regardless of compatibility was mostly fixed by bug 1220198 but I've also removed the redundant override in isCompatible.

Previously the "enabled until restart" feature worked with by not updating the DBAddonInternal object and instead using a hack to make the wrapper still seem enabled. This seems likely to break other code that relies on the state of the DBAddonInternal object so instead we update that as normal and simply don't persist the enabled state to disk.

Also switch the DBAddonInteral.prototype code to use some newer JS features.

I've removed the hack from addon.permissions which was hiding the enable/disable buttons in the UI and instead just hidden them in the UI stylesheet. This makes the API make sense and means callers can use addon.permissions to verify that enabling will work.

toolkit/mozapps/extensions/content/extensions.css

@@ -244,9 +244,13 @@ richlistitem:not([selected]) * {
 .view-pane[type="experiment"] .addon:not([pending="uninstall"]) .pending,
 .view-pane[type="experiment"] .disabled-postfix,
 .view-pane[type="experiment"] .update-postfix,
+.view-pane[type="experiment"] .addon-control.enable,
+.view-pane[type="experiment"] .addon-control.disable,
 #detail-view[type="experiment"] .alert-container,
 #detail-view[type="experiment"] #detail-version,
-#detail-view[type="experiment"] #detail-creator {
+#detail-view[type="experiment"] #detail-creator,
+#detail-view[type="experiment"] #detail-enable-btn,
+#detail-view[type="experiment"] #detail-disable-btn {
   display: none;
 }
 

toolkit/mozapps/extensions/content/extensions.xml

@@ -919,6 +919,7 @@
 
     <implementation>
       <constructor><![CDATA[
+        this._installStatus = document.getAnonymousElementByAttribute(this, "anonid", "install-status");
         this._installStatus.mControl = this;
 
         this.setAttribute("contextmenu", "addonitem-popup");

toolkit/mozapps/extensions/internal/XPIProvider.jsm

@@ -1189,16 +1189,16 @@ function loadManifestFromRDF(aUri, aStream) {
     addon.userDisabled = !!LightweightThemeManager.currentTheme ||
                          addon.internalName != XPIProvider.selectedSkin;
   }
-  // Experiments are disabled by default. It is up to the Experiments Manager
-  // to enable them (it drives installation).
   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;
-    addon.softDisabled = addon.blocklistState == Blocklist.STATE_SOFTBLOCKED;
   }
 
+  addon.softDisabled = addon.blocklistState == Blocklist.STATE_SOFTBLOCKED;
   addon.applyBackgroundUpdates = AddonManager.AUTOUPDATE_DEFAULT;
 
   // Experiments are managed and updated through an external "experiments
@@ -1207,9 +1207,6 @@ function loadManifestFromRDF(aUri, aStream) {
     addon.applyBackgroundUpdates = AddonManager.AUTOUPDATE_DISABLE;
     addon.updateURL = null;
     addon.updateKey = null;
-
-    addon.targetApplications = [];
-    addon.targetPlatforms = [];
   }
 
   // icons will be filled by the calling function
@@ -2327,8 +2324,6 @@ this.XPIProvider = {
   _mostRecentlyModifiedFile: {},
   // Per-addon telemetry information
   _telemetryDetails: {},
-  // Experiments are disabled by default. Track ones that are locally enabled.
-  _enabledExperiments: null,
   // A Map from an add-on install to its ID
   _addonFileMap: new Map(),
   // Flag to know if ToolboxProcess.jsm has already been loaded by someone or not
@@ -2539,8 +2534,6 @@ this.XPIProvider = {
       this._shutdownError = null;
       // Clear this at startup for xpcshell test restarts
       this._telemetryDetails = {};
-      // Clear the set of enabled experiments (experiments disabled by default).
-      this._enabledExperiments = new Set();
       // Register our details structure with AddonManager
       AddonManagerPrivate.setTelemetryDetails("XPI", this._telemetryDetails);
 
@@ -4558,15 +4551,11 @@ this.XPIProvider = {
     let appDisabledChanged = aAddon.appDisabled != appDisabled;
 
     // Update the properties in the database.
-    // We never persist this for experiments because the disabled flags
-    // are controlled by the Experiments Manager.
-    if (aAddon.type != "experiment") {
-      XPIDatabase.setAddonProperties(aAddon, {
-        userDisabled: aUserDisabled,
-        appDisabled: appDisabled,
-        softDisabled: aSoftDisabled
-      });
-    }
+    XPIDatabase.setAddonProperties(aAddon, {
+      userDisabled: aUserDisabled,
+      appDisabled: appDisabled,
+      softDisabled: aSoftDisabled
+    });
 
     let wrapper = createWrapper(aAddon);
 
@@ -6387,17 +6376,6 @@ AddonInternal.prototype = {
   },
 
   isCompatibleWith: function AddonInternal_isCompatibleWith(aAppVersion, aPlatformVersion) {
-    // Experiments are installed through an external mechanism that
-    // limits target audience to compatible clients. We trust it knows what
-    // it's doing and skip compatibility checks.
-    //
-    // This decision does forfeit defense in depth. If the experiments system
-    // is ever wrong about targeting an add-on to a specific application
-    // or platform, the client will likely see errors.
-    if (this.type == "experiment") {
-      return true;
-    }
-
     let app = this.matchingTargetApplication;
     if (!app)
       return false;
@@ -6573,13 +6551,6 @@ AddonInternal.prototype = {
     if (!(this.inDatabase))
       return permissions;
 
-    // Experiments can only be uninstalled. An uninstall reflects the user
-    // intent of "disable this experiment." This is partially managed by the
-    // experiments manager.
-    if (this.type == "experiment") {
-      return AddonManager.PERM_CAN_UNINSTALL;
-    }
-
     if (!this.appDisabled) {
       if (this.userDisabled || this.softDisabled) {
         permissions |= AddonManager.PERM_CAN_ENABLE;
@@ -6592,8 +6563,9 @@ AddonInternal.prototype = {
     // Add-ons that are in locked install locations, or are pending uninstall
     // cannot be upgraded or uninstalled
     if (!this._installLocation.locked && !this.pendingUninstall) {
-      // Add-ons that are installed by a file link cannot be upgraded
-      if (!this._installLocation.isLinkedAddon(this.id)) {
+      // Experiments cannot be upgraded.
+      // Add-ons that are installed by a file link cannot be upgraded.
+      if (this.type != "experiment" && !this._installLocation.isLinkedAddon(this.id)) {
         permissions |= AddonManager.PERM_CAN_UPGRADE;
       }
 
@@ -6924,15 +6896,10 @@ function AddonWrapper(aAddon) {
       return AddonManager.PENDING_UNINSTALL;
     }
 
-    // Extensions have an intentional inconsistency between what the DB says is
-    // enabled and what we say to the ouside world. so we need to cover up that
-    // lie here as well.
-    if (aAddon.type != "experiment") {
-      if (aAddon.active && aAddon.disabled)
-        pending |= AddonManager.PENDING_DISABLE;
-      else if (!aAddon.active && !aAddon.disabled)
-        pending |= AddonManager.PENDING_ENABLE;
-    }
+    if (aAddon.active && aAddon.disabled)
+      pending |= AddonManager.PENDING_DISABLE;
+    else if (!aAddon.active && !aAddon.disabled)
+      pending |= AddonManager.PENDING_ENABLE;
 
     if (aAddon.pendingUpgrade)
       pending |= AddonManager.PENDING_UPGRADE;
@@ -6971,10 +6938,6 @@ function AddonWrapper(aAddon) {
   });
 
   this.__defineGetter__("userDisabled", function AddonWrapper_userDisabledGetter() {
-    if (XPIProvider._enabledExperiments.has(aAddon.id)) {
-      return false;
-    }
-
     return aAddon.softDisabled || aAddon.userDisabled;
   });
   this.__defineSetter__("userDisabled", function AddonWrapper_userDisabledSetter(val) {
@@ -6982,14 +6945,6 @@ function AddonWrapper(aAddon) {
       return val;
     }
 
-    if (aAddon.type == "experiment") {
-      if (val) {
-        XPIProvider._enabledExperiments.delete(aAddon.id);
-      } else {
-        XPIProvider._enabledExperiments.add(aAddon.id);
-      }
-    }
-
     if (aAddon.inDatabase) {
       if (aAddon.type == "theme" && val) {
         if (aAddon.internalName == XPIProvider.defaultSkin)

toolkit/mozapps/extensions/internal/XPIProviderUtils.js

@@ -357,45 +357,48 @@ function DBAddonInternal(aLoaded) {
     });
 }
 
-function DBAddonInternalPrototype()
-{
-  this.applyCompatibilityUpdate =
-    function(aUpdate, aSyncCompatibility) {
-      let wasCompatible = this.isCompatible;
+DBAddonInternal.prototype = Object.create(AddonInternal.prototype);
+Object.assign(DBAddonInternal.prototype, {
+  applyCompatibilityUpdate: function(aUpdate, aSyncCompatibility) {
+    let wasCompatible = this.isCompatible;
 
-      this.targetApplications.forEach(function(aTargetApp) {
-        aUpdate.targetApplications.forEach(function(aUpdateTarget) {
-          if (aTargetApp.id == aUpdateTarget.id && (aSyncCompatibility ||
-              Services.vc.compare(aTargetApp.maxVersion, aUpdateTarget.maxVersion) < 0)) {
-            aTargetApp.minVersion = aUpdateTarget.minVersion;
-            aTargetApp.maxVersion = aUpdateTarget.maxVersion;
-            XPIDatabase.saveChanges();
-          }
-        });
+    this.targetApplications.forEach(function(aTargetApp) {
+      aUpdate.targetApplications.forEach(function(aUpdateTarget) {
+        if (aTargetApp.id == aUpdateTarget.id && (aSyncCompatibility ||
+            Services.vc.compare(aTargetApp.maxVersion, aUpdateTarget.maxVersion) < 0)) {
+          aTargetApp.minVersion = aUpdateTarget.minVersion;
+          aTargetApp.maxVersion = aUpdateTarget.maxVersion;
+          XPIDatabase.saveChanges();
+        }
       });
-      if (aUpdate.multiprocessCompatible !== undefined &&
-          aUpdate.multiprocessCompatible != this.multiprocessCompatible) {
-        this.multiprocessCompatible = aUpdate.multiprocessCompatible;
-        XPIDatabase.saveChanges();
-      }
+    });
+    if (aUpdate.multiprocessCompatible !== undefined &&
+        aUpdate.multiprocessCompatible != this.multiprocessCompatible) {
+      this.multiprocessCompatible = aUpdate.multiprocessCompatible;
+      XPIDatabase.saveChanges();
+    }
 
-      if (wasCompatible != this.isCompatible)
-        XPIProvider.updateAddonDisabledState(this);
-    };
+    if (wasCompatible != this.isCompatible)
+      XPIProvider.updateAddonDisabledState(this);
+  },
 
-  this.toJSON =
-    function() {
-      return copyProperties(this, PROP_JSON_FIELDS);
-    };
+  toJSON: function() {
+    let jsonData = copyProperties(this, PROP_JSON_FIELDS);
 
-  Object.defineProperty(this, "inDatabase",
-                        { get: function() { return true; },
-                          enumerable: true,
-                          configurable: true });
-}
-DBAddonInternalPrototype.prototype = AddonInternal.prototype;
+    // Experiments are serialized as disabled so they aren't run on the next
+    // startup.
+    if (this.type == "experiment") {
+      jsonData.userDisabled = true;
+      jsonData.active = false;
+    }
 
-DBAddonInternal.prototype = new DBAddonInternalPrototype();
+    return jsonData;
+  },
+
+  get inDatabase() {
+    return true;
+  }
+});
 
 /**
  * Internal interface: find an addon from an already loaded addonDB

toolkit/mozapps/extensions/test/xpcshell/test_experiment.js

@@ -35,7 +35,7 @@ add_task(function* test_experiment() {
   Assert.equal(addon.updateURL, null, "No updateURL for experiments.");
   Assert.equal(addon.applyBackgroundUpdates, AddonManager.AUTOUPDATE_DISABLE,
                "Background updates are disabled.");
-  Assert.equal(addon.permissions, AddonManager.PERM_CAN_UNINSTALL,
+  Assert.equal(addon.permissions, AddonManager.PERM_CAN_UNINSTALL + AddonManager.PERM_CAN_ENABLE,
                "Permissions are minimal.");
   Assert.ok(!(addon.pendingOperations & AddonManager.PENDING_ENABLE),
             "Should not be pending enable");