Bug 1367598 - Sync shield-recipe-client from GitHub (v55, commit 4d836a) r=Gijs
authorMike Cooper <mcooper@mozilla.com>
Wed, 24 May 2017 15:59:50 -0700
changeset 360693 6953f7b4d74917385af231b3cea8abd9c7daeb3f
parent 360692 5772650731f5b570a087425cf430415236c3b8bd
child 360694 bdf2d1918fb0b434ab8782d003afbe3f9639cd45
child 360712 221302eab0e9687fabd0d4f146636b3d508ae495
push id90713
push userryanvm@gmail.com
push dateThu, 25 May 2017 20:36:06 +0000
treeherdermozilla-inbound@348e3ebeb9ac [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersGijs
bugs1367598
milestone55.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 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]