Bug 1367598 - Sync shield-recipe-client from GitHub (v55, commit 4d836a) r=Gijs draft
authorMike Cooper <mcooper@mozilla.com>
Wed, 24 May 2017 15:59:50 -0700
changeset 584535 ecb8134b8b8b64faf46fe4449ef37d0aa881d43a
parent 584534 3f7f4882f9d4ea7f583dcbf5f1b11af0da4e4be7
child 584551 82458af179102ce99aaf65a6a1c3db1bfddb1139
child 584553 700dab9d26b56abf769980e31b68f784b7150415
push id60775
push userbmo:mcooper@mozilla.com
push dateThu, 25 May 2017 16:53:26 +0000
reviewersGijs
bugs1367598
milestone55.0a1
Bug 1367598 - Sync shield-recipe-client from GitHub (v55, commit 4d836a) r=Gijs MozReview-Commit-ID: CA6nXwIeQo5
browser/extensions/shield-recipe-client/install.rdf.in
browser/extensions/shield-recipe-client/lib/Heartbeat.jsm
browser/extensions/shield-recipe-client/lib/NormandyApi.jsm
browser/extensions/shield-recipe-client/lib/NormandyDriver.jsm
browser/extensions/shield-recipe-client/lib/PreferenceExperiments.jsm
browser/extensions/shield-recipe-client/lib/RecipeRunner.jsm
browser/extensions/shield-recipe-client/lib/Storage.jsm
browser/extensions/shield-recipe-client/test/.eslintrc.js
browser/extensions/shield-recipe-client/test/browser/.eslintrc.js
browser/extensions/shield-recipe-client/test/browser/browser_NormandyDriver.js
browser/extensions/shield-recipe-client/test/browser/browser_Storage.js
browser/extensions/shield-recipe-client/test/browser/head.js
browser/extensions/shield-recipe-client/test/unit/.eslintrc.js
browser/extensions/shield-recipe-client/test/unit/head_xpc.js
browser/extensions/shield-recipe-client/test/unit/test_NormandyApi.js
browser/extensions/shield-recipe-client/test/unit/test_Sampling.js
browser/extensions/shield-recipe-client/test/unit/test_SandboxManager.js
browser/extensions/shield-recipe-client/test/unit/test_Utils.js
browser/extensions/shield-recipe-client/test/unit/utils.js
browser/extensions/shield-recipe-client/test/unit/xpc_head.js
browser/extensions/shield-recipe-client/test/unit/xpcshell.ini
--- a/browser/extensions/shield-recipe-client/install.rdf.in
+++ b/browser/extensions/shield-recipe-client/install.rdf.in
@@ -3,17 +3,17 @@
 #filter substitution
 
 <RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#" xmlns:em="http://www.mozilla.org/2004/em-rdf#">
   <Description about="urn:mozilla:install-manifest">
     <em:id>shield-recipe-client@mozilla.org</em:id>
     <em:type>2</em:type>
     <em:bootstrap>true</em:bootstrap>
     <em:unpack>false</em:unpack>
-    <em:version>51</em:version>
+    <em:version>55</em:version>
     <em:name>Shield Recipe Client</em:name>
     <em:description>Client to download and run recipes for SHIELD, Heartbeat, etc.</em:description>
     <em:multiprocessCompatible>true</em:multiprocessCompatible>
 
     <em:targetApplication>
       <Description>
         <em:id>{ec8030f7-c20a-464f-9b0e-13a3a9e97384}</em:id>
         <em:minVersion>@MOZ_APP_VERSION@</em:minVersion>
--- a/browser/extensions/shield-recipe-client/lib/Heartbeat.jsm
+++ b/browser/extensions/shield-recipe-client/lib/Heartbeat.jsm
@@ -3,17 +3,17 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 const {utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource://gre/modules/TelemetryController.jsm");
-Cu.import("resource://gre/modules/Timer.jsm"); /* globals setTimeout, clearTimeout */
+Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://shield-recipe-client/lib/CleanupManager.jsm");
 Cu.import("resource://shield-recipe-client/lib/EventEmitter.jsm");
 Cu.import("resource://shield-recipe-client/lib/LogManager.jsm");
 
 Cu.importGlobalProperties(["URL"]); /* globals URL */
 
 this.EXPORTED_SYMBOLS = ["Heartbeat"];
 
--- a/browser/extensions/shield-recipe-client/lib/NormandyApi.jsm
+++ b/browser/extensions/shield-recipe-client/lib/NormandyApi.jsm
@@ -57,19 +57,22 @@ this.NormandyApi = {
       return url;
     } else if (url.startsWith("/")) {
       return server + url;
     }
     throw new Error("Can't use relative urls");
   },
 
   async getApiUrl(name) {
-    const apiBase = prefs.getCharPref("api_url");
     if (!indexPromise) {
-      indexPromise = this.get(apiBase).then(res => res.json());
+      let apiBase = new URL(prefs.getCharPref("api_url"));
+      if (!apiBase.pathname.endsWith("/")) {
+        apiBase.pathname += "/";
+      }
+      indexPromise = this.get(apiBase.toString()).then(res => res.json());
     }
     const index = await indexPromise;
     if (!(name in index)) {
       throw new Error(`API endpoint with name "${name}" not found.`);
     }
     const url = index[name];
     return this.absolutify(url);
   },
