Bug 1233492 - make browser/experiments eslintable, r?felipe draft
authorGijs Kruitbosch <gijskruitbosch@gmail.com>
Thu, 17 Dec 2015 19:42:24 +0000
changeset 316029 2daaa302b04cef72b173265dccdba55a538e58eb
parent 316015 0dd30d9b7c9a86a0033c9529f2b540e9ffcba7ee
child 512118 8da2f5cfa7cc4f0dca98c534f540cdb3fa2d9c71
push id8501
push usergijskruitbosch@gmail.com
push dateThu, 17 Dec 2015 19:49:39 +0000
reviewersfelipe
bugs1233492
milestone46.0a1
Bug 1233492 - make browser/experiments eslintable, r?felipe
.eslintignore
browser/experiments/Experiments.jsm
browser/experiments/test/xpcshell/head.js
browser/experiments/test/xpcshell/test_disableExperiments.js
browser/experiments/test/xpcshell/test_healthreport.js
--- a/.eslintignore
+++ b/.eslintignore
@@ -73,17 +73,16 @@ browser/components/places/**
 browser/components/pocket/**
 browser/components/preferences/**
 browser/components/privatebrowsing/**
 browser/components/sessionstore/**
 browser/components/shell/**
 browser/components/tabview/**
 browser/components/translation/**
 browser/components/uitour/**
-browser/experiments/**
 browser/extensions/pdfjs/**
 browser/extensions/shumway/**
 browser/fuel/**
 browser/locales/**
 browser/modules/**
 
 # Loop specific exclusions
 
--- a/browser/experiments/Experiments.jsm
+++ b/browser/experiments/Experiments.jsm
@@ -172,25 +172,25 @@ function addonInstallForURL(url, hash) {
   return deferred.promise;
 }
 
 // Returns a promise that is resolved with an Array<Addon> of the installed
 // experiment addons.
 function installedExperimentAddons() {
   let deferred = Promise.defer();
   AddonManager.getAddonsByTypes(["experiment"], (addons) => {
-    deferred.resolve([a for (a of addons) if (!a.appDisabled)]);
+    deferred.resolve(addons.filter(a => !a.appDisabled));
   });
   return deferred.promise;
 }
 
 // Takes an Array<Addon> and returns a promise that is resolved when the
 // addons are uninstalled.
 function uninstallAddons(addons) {
-  let ids = new Set([a.id for (a of addons)]);
+  let ids = new Set(addons.map(addon => addon.id));
   let deferred = Promise.defer();
 
   let listener = {};
   listener.onUninstalled = addon => {
     if (!ids.has(addon.id)) {
       return;
     }
 
@@ -460,21 +460,22 @@ Experiments.Experiments.prototype = {
 	  this._networkRequest.abort();
 	} catch (e) {
 	  // pass
 	}
       }
       try {
         this._log.trace("uninit: waiting on _mainTask");
         yield this._mainTask;
-      } catch (e if e instanceof AlreadyShutdownError) {
-        // We error out of tasks after shutdown via that exception.
       } catch (e) {
-        this._latestError = e;
-        throw e;
+        // We error out of tasks after shutdown via this exception.
+        if (!(e instanceof AlreadyShutdownError)) {
+          this._latestError = e;
+          throw e;
+        }
       }
     }
 
     this._log.info("Completed uninitialization.");
   }),
 
   // Return state information, for debugging purposes.
   _getState: function() {
@@ -757,29 +758,33 @@ Experiments.Experiments.prototype = {
 
   _run: function() {
     this._log.trace("_run");
     this._checkForShutdown();
     if (!this._mainTask) {
       this._mainTask = Task.spawn(function*() {
         try {
           yield this._main();
-	} catch (e if e instanceof CacheWriteError) {
-	  // In this case we want to reschedule
         } catch (e) {
-          this._log.error("_main caught error: " + e);
-          return;
+          // In the CacheWriteError case we want to reschedule
+          if (!(e instanceof CacheWriteError)) {
+            this._log.error("_main caught error: " + e);
+            return;
+          }
         } finally {
           this._mainTask = null;
         }
         this._log.trace("_main finished, scheduling next run");
         try {
           yield this._scheduleNextRun();
-        } catch (ex if ex instanceof AlreadyShutdownError) {
-          // We error out of tasks after shutdown via that exception.
+        } catch (ex) {
+          // We error out of tasks after shutdown via this exception.
+          if (!(ex instanceof AlreadyShutdownError)) {
+            throw ex;
+          }
         }
       }.bind(this));
     }
     return this._mainTask;
   },
 
   _main: function*() {
     do {
@@ -863,25 +868,28 @@ Experiments.Experiments.prototype = {
     let activeExperiment = this._getActiveExperiment();
     if (!activeExperiment || activeExperiment._addonId != addon.id) {
       return;
     }
 
     this.disableExperiment(TELEMETRY_LOG.TERMINATION.ADDON_UNINSTALLED);
   },
 
+  /**
+   * @returns {Boolean} returns false when we cancel the install.
+   */
   onInstallStarted: function (install) {
     if (install.addon.type != "experiment") {
-      return;
+      return true;
     }
 
     this._log.trace("onInstallStarted() - " + install.addon.id);
     if (install.addon.appDisabled) {
       // This is a PreviousExperiment
-      return;
+      return true;
     }
 
     // We want to be in control of all experiment add-ons: reject installs
     // for add-ons that we don't know about.
 
     // We have a race condition of sorts to worry about here. We have 2
     // onInstallStarted listeners. This one (the global one) and the one
     // created as part of ExperimentEntry._installAddon. Because of the order
@@ -891,23 +899,23 @@ Experiments.Experiments.prototype = {
     // will have its add-on ID set to null. We work around this by storing a
     // identifying field - the source URL of the install - in a module-level
     // variable (so multiple Experiments instances doesn't cancel each other
     // out).
 
     if (this._trackedAddonIds.has(install.addon.id)) {
       this._log.info("onInstallStarted allowing install because add-on ID " +
                      "tracked by us.");
-      return;
+      return true;
     }
 
     if (gActiveInstallURLs.has(install.sourceURI.spec)) {
       this._log.info("onInstallStarted allowing install because install " +
                      "tracked by us.");
-      return;
+      return true;
     }
 
     this._log.warn("onInstallStarted cancelling install of unknown " +
                    "experiment add-on: " + install.addon.id);
     return false;
   },
 
   // END OF ADD-ON LISTENERS.
@@ -982,17 +990,17 @@ Experiments.Experiments.prototype = {
    */
   _saveToCache: function* () {
     this._log.trace("_saveToCache");
     let path = this._cacheFilePath;
     this._dirty = false;
     try {
       let textData = JSON.stringify({
         version: CACHE_VERSION,
-        data: [e[1].toJSON() for (e of this._experiments.entries())],
+        data: [...this._experiments.values()].map(e => e.toJSON()),
       });
 
       let encoder = new TextEncoder();
       let data = encoder.encode(textData);
       let options = { tmpPath: path + ".tmp", compression: "lz4" };
       yield this._policy.delayCacheWrite(OS.File.writeAtomic(path, data, options));
     } catch (e) {
       // We failed to write the cache, it's still dirty.
@@ -1008,19 +1016,23 @@ Experiments.Experiments.prototype = {
    * Task function, load the cached experiments manifest file from disk.
    */
   _loadFromCache: Task.async(function* () {
     this._log.trace("_loadFromCache");
     let path = this._cacheFilePath;
     try {
       let result = yield loadJSONAsync(path, { compression: "lz4" });
       this._populateFromCache(result);
-    } catch (e if e instanceof OS.File.Error && e.becauseNoSuchFile) {
-      // No cached manifest yet.
-      this._experiments = new Map();
+    } catch (e) {
+      if (e instanceof OS.File.Error && e.becauseNoSuchFile) {
+        // No cached manifest yet.
+        this._experiments = new Map();
+      } else {
+        throw e;
+      }
     }
   }),
 
   _populateFromCache: function (data) {
     this._log.trace("populateFromCache() - data: " + JSON.stringify(data));
 
     // If the user has a newer cache version than we can understand, we fail
     // hard; no experiments should be active in this older client.
@@ -1113,17 +1125,17 @@ Experiments.Experiments.prototype = {
     let e = this._getActiveExperiment();
     if (!e) {
       return null;
     }
     return e.branch;
   },
 
   _getActiveExperiment: function () {
-    let enabled = [experiment for ([,experiment] of this._experiments) if (experiment._enabled)];
+    let enabled = [...this._experiments.values()].filter(experiment => experiment._enabled);
 
     if (enabled.length == 1) {
       return enabled[0];
     }
 
     if (enabled.length > 1) {
       this._log.error("getActiveExperimentId() - should not have more than 1 active experiment");
       throw new Error("have more than 1 active experiment");
@@ -1150,17 +1162,17 @@ Experiments.Experiments.prototype = {
   /**
    * The Set of add-on IDs that we know about from manifests.
    */
   get _trackedAddonIds() {
     if (!this._experiments) {
       return new Set();
     }
 
-    return new Set([e._addonId for ([,e] of this._experiments) if (e._addonId)]);
+    return new Set([...this._experiments.values()].map(e => e._addonId));
   },
 
   /*
    * Task function to check applicability of experiments, disable the active
    * experiment if needed and activate the first applicable candidate.
    */
   _evaluateExperiments: function*() {
     this._log.trace("_evaluateExperiments");
@@ -1178,20 +1190,20 @@ Experiments.Experiments.prototype = {
     // ExperimentEntry instances and stuff them inside this._experiments.
     // However, since ExperimentEntry contain lots of metadata from the
     // manifest and trying to make up data could be error prone, it's safer
     // to not try. Furthermore, if an experiment really did come from us, we
     // should have some record of it. In the end, we decide to discard all
     // knowledge for these unknown experiment add-ons.
     let installedExperiments = yield installedExperimentAddons();
     let expectedAddonIds = this._trackedAddonIds;
-    let unknownAddons = [a for (a of installedExperiments) if (!expectedAddonIds.has(a.id))];
+    let unknownAddons = installedExperiments.filter(a => !expectedAddonIds.has(a.id));
     if (unknownAddons.length) {
       this._log.warn("_evaluateExperiments() - unknown add-ons in AddonManager: " +
-                     [a.id for (a of unknownAddons)].join(", "));
+                     unknownAddons.map(a => a.id).join(", "));
 
       yield uninstallAddons(unknownAddons);
     }
 
     let activeExperiment = this._getActiveExperiment();
     let activeChanged = false;
     let now = this._policy.now();
 
@@ -2260,28 +2272,28 @@ this.Experiments.PreviousExperimentProvi
   },
 
   getAddonsByTypes: function (types, cb) {
     if (types && types.length > 0 && types.indexOf("experiment") == -1) {
       cb([]);
       return;
     }
 
-    cb([new PreviousExperimentAddon(e) for (e of this._experimentList)]);
+    cb(this._experimentList.map(e => new PreviousExperimentAddon(e)));
   },
 
   _updateExperimentList: function () {
     return this._experiments.getExperiments().then((experiments) => {
-      let list = [e for (e of experiments) if (!e.active)];
+      let list = experiments.filter(e => !e.active);
 
-      let newMap = new Map([[e.id, e] for (e of list)]);
-      let oldMap = new Map([[e.id, e] for (e of this._experimentList)]);
+      let newMap = new Map(list.map(e => [e.id, e]));
+      let oldMap = new Map(this._experimentList.map(e => [e.id, e]));
 
-      let added = [e.id for (e of list) if (!oldMap.has(e.id))];
-      let removed = [e.id for (e of this._experimentList) if (!newMap.has(e.id))];
+      let added = [...newMap.keys()].filter(id => !oldMap.has(id));
+      let removed = [...oldMap.keys()].filter(id => !newMap.has(id));
 
       for (let id of added) {
         this._log.trace("updateExperimentList() - adding " + id);
         let wrapper = new PreviousExperimentAddon(newMap.get(id));
         AddonManagerPrivate.callInstallListeners("onExternalInstall", null, wrapper, null, false);
         AddonManagerPrivate.callAddonListeners("onInstalling", wrapper, false);
       }
 
--- a/browser/experiments/test/xpcshell/head.js
+++ b/browser/experiments/test/xpcshell/head.js
@@ -33,19 +33,21 @@ function sha1File(path) {
 
   let is = Cc["@mozilla.org/network/file-input-stream;1"]
              .createInstance(Ci.nsIFileInputStream);
   is.init(f, -1, 0, 0);
   hasher.updateFromStream(is, Math.pow(2, 32) - 1);
   is.close();
   let bytes = hasher.finish(false);
 
-  return [("0" + bytes.charCodeAt(byte).toString(16)).slice(-2)
-          for (byte in bytes)]
-         .join("");
+  let rv = "";
+  for (let i = 0; i < bytes.length; i++) {
+    rv += ("0" + bytes.charCodeAt(i).toString(16)).substr(-2);
+  }
+  return rv;
 }
 
 const EXPERIMENT1_ID       = "test-experiment-1@tests.mozilla.org";
 const EXPERIMENT1_XPI_NAME = "experiment-1.xpi";
 const EXPERIMENT1_NAME     = "Test experiment 1";
 const EXPERIMENT1_PATH     = getExperimentPath(EXPERIMENT1_XPI_NAME);
 const EXPERIMENT1_XPI_SHA1 = "sha1:" + sha1File(EXPERIMENT1_PATH);
 
@@ -148,17 +150,17 @@ function startAddonManagerOnly() {
 
 function getExperimentAddons(previous=false) {
   let deferred = Promise.defer();
 
   AddonManager.getAddonsByTypes(["experiment"], (addons) => {
     if (previous) {
       deferred.resolve(addons);
     } else {
-      deferred.resolve([a for (a of addons) if (!a.appDisabled)]);
+      deferred.resolve(addons.filter(a => !a.appDisabled));
     }
   });
 
   return deferred.promise;
 }
 
 function createAppInfo(optionsIn) {
   const XULAPPINFO_CONTRACTID = "@mozilla.org/xre/app-info;1";
--- a/browser/experiments/test/xpcshell/test_disableExperiments.js
+++ b/browser/experiments/test/xpcshell/test_disableExperiments.js
@@ -151,18 +151,21 @@ add_task(function* test_disableExperimen
 
   // Trigger update, clock set for experiment 2 to start. Verify we don't start it.
 
   now = startDate2;
   defineNow(gPolicy, now);
 
   try {
     yield experiments.updateManifest();
-  } catch (e if e.message == "experiments are disabled") {
-    // This exception is expected.
+  } catch (e) {
+    // This exception is expected, we rethrow everything else
+    if (e.message != "experiments are disabled") {
+      throw e;
+    }
   }
 
   experiments.notify();
   yield experiments._mainTask;
 
   Assert.equal(observerFireCount, expectedObserverFireCount,
                "Experiments observer should not have been called.");
 
--- a/browser/experiments/test/xpcshell/test_healthreport.js
+++ b/browser/experiments/test/xpcshell/test_healthreport.js
@@ -35,17 +35,17 @@ add_test(function setup() {
 
   Services.prefs.setBoolPref(PREF_EXPERIMENTS_ENABLED, true);
   Services.prefs.setBoolPref(PREF_TELEMETRY_ENABLED, true);
   Services.prefs.setBoolPref(PREF_HEALTHREPORT_ENABLED, true);
 
   run_next_test();
 });
 
-add_task(function test_constructor() {
+add_task(function* test_constructor() {
   Experiments.instance();
   yield Experiments._mainTask;
   let provider = new ExperimentsProvider();
 });
 
 add_task(function* test_init() {
   let storage = yield Metrics.Storage("init");
   let provider = new ExperimentsProvider();