Bug 1330494 - Part 1: Remove async from functions in ExtensionSettingsStore that don't need to be async, r?aswan draft
authorBob Silverberg <bsilverberg@mozilla.com>
Thu, 29 Jun 2017 11:27:47 -0700
changeset 608552 c18623deb90ea77f81b05c7adec4fb66ddb4adf7
parent 608448 5c3e7cb5a35e1e56c7eb24ca392d22309f7af9f5
child 608553 746fd8ac359395838281e2d0abcc9d5269131024
push id68323
push userbmo:bob.silverberg@gmail.com
push dateThu, 13 Jul 2017 20:45:14 +0000
reviewersaswan
bugs1330494
milestone56.0a1
Bug 1330494 - Part 1: Remove async from functions in ExtensionSettingsStore that don't need to be async, r?aswan MozReview-Commit-ID: KmpBdrLmVNK Also added a hasSetting method to ExtensionSettingsStore to check whether a particular extension has a setting stored for a particulay key.
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
@@ -72,17 +72,17 @@ function getStore(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) {
+function getTopItem(type, key) {
   let store = getStore(type);
 
   let keyInfo = store.data[type][key];
   if (!keyInfo) {
     return null;
   }
 
   // Find the highest precedence, enabled setting.
@@ -121,17 +121,17 @@ function precedenceComparator(a, b) {
  *        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) {
+function alterSetting(extension, type, key, action) {
   let returnItem;
   let store = getStore(type);
 
   let keyInfo = store.data[type][key];
   if (!keyInfo) {
     if (action === "remove") {
       return null;
     }
@@ -166,17 +166,17 @@ async function alterSetting(extension, t
       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);
+    returnItem = getTopItem(type, key);
   }
 
   if (action === "remove" && keyInfo.precedenceList.length === 0) {
     delete store.data[type][key];
   }
 
   store.saveSoon();
 
@@ -313,17 +313,17 @@ this.ExtensionSettingsStore = {
   /**
    * 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) {
+  getAllForExtension(extension, 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);
       }
@@ -340,16 +340,30 @@ this.ExtensionSettingsStore = {
    *
    * @returns {object} An object with properties for key and value.
    */
   getSetting(type, key) {
     return getTopItem(type, key);
   },
 
   /**
+   * Returns whether an extension currently has a stored setting for a given
+   * key.
+   *
+   * @param {Extension} extension The extension which is being checked.
+   * @param {string} type The type of setting to be checked.
+   * @param {string} key A string that uniquely identifies the setting.
+   *
+   * @returns {boolean} Whether the extension currently has a stored setting.
+   */
+  hasSetting(extension, type, key) {
+    return this.getAllForExtension(extension, type).includes(key);
+  },
+
+  /**
    * Return the levelOfControl for a key / extension combo.
    * levelOfControl is required by Google's ChromeSetting prototype which
    * in turn is used by the privacy API among others.
    *
    * 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
--- a/toolkit/components/extensions/test/xpcshell/test_ext_extensionSettingsStore.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_extensionSettingsStore.js
@@ -92,16 +92,18 @@ add_task(async function test_settings_st
       item,
       itemToAdd,
       "getSetting returns correct item with only one item in the list.");
     levelOfControl = await ExtensionSettingsStore.getLevelOfControl(extensions[extensionIndex], TEST_TYPE, key);
     equal(
       levelOfControl,
       "controlled_by_this_extension",
       "getLevelOfControl returns correct levelOfControl with only one item in the list.");
+    ok(ExtensionSettingsStore.hasSetting(extensions[extensionIndex], TEST_TYPE, key),
+       "hasSetting returns the correct value when an extension has a setting set.");
   }
 
   // Add a setting for the oldest extension.
   for (let key of KEY_LIST) {
     let extensionIndex = 0;
     let itemToAdd = ITEMS[key][extensionIndex];
     let item = await ExtensionSettingsStore.addSetting(
       extensions[extensionIndex], TEST_TYPE, itemToAdd.key, itemToAdd.value, initialValue);
@@ -153,26 +155,24 @@ add_task(async function test_settings_st
     deepEqual(items, KEY_LIST, "getAllForExtension returns expected keys.");
   }
 
   // Attempting to remove a setting that has not been set should *not* throw an exception.
   let removeResult = await ExtensionSettingsStore.removeSetting(extensions[0], "myType", "unset_key");
   equal(removeResult, null, "Removing a setting that was not previously set returns null.");
 
   // 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.");
+  Assert.throws(() => 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.");
+  Assert.throws(() => 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;
   // 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,
@@ -227,16 +227,18 @@ add_task(async function test_settings_st
       item,
       ITEMS[key][2],
       "getSetting returns correct item after a removal.");
     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.");
+    ok(!ExtensionSettingsStore.hasSetting(extensions[extensionIndex], TEST_TYPE, key),
+       "hasSetting returns the correct value when an extension does not have a setting set.");
   }
 
   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],