Bug 1367598 - Sync shield-recipe-client from GitHub (v55, commit 4d836a) r=Gijs

MozReview-Commit-ID: CA6nXwIeQo5

--HG--
rename : browser/extensions/shield-recipe-client/test/unit/xpc_head.js => browser/extensions/shield-recipe-client/test/unit/head_xpc.js
extra : rebase_source : ecb8134b8b8b64faf46fe4449ef37d0aa881d43a

browser/extensions/shield-recipe-client/install.rdf.in

@@ -8,7 +8,7 @@
     <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>

browser/extensions/shield-recipe-client/lib/Heartbeat.jsm

@@ -8,7 +8,7 @@ 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");

browser/extensions/shield-recipe-client/lib/NormandyApi.jsm

@@ -62,9 +62,12 @@ this.NormandyApi = {
   },
 
   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)) {

browser/extensions/shield-recipe-client/lib/NormandyDriver.jsm

@@ -10,7 +10,7 @@ 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");
@@ -123,15 +123,20 @@ this.NormandyDriver = function(sandboxManager) {
       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) {

browser/extensions/shield-recipe-client/lib/PreferenceExperiments.jsm

@@ -41,8 +41,6 @@
  *   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
@@ -173,8 +171,9 @@ this.PreferenceExperiments = {
    * @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})`);
@@ -221,7 +220,8 @@ this.PreferenceExperiments = {
 
     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})`
       );
     }
 

browser/extensions/shield-recipe-client/lib/RecipeRunner.jsm

@@ -30,7 +30,7 @@ XPCOMUtils.defineLazyModuleGetter(this, "CleanupManager",
 XPCOMUtils.defineLazyModuleGetter(this, "ActionSandboxManager",
                                   "resource://shield-recipe-client/lib/ActionSandboxManager.jsm");
 
-Cu.importGlobalProperties(["fetch"]); /* globals fetch */
+Cu.importGlobalProperties(["fetch"]);
 
 this.EXPORTED_SYMBOLS = ["RecipeRunner"];
 

browser/extensions/shield-recipe-client/lib/Storage.jsm

@@ -6,142 +6,84 @@
 
 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;
+// 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;
+});
 
-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;
-    })();
+this.Storage = class {
+  constructor(prefix) {
+    this.prefix = prefix;
   }
-  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());
-            });
-        });
-      },
-
-      /**
-       * 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,
-    });
-  },
 
   /**
    * 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();
+  }
 };

browser/extensions/shield-recipe-client/test/.eslintrc.js

@@ -1,13 +1,6 @@
 "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,

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
+  }
 };

browser/extensions/shield-recipe-client/test/browser/browser_NormandyDriver.js

@@ -1,5 +1,7 @@
 "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();
@@ -42,3 +44,39 @@ add_task(withDriver(Assert, async function distribution(driver) {
   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");
+    })();
+  `);
+}));

browser/extensions/shield-recipe-client/test/browser/browser_Storage.js

@@ -4,9 +4,8 @@ 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);
@@ -45,25 +44,3 @@ add_task(async function() {
   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");
-});

browser/extensions/shield-recipe-client/test/browser/head.js

@@ -10,6 +10,7 @@ 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

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"
+  ],
 };

browser/extensions/shield-recipe-client/test/unit/head_xpc.js

@@ -1,5 +1,7 @@
 "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");
@@ -24,5 +26,7 @@ Components.manager.addBootstrappedManifestLocation(extensionDir);
 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");

browser/extensions/shield-recipe-client/test/unit/test_NormandyApi.js

@@ -1,10 +1,8 @@
 "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 */
@@ -20,7 +18,7 @@ function withServer(server, task) {
     );
 
     try {
-      await task(serverUrl);
+      await task(serverUrl, preferences);
     } finally {
       await new Promise(resolve => server.stop(resolve));
     }
@@ -77,7 +75,7 @@ function withMockApiServer(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");
 }));
@@ -89,6 +87,38 @@ add_task(withMockApiServer(async function test_getApiUrl(serverUrl) {
   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);

browser/extensions/shield-recipe-client/test/unit/test_Sampling.js

@@ -1,6 +1,4 @@
 "use strict";
-// Cu is defined in xpc_head.js
-/* globals Cu */
 
 Cu.import("resource://shield-recipe-client/lib/Sampling.jsm", this);
 

browser/extensions/shield-recipe-client/test/unit/test_SandboxManager.js

@@ -4,7 +4,7 @@ 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");
 
@@ -25,7 +25,7 @@ add_task(async function() {
   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
@@ -45,7 +45,7 @@ add_task(async function() {
   });
   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(),
@@ -59,7 +59,7 @@ add_task(async function() {
 });
 
 // wrapAsync cloning options
-add_task(async function() {
+add_task(function* () {
   const manager = new SandboxManager();
   manager.addHold("testing");
 
@@ -80,7 +80,7 @@ add_task(async function() {
   manager.addGlobal("ok", ok);
   manager.addGlobal("deepEqual", deepEqual);
 
-  await new Promise(resolve => {
+  yield new Promise(resolve => {
     manager.addGlobal("resolve", resolve);
     manager.evalInSandbox(`
       (async function() {

browser/extensions/shield-recipe-client/test/unit/test_Utils.js

@@ -1,5 +1,4 @@
 "use strict";
-/* globals Cu */
 
 Cu.import("resource://shield-recipe-client/lib/Utils.jsm");
 

browser/extensions/shield-recipe-client/test/unit/utils.js

@@ -1,6 +1,9 @@
 "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 = {

browser/extensions/shield-recipe-client/test/unit/xpcshell.ini

@@ -1,5 +1,5 @@
 [DEFAULT]
-head = xpc_head.js
+head = head_xpc.js
 support-files =
   mock_api/**
   query_server.sjs