Bug 1341277 - Part 1: Update ExtensionSettingsStore to support disabled settings. r=aswan
authorBob Silverberg <bsilverberg@mozilla.com>
Wed, 22 Feb 2017 11:35:10 -0500
changeset 374687 68d9a7c54c002b69afc30fa7e2bb133c63be7c6a
parent 374686 63b5159df4e5c815db0400ce3e3258727423cae1
child 374688 02d52f2c1d5534b3ac784ffe3dcf1c27ad6d7281
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1341277
milestone54.0a1
Bug 1341277 - Part 1: Update ExtensionSettingsStore to support disabled settings. r=aswan MozReview-Commit-ID: 4N67JXfO81D
toolkit/components/extensions/ExtensionSettingsStore.jsm
toolkit/components/extensions/test/xpcshell/test_ext_extensionSettingsStore.js
--- a/toolkit/components/extensions/ExtensionSettingsStore.jsm
+++ b/toolkit/components/extensions/ExtensionSettingsStore.jsm
@@ -19,17 +19,18 @@
  * {
  *   type: { // The type of settings being stored in this object, i.e., prefs.
  *     key: { // The unique key for the setting.
  *       initialValue, // The initial value of the setting.
  *       precedenceList: [
  *         {
  *           id, // The id of the extension requesting the setting.
  *           installDate, // The install date of the extension.
- *           value // The value of the setting requested by the extension.
+ *           value, // The value of the setting requested by the extension.
+ *           enabled // Whether the setting is currently enabled.
  *         }
  *       ],
  *     },
  *     key: {
  *       // ...
  *     }
  *   }
  * }
@@ -52,48 +53,133 @@ XPCOMUtils.defineLazyModuleGetter(this, 
                                   "resource://gre/modules/JSONFile.jsm");
 
 const JSON_FILE_NAME = "extension-settings.json";
 const STORE_PATH = OS.Path.join(Services.dirsvc.get("ProfD", Ci.nsIFile).path, JSON_FILE_NAME);
 
 let _store;
 
 // Get the internal settings store, which is persisted in a JSON file.
