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 394637 68d9a7c54c002b69afc30fa7e2bb133c63be7c6a
parent 394636 63b5159df4e5c815db0400ce3e3258727423cae1
child 394638 02d52f2c1d5534b3ac784ffe3dcf1c27ad6d7281
push id1468
push userasasaki@mozilla.com
push dateMon, 05 Jun 2017 19:31:07 +0000
treeherdermozilla-release@0641fc6ee9d1 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaswan
bugs1341277
milestone54.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 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.