--- a/browser/extensions/shield-recipe-client/lib/NormandyDriver.jsm
+++ b/browser/extensions/shield-recipe-client/lib/NormandyDriver.jsm
@@ -5,17 +5,17 @@
 "use strict";
 
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
 Cu.import("resource://gre/modules/Preferences.jsm");
 Cu.import("resource:///modules/ShellService.jsm");
 Cu.import("resource://gre/modules/AddonManager.jsm");
-Cu.import("resource://gre/modules/Timer.jsm"); /* globals setTimeout, clearTimeout */
+Cu.import("resource://gre/modules/Timer.jsm");
 Cu.import("resource://shield-recipe-client/lib/LogManager.jsm");
 Cu.import("resource://shield-recipe-client/lib/Storage.jsm");
 Cu.import("resource://shield-recipe-client/lib/Heartbeat.jsm");
 Cu.import("resource://shield-recipe-client/lib/FilterExpressions.jsm");
 Cu.import("resource://shield-recipe-client/lib/ClientEnvironment.jsm");
 Cu.import("resource://shield-recipe-client/lib/PreferenceExperiments.jsm");
 Cu.import("resource://shield-recipe-client/lib/Sampling.jsm");
 
@@ -118,25 +118,30 @@ this.NormandyDriver = function(sandboxMa
     },
 
     uuid() {
       let ret = generateUUID().toString();
       ret = ret.slice(1, ret.length - 1);
       return ret;
     },
 