-async function getStore(type) {
+function getStore(type) {
   if (!_store) {
-    _store = new JSONFile({
+    let initStore = new JSONFile({
       path: STORE_PATH,
     });
-    await _store.load();
+    initStore.ensureDataReady();
+    _store = initStore;
   }
-  _store.ensureDataReady();
 
   // Ensure a property exists for the given type.
   if (!_store.data[type]) {
     _store.data[type] = {};
   }
 
   return _store;
 }
 
 // Return an object with properties for key and value|initialValue, or null
 // if no setting has been stored for that key.
 async function getTopItem(type, key) {
-  let store = await getStore(type);
+  let store = getStore(type);
 
   let keyInfo = store.data[type][key];
   if (!keyInfo) {
     return null;
   }
 
-  if (!keyInfo.precedenceList.length) {
-    return {key, initialValue: keyInfo.initialValue};
+  // Find the highest precedence, enabled setting.
+  for (let item of keyInfo.precedenceList) {
+    if (item.enabled) {
+      return {key, value: item.value};
+    }
   }
 
-  return {key, value: keyInfo.precedenceList[0].value};
+  // Nothing found in the precedenceList, return the initialValue.
+  return {key, initialValue: keyInfo.initialValue};
+}
+
+// Comparator used when sorting the precedence list.
+function precedenceComparator(a, b) {
+  if (a.enabled && !b.enabled) {
+    return -1;
+  }
+  if (b.enabled && !a.enabled) {
+    return 1;
+  }
+  return b.installDate - a.installDate;
+}
+
+/**
+ * Helper method that alters a setting, either by changing its enabled status
+ * or by removing it.
+ *
+ * @param {Extension} extension
+ *        The extension for which a setting is being removed/disabled.
+ * @param {string} type
+ *        The type of setting to be altered.
+ * @param {string} key
+ *        A string that uniquely identifies the setting.
+ * @param {string} action
+ *        The action to perform on the setting.
+ *        Will be one of remove|enable|disable.
+ *
+ * @returns {object | null}
+ *          Either an object with properties for key and value, which
+ *          corresponds to the current top precedent setting, or null if
+ *          the current top precedent setting has not changed.
+ */
+async function alterSetting(extension, type, key, action) {
+  let returnItem;
+  let store = getStore(type);
+
+  let keyInfo = store.data[type][key];
+  if (!keyInfo) {
+    throw new Error(
+      `Cannot alter the setting for ${type}:${key} as it does not exist.`);
+  }
+
+  let id = extension.id;
+  let foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
+
+  if (foundIndex === -1) {
+    throw new Error(
+      `Cannot alter the setting for ${type}:${key} as it does not exist.`);
+  }
+
+  switch (action) {
+    case "remove":
+      keyInfo.precedenceList.splice(foundIndex, 1);
+      break;
+
+    case "enable":
+      keyInfo.precedenceList[foundIndex].enabled = true;
+      keyInfo.precedenceList.sort(precedenceComparator);
+      foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
+      break;
+
+    case "disable":
+      keyInfo.precedenceList[foundIndex].enabled = false;
+      keyInfo.precedenceList.sort(precedenceComparator);
+      break;
+
+    default:
+      throw new Error(`${action} is not a valid action for alterSetting.`);
+  }
+
+  if (foundIndex === 0) {
+    returnItem = await getTopItem(type, key);
+  }
+
+  if (action === "remove" && keyInfo.precedenceList.length === 0) {
+    delete store.data[type][key];
+  }
+
+  store.saveSoon();
+
+  return returnItem;
 }
 
 this.ExtensionSettingsStore = {
   /**
    * Adds a setting to the store, possibly returning the current top precedent
    * setting.
    *
    * @param {Extension} extension
@@ -119,107 +205,120 @@ this.ExtensionSettingsStore = {
    *                          at the top of the precedence list.
    */
   async addSetting(extension, type, key, value, initialValueCallback, callbackArgument = key) {
     if (typeof initialValueCallback != "function") {
       throw new Error("initialValueCallback must be a function.");
     }
 
     let id = extension.id;
-    let store = await getStore(type);
+    let store = getStore(type);
 
     if (!store.data[type][key]) {
       // The setting for this key does not exist. Set the initial value.
       let initialValue = await initialValueCallback(callbackArgument);
       store.data[type][key] = {
         initialValue,
         precedenceList: [],
       };
     }
     let keyInfo = store.data[type][key];
     // Check for this item in the precedenceList.
     let foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
-    if (foundIndex == -1) {
+    if (foundIndex === -1) {
       // No item for this extension, so add a new one.
       let addon = await AddonManager.getAddonByID(id);
-      keyInfo.precedenceList.push({id, installDate: addon.installDate, value});
+      keyInfo.precedenceList.push({id, installDate: addon.installDate, value, enabled: true});
     } else {
       // Item already exists or this extension, so update it.
       keyInfo.precedenceList[foundIndex].value = value;
     }
 
     // Sort the list.
-    keyInfo.precedenceList.sort((a, b) => {
-      return b.installDate - a.installDate;
-    });
+    keyInfo.precedenceList.sort(precedenceComparator);
 
     store.saveSoon();
 
     // Check whether this is currently the top item.
     if (keyInfo.precedenceList[0].id == id) {
       return {key, value};
     }
     return null;
   },
 
   /**
-   * Removes a setting from the store, returning the current top precedent
-   * setting.
+   * Removes a setting from the store, possibly returning the current top
+   * precedent setting.
    *
-   * @param {Extension} extension The extension for which a setting is being removed.
-   * @param {string} type The type of setting to be removed.
-   * @param {string} key A string that uniquely identifies the setting.
+   * @param {Extension} extension
+   *        The extension for which a setting is being removed.
+   * @param {string} type
+   *        The type of setting to be removed.
+   * @param {string} key
+   *        A string that uniquely identifies the setting.
    *
-   * @returns {object | null} Either an object with properties for key and
-   *                          value, which corresponds to the current top
-   *                          precedent setting, or null if the current top
-   *                          precedent setting has not changed.
+   * @returns {object | null}
+   *          Either an object with properties for key and value, which
+   *          corresponds to the current top precedent setting, or null if
+   *          the current top precedent setting has not changed.
    */
   async removeSetting(extension, type, key) {
-    let returnItem;
-    let store = await getStore(type);
-
-    let keyInfo = store.data[type][key];
-    if (!keyInfo) {
-      throw new Error(
-        `Cannot remove setting for ${type}:${key} as it does not exist.`);
-    }
-
-    let id = extension.id;
-    let foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
+    return await alterSetting(extension, type, key, "remove");
+  },
 
-    if (foundIndex == -1) {
-      throw new Error(
-        `Cannot remove setting for ${type}:${key} as it does not exist.`);
-    }
-
-    keyInfo.precedenceList.splice(foundIndex, 1);
+  /**
+   * Enables a setting in the store, possibly returning the current top
+   * precedent setting.
+   *
+   * @param {Extension} extension
+   *        The extension for which a setting is being enabled.
+   * @param {string} type
+   *        The type of setting to be enabled.
+   * @param {string} key
+   *        A string that uniquely identifies the setting.
+   *
+   * @returns {object | null}
+   *          Either an object with properties for key and value, which
+   *          corresponds to the current top precedent setting, or null if
+   *          the current top precedent setting has not changed.
+   */
+  async enable(extension, type, key) {
+    return await alterSetting(extension, type, key, "enable");
+  },
 
-    if (foundIndex == 0) {
-      returnItem = await getTopItem(type, key);
-    }
-
-    if (keyInfo.precedenceList.length == 0) {
-      delete store.data[type][key];
-    }
-    store.saveSoon();
-
-    return returnItem;
+  /**
+   * Disables a setting in the store, possibly returning the current top
+   * precedent setting.
+   *
+   * @param {Extension} extension
+   *        The extension for which a setting is being disabled.
+   * @param {string} type
+   *        The type of setting to be disabled.
+   * @param {string} key
+   *        A string that uniquely identifies the setting.
+   *
+   * @returns {object | null}
+   *          Either an object with properties for key and value, which
+   *          corresponds to the current top precedent setting, or null if
+   *          the current top precedent setting has not changed.
+   */
+  async disable(extension, type, key) {
+    return await alterSetting(extension, type, key, "disable");
   },
 
   /**
    * Retrieves all settings from the store for a given extension.
    *
    * @param {Extension} extension The extension for which a settings are being retrieved.
    * @param {string} type The type of setting to be returned.
    *
    * @returns {array} A list of settings which have been stored for the extension.
    */
   async getAllForExtension(extension, type) {
-    let store = await getStore(type);
+    let store = getStore(type);
 
     let keysObj = store.data[type];
     let items = [];
     for (let key in keysObj) {
       if (keysObj[key].precedenceList.find(item => item.id == extension.id)) {
         items.push(key);
       }
     }
@@ -246,34 +345,42 @@ this.ExtensionSettingsStore = {
    *
    * It informs a caller of the state of a setting with respect to the current
    * extension, and can be one of the following values:
    *
    * controlled_by_other_extensions: controlled by extensions with higher precedence
    * controllable_by_this_extension: can be controlled by this extension
    * controlled_by_this_extension: controlled by this extension
    *
-   * @param {Extension} extension The extension for which levelOfControl is being
-   *                              requested.
-   * @param {string} type The type of setting to be returned. For example `pref`.
-   * @param {string} key A string that uniquely identifies the setting, for
-   *                     example, a preference name.
+   * @param {Extension} extension
+   *        The extension for which levelOfControl is being requested.
+   * @param {string} type
+   *        The type of setting to be returned. For example `pref`.
+   * @param {string} key
+   *        A string that uniquely identifies the setting, for example, a
+   *        preference name.
    *
-   * @returns {string} The level of control of the extension over the key.
+   * @returns {string}
+   *          The level of control of the extension over the key.
    */
   async getLevelOfControl(extension, type, key) {
-    let store = await getStore(type);
+    let store = getStore(type);
 
     let keyInfo = store.data[type][key];
     if (!keyInfo || !keyInfo.precedenceList.length) {
       return "controllable_by_this_extension";
     }
 
     let id = extension.id;
-    let topItem = keyInfo.precedenceList[0];
+    let enabledItems = keyInfo.precedenceList.filter(item => item.enabled);
+    if (!enabledItems.length) {
+      return "controllable_by_this_extension";
+    }
+
+    let topItem = enabledItems[0];
     if (topItem.id == id) {
       return "controlled_by_this_extension";
     }
 
     let addon = await AddonManager.getAddonByID(id);
     return topItem.installDate > addon.installDate ?
       "controlled_by_other_extensions" :
       "controllable_by_this_extension";
--- a/toolkit/components/extensions/test/xpcshell/test_ext_extensionSettingsStore.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_extensionSettingsStore.js
@@ -152,30 +152,80 @@ add_task(async function test_settings_st
     let items = await ExtensionSettingsStore.getAllForExtension(extension, TEST_TYPE);
     deepEqual(items, KEY_LIST, "getAllForExtension returns expected keys.");
   }
 
   // Attempting to remove a setting that has not been set should throw an exception.
   await Assert.rejects(
     ExtensionSettingsStore.removeSetting(
       extensions[0], "myType", "unset_key"),
-    /Cannot remove setting for myType:unset_key as it does not exist/,
+    /Cannot alter the setting for myType:unset_key as it does not exist/,
     "removeSetting rejects with an unset key.");
 
+  // Attempting to disable a setting that has not been set should throw an exception.
+  await Assert.rejects(
+    ExtensionSettingsStore.disable(extensions[0], "myType", "unset_key"),
+    /Cannot alter the setting for myType:unset_key as it does not exist/,
+    "disable rejects with an unset key.");
+
+  // Attempting to enable a setting that has not been set should throw an exception.
+  await Assert.rejects(
+    ExtensionSettingsStore.enable(extensions[0], "myType", "unset_key"),
+    /Cannot alter the setting for myType:unset_key as it does not exist/,
+    "enable rejects with an unset key.");
+
   let expectedKeys = KEY_LIST;
-  // Remove the non-top item for a key.
+  // Disable the non-top item for a key.
   for (let key of KEY_LIST) {
     let extensionIndex = 0;
     let item = await ExtensionSettingsStore.addSetting(
       extensions[extensionIndex], TEST_TYPE, key, "new value", initialValue);
     equal(callbackCount,
       expectedCallbackCount,
       "initialValueCallback called the expected number of times.");
     equal(item, null, "Updating non-top item for a key returns null");
-    item = await ExtensionSettingsStore.removeSetting(extensions[extensionIndex], TEST_TYPE, key);
+    item = await ExtensionSettingsStore.disable(extensions[extensionIndex], TEST_TYPE, key);
+    equal(item, null, "Disabling non-top item for a key returns null.");
+    let allForExtension = await ExtensionSettingsStore.getAllForExtension(extensions[extensionIndex], TEST_TYPE);
+    deepEqual(allForExtension, expectedKeys, "getAllForExtension returns expected keys after a disable.");
+    item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
+    deepEqual(
+      item,
+      ITEMS[key][2],
+      "getSetting returns correct item after a disable.");
+    let levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[extensionIndex], TEST_TYPE, key);
+    equal(
+      levelOfControl,
+      "controlled_by_other_extensions",
+      "getLevelOfControl returns correct levelOfControl after disabling of non-top item.");
+  }
+
+  // Re-enable the non-top item for a key.
+  for (let key of KEY_LIST) {
+    let extensionIndex = 0;
+    let item = await ExtensionSettingsStore.enable(extensions[extensionIndex], TEST_TYPE, key);
+    equal(item, null, "Enabling non-top item for a key returns null.");
+    let allForExtension = await ExtensionSettingsStore.getAllForExtension(extensions[extensionIndex], TEST_TYPE);
+    deepEqual(allForExtension, expectedKeys, "getAllForExtension returns expected keys after an enable.");
+    item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
+    deepEqual(
+      item,
+      ITEMS[key][2],
+      "getSetting returns correct item after an enable.");
+    let levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[extensionIndex], TEST_TYPE, key);
+    equal(
+      levelOfControl,
+      "controlled_by_other_extensions",
+      "getLevelOfControl returns correct levelOfControl after enabling of non-top item.");
+  }
+
+  // Remove the non-top item for a key.
+  for (let key of KEY_LIST) {
+    let extensionIndex = 0;
+    let item = await ExtensionSettingsStore.removeSetting(extensions[extensionIndex], TEST_TYPE, key);
     equal(item, null, "Removing non-top item for a key returns null.");
     expectedKeys = expectedKeys.filter(expectedKey => expectedKey != key);
     let allForExtension = await ExtensionSettingsStore.getAllForExtension(extensions[extensionIndex], TEST_TYPE);
     deepEqual(allForExtension, expectedKeys, "getAllForExtension returns expected keys after a removal.");
     item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
     deepEqual(
       item,
       ITEMS[key][2],
@@ -183,28 +233,62 @@ add_task(async function test_settings_st
     let levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[extensionIndex], TEST_TYPE, key);
     equal(
       levelOfControl,
       "controlled_by_other_extensions",
       "getLevelOfControl returns correct levelOfControl after removal of non-top item.");
   }
 
   for (let key of KEY_LIST) {
+    // Disable the top item for a key.
+    let item = await ExtensionSettingsStore.disable(extensions[2], TEST_TYPE, key);
+    deepEqual(
+      item,
+      ITEMS[key][1],
+      "Disabling top item for a key returns the new top item.");
+    item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
+    deepEqual(
+      item,
+      ITEMS[key][1],
+      "getSetting returns correct item after a disable.");
+    let levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[2], TEST_TYPE, key);
+    equal(
+      levelOfControl,
+      "controllable_by_this_extension",
+      "getLevelOfControl returns correct levelOfControl after disabling of top item.");
+
+    // Re-enable the top item for a key.
+    item = await ExtensionSettingsStore.enable(extensions[2], TEST_TYPE, key);
+    deepEqual(
+      item,
+      ITEMS[key][2],
+      "Re-enabling top item for a key returns the old top item.");
+    item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
+    deepEqual(
+      item,
+      ITEMS[key][2],
+      "getSetting returns correct item after an enable.");
+    levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[2], TEST_TYPE, key);
+    equal(
+      levelOfControl,
+      "controlled_by_this_extension",
+      "getLevelOfControl returns correct levelOfControl after re-enabling top item.");
+
     // Remove the top item for a key.
-    let item = await ExtensionSettingsStore.removeSetting(extensions[2], TEST_TYPE, key);
+    item = await ExtensionSettingsStore.removeSetting(extensions[2], TEST_TYPE, key);
     deepEqual(
       item,
       ITEMS[key][1],
       "Removing top item for a key returns the new top item.");
     item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
     deepEqual(
       item,
       ITEMS[key][1],
       "getSetting returns correct item after a removal.");
-    let levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[2], TEST_TYPE, key);
+    levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[2], TEST_TYPE, key);
     equal(
       levelOfControl,
       "controllable_by_this_extension",
       "getLevelOfControl returns correct levelOfControl after removal of top item.");
 
     // Add a setting for the current top item.
     let itemToAdd = {key, value: `new-${key}`};
     item = await ExtensionSettingsStore.addSetting(
@@ -222,21 +306,55 @@ add_task(async function test_settings_st
       itemToAdd,
       "getSetting returns correct item after updating.");
     levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[1], TEST_TYPE, key);
     equal(
       levelOfControl,
       "controlled_by_this_extension",
       "getLevelOfControl returns correct levelOfControl after updating.");
 
-    // Remove the last remaining item for a key.
+    // Disable the last remaining item for a key.
     let expectedItem = {key, initialValue: initialValue(key)};
     // We're using the callback to set the expected value, so we need to increment the
     // expectedCallbackCount.
     expectedCallbackCount++;
+    item = await ExtensionSettingsStore.disable(extensions[1], TEST_TYPE, key);
+    deepEqual(
+      item,
+      expectedItem,
+      "Disabling last item for a key returns the initial value.");
+    item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
+    deepEqual(
+      item,
+      expectedItem,
+      "getSetting returns the initial value after all are disabled.");
+    levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[1], TEST_TYPE, key);
+    equal(
+      levelOfControl,
+      "controllable_by_this_extension",
+      "getLevelOfControl returns correct levelOfControl after all are disabled.");
+
+    // Re-enable the last remaining item for a key.
+    item = await ExtensionSettingsStore.enable(extensions[1], TEST_TYPE, key);
+    deepEqual(
+      item,
+      itemToAdd,
+      "Re-enabling last item for a key returns the old value.");
+    item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
+    deepEqual(
+      item,
+      itemToAdd,
+      "getSetting returns expected value after re-enabling.");
+    levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[1], TEST_TYPE, key);
+    equal(
+      levelOfControl,
+      "controlled_by_this_extension",
+      "getLevelOfControl returns correct levelOfControl after re-enabling.");
+
+    // Remove the last remaining item for a key.
     item = await ExtensionSettingsStore.removeSetting(extensions[1], TEST_TYPE, key);
     deepEqual(
       item,
       expectedItem,
       "Removing last item for a key returns the initial value.");
     item = await ExtensionSettingsStore.getSetting(TEST_TYPE, key);
     deepEqual(
       item,
@@ -244,17 +362,17 @@ add_task(async function test_settings_st
       "getSetting returns null after all are removed.");
     levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[1], TEST_TYPE, key);
     equal(
       levelOfControl,
       "controllable_by_this_extension",
       "getLevelOfControl returns correct levelOfControl after all are removed.");
 
     // Attempting to remove a setting that has had all extensions removed should throw an exception.
-    let expectedMessage = new RegExp(`Cannot remove setting for ${TEST_TYPE}:${key} as it does not exist`);
+    let expectedMessage = new RegExp(`Cannot alter the setting for ${TEST_TYPE}:${key} as it does not exist`);
     await Assert.rejects(
       ExtensionSettingsStore.removeSetting(
         extensions[1], TEST_TYPE, key),
       expectedMessage,
       "removeSetting rejects with an key that has all records removed.");
   }
 
   // Test adding a setting with a value in callbackArgument.