-    createStorage(keyPrefix) {
-      let storage;
-      try {
-        storage = Storage.makeStorage(keyPrefix, sandbox);
-      } catch (e) {
-        log.error(e.stack);
-        throw e;
+    createStorage(prefix) {
+      const storage = new Storage(prefix);
+
+      // Wrapped methods that we expose to the sandbox. These are documented in
+      // the driver spec in docs/dev/driver.rst.
+      const storageInterface = {};
+      for (const method of ["getItem", "setItem", "removeItem", "clear"]) {
+        storageInterface[method] = sandboxManager.wrapAsync(storage[method].bind(storage), {
+          cloneArguments: true,
+          cloneInto: true,
+        });
       }
-      return storage;
+
+      return sandboxManager.cloneInto(storageInterface, {cloneFunctions: true});
     },
 
     setTimeout(cb, time) {
       if (typeof cb !== "function") {
         throw new sandbox.Error(`setTimeout must be called with a function, got "${typeof cb}"`);
       }
       const token = setTimeout(() => {
         cb();
--- a/browser/extensions/shield-recipe-client/lib/PreferenceExperiments.jsm
+++ b/browser/extensions/shield-recipe-client/lib/PreferenceExperiments.jsm
@@ -36,18 +36,16 @@
  *   Value to change the preference to during the experiment.
  * @property {string} preferenceType
  *   Type of the preference value being set.
  * @property {string|integer|boolean|undefined} previousPreferenceValue
  *   Value of the preference prior to the experiment, or undefined if it was
  *   unset.
  * @property {PreferenceBranchType} preferenceBranchType
  *   Controls how we modify the preference to affect the client.
- * @rejects {Error}
- *   If the given preferenceType does not match the existing stored preference.
  *
  *   If "default", when the experiment is active, the default value for the
  *   preference is modified on startup of the add-on. If "user", the user value
  *   for the preference is modified when the experiment starts, and is reset to
  *   its original value when the experiment ends.
  */
 
 "use strict";
@@ -168,18 +166,19 @@ this.PreferenceExperiments = {
    * Start a new preference experiment.
    * @param {Object} experiment
    * @param {string} experiment.name
    * @param {string} experiment.branch
    * @param {string} experiment.preferenceName
    * @param {string|integer|boolean} experiment.preferenceValue
    * @param {PreferenceBranchType} experiment.preferenceBranchType
    * @rejects {Error}
-   *   If an experiment with the given name already exists, or if an experiment
-   *   for the given preference is active.
+   *   - If an experiment with the given name already exists
+   *   - if an experiment for the given preference is active
+   *   - If the given preferenceType does not match the existing stored preference
    */
   async start({name, branch, preferenceName, preferenceValue, preferenceBranchType, preferenceType}) {
     log.debug(`PreferenceExperiments.start(${name}, ${branch})`);
 
     const store = await ensureStorage();
     if (name in store.data) {
       throw new Error(`A preference experiment named "${name}" already exists.`);
     }
@@ -216,17 +215,18 @@ this.PreferenceExperiments = {
     const givenPrefType = PREFERENCE_TYPE_MAP[preferenceType];
 
     if (!preferenceType || !givenPrefType) {
       throw new Error(`Invalid preferenceType provided (given "${preferenceType}")`);
     }
 
     if (prevPrefType !== Services.prefs.PREF_INVALID && prevPrefType !== givenPrefType) {
       throw new Error(
-        `Previous preference value is of type "${prevPrefType}", but was given "${givenPrefType}" (${preferenceType})`
+        `Previous preference value is of type "${prevPrefType}", but was given ` +
+        `"${givenPrefType}" (${preferenceType})`
       );
     }
 
     preferences.set(preferenceName, preferenceValue);
     PreferenceExperiments.startObserver(name, preferenceName, preferenceValue);
     store.data[name] = experiment;
     store.saveSoon();
 
--- a/browser/extensions/shield-recipe-client/lib/RecipeRunner.jsm
+++ b/browser/extensions/shield-recipe-client/lib/RecipeRunner.jsm
@@ -25,17 +25,17 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://shield-recipe-client/lib/SandboxManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ClientEnvironment",
                                   "resource://shield-recipe-client/lib/ClientEnvironment.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "CleanupManager",
                                   "resource://shield-recipe-client/lib/CleanupManager.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "ActionSandboxManager",
                                   "resource://shield-recipe-client/lib/ActionSandboxManager.jsm");
 
-Cu.importGlobalProperties(["fetch"]); /* globals fetch */
+Cu.importGlobalProperties(["fetch"]);
 
 this.EXPORTED_SYMBOLS = ["RecipeRunner"];
 
 const log = LogManager.getLogger("recipe-runner");
 const prefs = Services.prefs.getBranch("extensions.shield-recipe-client.");
 const TIMER_NAME = "recipe-client-addon-run";
 const RUN_INTERVAL_PREF = "run_interval_seconds";
 
--- a/browser/extensions/shield-recipe-client/lib/Storage.jsm
+++ b/browser/extensions/shield-recipe-client/lib/Storage.jsm
@@ -1,147 +1,89 @@
 /* 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";
 
 const {utils: Cu} = Components;
 Cu.import("resource://gre/modules/XPCOMUtils.jsm");
-Cu.import("resource://shield-recipe-client/lib/LogManager.jsm");
 
 XPCOMUtils.defineLazyModuleGetter(this, "JSONFile", "resource://gre/modules/JSONFile.jsm");
 XPCOMUtils.defineLazyModuleGetter(this, "OS", "resource://gre/modules/osfile.jsm");
 
 this.EXPORTED_SYMBOLS = ["Storage"];
 
-const log = LogManager.getLogger("storage");
-let storePromise;
-
-function loadStorage() {
-  if (storePromise === undefined) {
-    const path = OS.Path.join(OS.Constants.Path.profileDir, "shield-recipe-client.json");
-    const storage = new JSONFile({path});
-    storePromise = (async function() {
-      await storage.load();
-      return storage;
-    })();
-  }
-  return storePromise;
-}
-
-this.Storage = {
-  makeStorage(prefix, sandbox) {
-    if (!sandbox) {
-      throw new Error("No sandbox passed");
-    }
-
-    const storageInterface = {
-      /**
-       * Sets an item in the prefixed storage.
-       * @returns {Promise}
-       * @resolves With the stored value, or null.
-       * @rejects Javascript exception.
-       */
-      getItem(keySuffix) {
-        return new sandbox.Promise((resolve, reject) => {
-          loadStorage()
-            .then(store => {
-              const namespace = store.data[prefix] || {};
-              const value = namespace[keySuffix] || null;
-              resolve(Cu.cloneInto(value, sandbox));
-            })
-            .catch(err => {
-              log.error(err);
-              reject(new sandbox.Error());
-            });
-        });
-      },
+// Lazy-load JSON file that backs Storage instances.
+XPCOMUtils.defineLazyGetter(this, "lazyStore", async function() {
+  const path = OS.Path.join(OS.Constants.Path.profileDir, "shield-recipe-client.json");
+  const store = new JSONFile({path});
+  await store.load();
+  return store;
+});
 
-      /**
-       * Sets an item in the prefixed storage.
-       * @returns {Promise}
-       * @resolves When the operation is completed succesfully
-       * @rejects Javascript exception.
-       */
-      setItem(keySuffix, value) {
-        return new sandbox.Promise((resolve, reject) => {
-          loadStorage()
-            .then(store => {
-              if (!(prefix in store.data)) {
-                store.data[prefix] = {};
-              }
-              store.data[prefix][keySuffix] = Cu.cloneInto(value, {});
-              store.saveSoon();
-              resolve();
-            })
-            .catch(err => {
-              log.error(err);
-              reject(new sandbox.Error());
-            });
-        });
-      },
-
-      /**
-       * Removes a single item from the prefixed storage.
-       * @returns {Promise}
-       * @resolves When the operation is completed succesfully
-       * @rejects Javascript exception.
-       */
-      removeItem(keySuffix) {
-        return new sandbox.Promise((resolve, reject) => {
-          loadStorage()
-            .then(store => {
-              if (!(prefix in store.data)) {
-                return;
-              }
-              delete store.data[prefix][keySuffix];
-              store.saveSoon();
-              resolve();
-            })
-            .catch(err => {
-              log.error(err);
-              reject(new sandbox.Error());
-            });
-        });
-      },
-
-      /**
-       * Clears all storage for the prefix.
-       * @returns {Promise}
-       * @resolves When the operation is completed succesfully
-       * @rejects Javascript exception.
-       */
-      clear() {
-        return new sandbox.Promise((resolve, reject) => {
-          return loadStorage()
-            .then(store => {
-              store.data[prefix] = {};
-              store.saveSoon();
-              resolve();
-            })
-            .catch(err => {
-              log.error(err);
-              reject(new sandbox.Error());
-            });
-        });
-      },
-    };
-
-    return Cu.cloneInto(storageInterface, sandbox, {
-      cloneFunctions: true,
-    });
-  },
+this.Storage = class {
+  constructor(prefix) {
+    this.prefix = prefix;
+  }
 
   /**
    * Clear ALL storage data and save to the disk.
    */
-  clearAllStorage() {
-    return loadStorage()
-      .then(store => {
-        store.data = {};
-        store.saveSoon();
-      })
-      .catch(err => {
-        log.error(err);
-      });
-  },
+  static async clearAllStorage() {
+    const store = await lazyStore;
+    store.data = {};
+    store.saveSoon();
+  }
+
+  /**
+   * Sets an item in the prefixed storage.
+   * @returns {Promise}
+   * @resolves With the stored value, or null.
+   * @rejects Javascript exception.
+   */
+  async getItem(name) {
+    const store = await lazyStore;
+    const namespace = store.data[this.prefix] || {};
+    return namespace[name] || null;
+  }
+
+  /**
+   * Sets an item in the prefixed storage.
+   * @returns {Promise}
+   * @resolves When the operation is completed succesfully
+   * @rejects Javascript exception.
+   */
+  async setItem(name, value) {
+    const store = await lazyStore;
+    if (!(this.prefix in store.data)) {
+      store.data[this.prefix] = {};
+    }
+    store.data[this.prefix][name] = value;
+    store.saveSoon();
+  }
+
+  /**
+   * Removes a single item from the prefixed storage.
+   * @returns {Promise}
+   * @resolves When the operation is completed succesfully
+   * @rejects Javascript exception.
+   */
+  async removeItem(name) {
+    const store = await lazyStore;
+    if (this.prefix in store.data) {
+      delete store.data[this.prefix][name];
+      store.saveSoon();
+    }
+  }
+
+  /**
+   * Clears all storage for the prefix.
+   * @returns {Promise}
+   * @resolves When the operation is completed succesfully
+   * @rejects Javascript exception.
+   */
+  async clear() {
+    const store = await lazyStore;
+    store.data[this.prefix] = {};
+    store.saveSoon();
+  }
 };
--- a/browser/extensions/shield-recipe-client/test/.eslintrc.js
+++ b/browser/extensions/shield-recipe-client/test/.eslintrc.js
@@ -1,16 +1,9 @@
 "use strict";
 
 module.exports = {
-  globals: {
-    Assert: false,
-    add_task: false,
-    getRootDirectory: false,
-    gTestPath: false,
-    Cu: false,
-  },
   rules: {
     "spaced-comment": 2,
     "space-before-function-paren": 2,
     "require-yield": 0
   }
 };
--- a/browser/extensions/shield-recipe-client/test/browser/.eslintrc.js
+++ b/browser/extensions/shield-recipe-client/test/browser/.eslintrc.js
@@ -1,23 +1,17 @@
 "use strict";
 
 module.exports = {
+  extends: [
+    "plugin:mozilla/browser-test"
+  ],
+
+  plugins: [
+    "mozilla"
+  ],
+
   globals: {
-    BrowserTestUtils: false,
-    is: false,
-    isnot: false,
-    ok: false,
-    SpecialPowers: false,
-    SimpleTest: false,
-    registerCleanupFunction: false,
-    window: false,
-    sinon: false,
-    Cu: false,
-    Ci: false,
-    Cc: false,
-    UUID_REGEX: false,
-    withSandboxManager: false,
-    withDriver: false,
-    withMockNormandyApi: false,
-    withMockPreferences: false,
-  },
+    // Bug 1366720 - SimpleTest isn't being exported correctly, so list
+    // it here for now.
+    "SimpleTest": false
+  }
 };
--- a/browser/extensions/shield-recipe-client/test/browser/browser_NormandyDriver.js
+++ b/browser/extensions/shield-recipe-client/test/browser/browser_NormandyDriver.js
@@ -1,10 +1,12 @@
 "use strict";
 
+Cu.import("resource://shield-recipe-client/lib/NormandyDriver.jsm", this);
+
 add_task(withDriver(Assert, async function uuids(driver) {
   // Test that it is a UUID
   const uuid1 = driver.uuid();
   ok(UUID_REGEX.test(uuid1), "valid uuid format");
 
   // Test that UUIDs are different each time
   const uuid2 = driver.uuid();
   isnot(uuid1, uuid2, "uuids are unique");
@@ -37,8 +39,44 @@ add_task(withDriver(Assert, async functi
 add_task(withDriver(Assert, async function distribution(driver) {
   let client = await driver.client();
   is(client.distribution, "default", "distribution has a default value");
 
   await SpecialPowers.pushPrefEnv({set: [["distribution.id", "funnelcake"]]});
   client = await driver.client();
   is(client.distribution, "funnelcake", "distribution is read from preferences");
 }));
+
+add_task(withSandboxManager(Assert, async function testCreateStorage(sandboxManager) {
+  const driver = new NormandyDriver(sandboxManager);
+  sandboxManager.cloneIntoGlobal("driver", driver, {cloneFunctions: true});
+
+  // Assertion helpers
+  sandboxManager.addGlobal("is", is);
+  sandboxManager.addGlobal("deepEqual", (...args) => Assert.deepEqual(...args));
+
+  await sandboxManager.evalInSandbox(`
+    (async function sandboxTest() {
+      const store = driver.createStorage("testprefix");
+      const otherStore = driver.createStorage("othertestprefix");
+      await store.clear();
+      await otherStore.clear();
+
+      await store.setItem("willremove", 7);
+      await otherStore.setItem("willremove", 4);
+      is(await store.getItem("willremove"), 7, "createStorage stores sandbox values");
+      is(
+        await otherStore.getItem("willremove"),
+        4,
+        "values are not shared between createStorage stores",
+      );
+
+      const deepValue = {"foo": ["bar", "baz"]};
+      await store.setItem("deepValue", deepValue);
+      deepEqual(await store.getItem("deepValue"), deepValue, "createStorage clones stored values");
+
+      await store.removeItem("willremove");
+      is(await store.getItem("willremove"), null, "createStorage removes items");
+
+      is('prefix' in store, false, "createStorage doesn't expose non-whitelist attributes");
+    })();
+  `);
+}));
--- a/browser/extensions/shield-recipe-client/test/browser/browser_Storage.js
+++ b/browser/extensions/shield-recipe-client/test/browser/browser_Storage.js
@@ -1,17 +1,16 @@
 "use strict";
 
 Cu.import("resource://shield-recipe-client/lib/Storage.jsm", this);
 Cu.import("resource://shield-recipe-client/lib/SandboxManager.jsm", this);
 
 add_task(async function() {
-  const fakeSandbox = {Promise};
-  const store1 = Storage.makeStorage("prefix1", fakeSandbox);
-  const store2 = Storage.makeStorage("prefix2", fakeSandbox);
+  const store1 = new Storage("prefix1");
+  const store2 = new Storage("prefix2");
 
   // Make sure values return null before being set
   Assert.equal(await store1.getItem("key"), null);
   Assert.equal(await store2.getItem("key"), null);
 
   // Set values to check
   await store1.setItem("key", "value1");
   await store2.setItem("key", "value2");
@@ -40,30 +39,8 @@ add_task(async function() {
   await store1.setItem("removeTest", 1);
   await store2.setItem("removeTest", 2);
   Assert.equal(await store1.getItem("removeTest"), 1);
   Assert.equal(await store2.getItem("removeTest"), 2);
   await Storage.clearAllStorage();
   Assert.equal(await store1.getItem("removeTest"), null);
   Assert.equal(await store2.getItem("removeTest"), null);
 });
-
-add_task(async function testSandboxValueStorage() {
-  const manager1 = new SandboxManager();
-  const manager2 = new SandboxManager();
-  const store1 = Storage.makeStorage("testSandboxValueStorage", manager1.sandbox);
-  const store2 = Storage.makeStorage("testSandboxValueStorage", manager2.sandbox);
-  manager1.addGlobal("store", store1);
-  manager2.addGlobal("store", store2);
-  manager1.addHold("testing");
-  manager2.addHold("testing");
-
-  await manager1.evalInSandbox("store.setItem('foo', {foo: 'bar'});");
-  manager1.removeHold("testing");
-  await manager1.isNuked();
-
-  const objectMatches = await manager2.evalInSandbox(`
-    store.getItem("foo").then(item => item.foo === "bar");
-  `);
-  ok(objectMatches, "Values persisted in a store survive after their originating sandbox is nuked");
-
-  manager2.removeHold("testing");
-});
--- a/browser/extensions/shield-recipe-client/test/browser/head.js
+++ b/browser/extensions/shield-recipe-client/test/browser/head.js
@@ -5,16 +5,17 @@ Cu.import("resource://gre/modules/Prefer
 Cu.import("resource://shield-recipe-client/lib/SandboxManager.jsm", this);
 Cu.import("resource://shield-recipe-client/lib/NormandyDriver.jsm", this);
 Cu.import("resource://shield-recipe-client/lib/NormandyApi.jsm", this);
 Cu.import("resource://shield-recipe-client/lib/Utils.jsm", this);
 
 // Load mocking/stubbing library, sinon
 // docs: http://sinonjs.org/docs/
 const loader = Cc["@mozilla.org/moz/jssubscript-loader;1"].getService(Ci.mozIJSSubScriptLoader);
+/* global sinon */
 loader.loadSubScript("resource://testing-common/sinon-1.16.1.js");
 
 // Make sinon assertions fail in a way that mochitest understands
 sinon.assert.fail = function(message) {
   ok(false, message);
 };
 
 registerCleanupFunction(async function() {
--- a/browser/extensions/shield-recipe-client/test/unit/.eslintrc.js
+++ b/browser/extensions/shield-recipe-client/test/unit/.eslintrc.js
@@ -1,15 +1,11 @@
 "use strict";
 
 module.exports = {
-  globals: {
-    do_get_file: false,
-    equal: false,
-    Cu: false,
-    ok: false,
-    load: false,
-    do_register_cleanup: false,
-    sinon: false,
-    notEqual: false,
-    deepEqual: false,
-  },
+  extends: [
+    "plugin:mozilla/xpcshell-test"
+  ],
+
+  plugins: [
+    "mozilla"
+  ],
 };
rename from browser/extensions/shield-recipe-client/test/unit/xpc_head.js
rename to browser/extensions/shield-recipe-client/test/unit/head_xpc.js
--- a/browser/extensions/shield-recipe-client/test/unit/xpc_head.js
+++ b/browser/extensions/shield-recipe-client/test/unit/head_xpc.js
@@ -1,10 +1,12 @@
 "use strict";
 
+// List these manually due to bug 1366719.
+/* global Cc, Ci, Cu */
 const {classes: Cc, interfaces: Ci, utils: Cu} = Components;
 
 Cu.import("resource://gre/modules/Services.jsm");
 
 // Load our bootstrap extension manifest so we can access our chrome/resource URIs.
 // Cargo culted from formautofill system add-on
 const EXTENSION_ID = "shield-recipe-client@mozilla.org";
 let extensionDir = Services.dirsvc.get("GreD", Ci.nsIFile);
@@ -19,10 +21,12 @@ if (!extensionDir.exists()) {
 Components.manager.addBootstrappedManifestLocation(extensionDir);
 
 // Load Sinon for mocking/stubbing during tests.
 // Sinon assumes that setTimeout and friends are available, and looks for a
 // global object named self during initialization.
 Cu.import("resource://gre/modules/Timer.jsm");
 const self = {}; // eslint-disable-line no-unused-vars
 
+/* global sinon */
+/* exported sinon */
 const loader = Cc["@mozilla.org/moz/jssubscript-loader;1"].getService(Ci.mozIJSSubScriptLoader);
 loader.loadSubScript("resource://testing-common/sinon-1.16.1.js");
--- a/browser/extensions/shield-recipe-client/test/unit/test_NormandyApi.js
+++ b/browser/extensions/shield-recipe-client/test/unit/test_NormandyApi.js
@@ -1,31 +1,29 @@
 "use strict";
-// Cu is defined in xpc_head.js
-/* globals Cu */
 
 Cu.import("resource://gre/modules/Services.jsm");
-Cu.import("resource://testing-common/httpd.js"); /* globals HttpServer */
-Cu.import("resource://gre/modules/osfile.jsm", this); /* globals OS */
+Cu.import("resource://testing-common/httpd.js");
+Cu.import("resource://gre/modules/osfile.jsm", this);
 Cu.import("resource://shield-recipe-client/lib/NormandyApi.jsm", this);
 
 load("utils.js"); /* globals withMockPreferences */
 
 function withServer(server, task) {
   return withMockPreferences(async function inner(preferences) {
     const serverUrl = `http://localhost:${server.identity.primaryPort}`;
     preferences.set("extensions.shield-recipe-client.api_url", `${serverUrl}/api/v1`);
     preferences.set(
       "security.content.signature.root_hash",
       // Hash of the key that signs the normandy dev certificates
       "4C:35:B1:C3:E3:12:D9:55:E7:78:ED:D0:A7:E7:8A:38:83:04:EF:01:BF:FA:03:29:B2:46:9F:3C:C5:EC:36:04"
     );
 
     try {
-      await task(serverUrl);
+      await task(serverUrl, preferences);
     } finally {
       await new Promise(resolve => server.stop(resolve));
     }
   });
 }
 
 function makeScriptServer(scriptPath) {
   const server = new HttpServer();
@@ -72,28 +70,60 @@ function makeMockApiServer() {
 }
 
 function withMockApiServer(task) {
   return withServer(makeMockApiServer(), task);
 }
 
 add_task(withMockApiServer(async function test_get(serverUrl) {
   // Test that NormandyApi can fetch from the test server.
-  const response = await NormandyApi.get(`${serverUrl}/api/v1`);
+  const response = await NormandyApi.get(`${serverUrl}/api/v1/`);
   const data = await response.json();
   equal(data["recipe-list"], "/api/v1/recipe/", "Expected data in response");
 }));
 
 add_task(withMockApiServer(async function test_getApiUrl(serverUrl) {
   const apiBase = `${serverUrl}/api/v1`;
   // Test that NormandyApi can use the self-describing API's index
   const recipeListUrl = await NormandyApi.getApiUrl("action-list");
   equal(recipeListUrl, `${apiBase}/action/`, "Can retrieve action-list URL from API");
 }));
 
+add_task(withMockApiServer(async function test_getApiUrlSlashes(serverUrl, preferences) {
+  const fakeResponse = {
+    async json() {
+      return {"test-endpoint": `${serverUrl}/test/`};
+    },
+  };
+  const mockGet = sinon.stub(NormandyApi, "get", async () => fakeResponse);
+
+  // without slash
+  {
+    NormandyApi.clearIndexCache();
+    preferences.set("extensions.shield-recipe-client.api_url", `${serverUrl}/api/v1`);
+    const endpoint = await NormandyApi.getApiUrl("test-endpoint");
+    equal(endpoint, `${serverUrl}/test/`);
+    ok(mockGet.calledWithExactly(`${serverUrl}/api/v1/`), "trailing slash was added");
+    mockGet.reset();
+  }
+
+  // with slash
+  {
+    NormandyApi.clearIndexCache();
+    preferences.set("extensions.shield-recipe-client.api_url", `${serverUrl}/api/v1/`);
+    const endpoint = await NormandyApi.getApiUrl("test-endpoint");
+    equal(endpoint, `${serverUrl}/test/`);
+    ok(mockGet.calledWithExactly(`${serverUrl}/api/v1/`), "existing trailing slash was preserved");
+    mockGet.reset();
+  }
+
+  NormandyApi.clearIndexCache();
+  mockGet.restore();
+}));
+
 add_task(withMockApiServer(async function test_fetchRecipes() {
   const recipes = await NormandyApi.fetchRecipes();
   equal(recipes.length, 1);
   equal(recipes[0].name, "system-addon-test");
 }));
 
 add_task(withMockApiServer(async function test_classifyClient() {
   const classification = await NormandyApi.classifyClient();
--- a/browser/extensions/shield-recipe-client/test/unit/test_Sampling.js
+++ b/browser/extensions/shield-recipe-client/test/unit/test_Sampling.js
@@ -1,11 +1,9 @@
 "use strict";
-// Cu is defined in xpc_head.js
-/* globals Cu */
 
 Cu.import("resource://shield-recipe-client/lib/Sampling.jsm", this);
 
 add_task(async function testStableSample() {
   // Absolute samples
   equal(await Sampling.stableSample("test", 1), true, "stableSample returns true for 100% sample");
   equal(await Sampling.stableSample("test", 0), false, "stableSample returns false for 0% sample");
 
--- a/browser/extensions/shield-recipe-client/test/unit/test_SandboxManager.js
+++ b/browser/extensions/shield-recipe-client/test/unit/test_SandboxManager.js
@@ -1,15 +1,15 @@
 "use strict";
 
 Cu.import("resource://shield-recipe-client/lib/SandboxManager.jsm");
 
 // wrapAsync should wrap privileged Promises with Promises that are usable by
 // the sandbox.
-add_task(async function() {
+add_task(function* () {
   const manager = new SandboxManager();
   manager.addHold("testing");
 
   manager.cloneIntoGlobal("driver", {
     async privileged() {
       return "privileged";
     },
     wrapped: manager.wrapAsync(async function() {
@@ -20,17 +20,17 @@ add_task(async function() {
       return this.aValue;
     }),
   }, {cloneFunctions: true});
 
   // Assertion helpers
   manager.addGlobal("ok", ok);
   manager.addGlobal("equal", equal);
 
-  const sandboxResult = await new Promise(resolve => {
+  const sandboxResult = yield new Promise(resolve => {
     manager.addGlobal("resolve", result => resolve(result));
     manager.evalInSandbox(`
       // Unwrapped privileged promises are not accessible in the sandbox
       try {
         const privilegedResult = driver.privileged().then(() => false);
         ok(false, "The sandbox could not use a privileged Promise");
       } catch (err) { }
 
@@ -40,31 +40,31 @@ add_task(async function() {
 
       // Resolve the Promise around the sandbox with the wrapped result to test
       // that the Promise in the sandbox works.
       wrappedResult.then(resolve);
     `);
   });
   equal(sandboxResult, "wrapped", "wrapAsync methods return Promises that work in the sandbox");
 
-  await manager.evalInSandbox(`
+  yield manager.evalInSandbox(`
     (async function sandboxTest() {
       equal(
         await driver.wrappedThis(),
         "aValue",
         "wrapAsync preserves the behavior of the this keyword",
       );
     })();
   `);
 
   manager.removeHold("testing");
 });
 
 // wrapAsync cloning options
-add_task(async function() {
+add_task(function* () {
   const manager = new SandboxManager();
   manager.addHold("testing");
 
   // clonedArgument stores the argument passed to cloned(), which we use to test
   // that arguments from within the sandbox are cloned outside.
   let clonedArgument = null;
   manager.cloneIntoGlobal("driver", {
     uncloned: manager.wrapAsync(async function() {
@@ -75,17 +75,17 @@ add_task(async function() {
       return {value: "cloned"};
     }, {cloneInto: true, cloneArguments: true}),
   }, {cloneFunctions: true});
 
   // Assertion helpers
   manager.addGlobal("ok", ok);
   manager.addGlobal("deepEqual", deepEqual);
 
-  await new Promise(resolve => {
+  yield new Promise(resolve => {
     manager.addGlobal("resolve", resolve);
     manager.evalInSandbox(`
       (async function() {
         // The uncloned return value should be privileged and inaccesible.
         const uncloned = await driver.uncloned();
         ok(!("value" in uncloned), "The sandbox could not use an uncloned return value");
 
         // The cloned return value should be usable.
--- a/browser/extensions/shield-recipe-client/test/unit/test_Utils.js
+++ b/browser/extensions/shield-recipe-client/test/unit/test_Utils.js
@@ -1,10 +1,9 @@
 "use strict";
-/* globals Cu */
 
 Cu.import("resource://shield-recipe-client/lib/Utils.jsm");
 
 add_task(async function testKeyBy() {
   const list = [];
   deepEqual(Utils.keyBy(list, "foo"), {});
 
   const foo = {name: "foo", value: 1};
--- a/browser/extensions/shield-recipe-client/test/unit/utils.js
+++ b/browser/extensions/shield-recipe-client/test/unit/utils.js
@@ -1,11 +1,14 @@
 "use strict";
 /* eslint-disable no-unused-vars */
 
+// Loaded into the same scope as head_xpc.js
+/* import-globals-from head_xpc.js */
+
 Cu.import("resource://gre/modules/Preferences.jsm");
 
 const preferenceBranches = {
   user: Preferences,
   default: new Preferences({defaultBranch: true}),
 };
 
 // duplicated from test/browser/head.js until we move everything over to mochitests.
--- a/browser/extensions/shield-recipe-client/test/unit/xpcshell.ini
+++ b/browser/extensions/shield-recipe-client/test/unit/xpcshell.ini
@@ -1,10 +1,10 @@
 [DEFAULT]
-head = xpc_head.js
+head = head_xpc.js
 support-files =
   mock_api/**
   query_server.sjs
   echo_server.sjs
   utils.js
 
 [test_NormandyApi.js]
 [test_Sampling.js]