Bug 1367453 - Do not throw an exception from ExtensionSettingsStore when trying to remove a setting that was not previously set, r=mixedpuppy
authorBob Silverberg <bsilverberg@mozilla.com>
Wed, 24 May 2017 12:29:16 -0400
changeset 585290 bff7c4b4d2d9e9437085f34d20c344a39280be22
parent 585289 715ae99bc12638aa2b6a9b89325a2d4b2e579bb8
child 585291 81c038f7f40ca05b5329eaa5d7ce05b306934ca0
push id61093
push userdgottwald@mozilla.com
push dateFri, 26 May 2017 20:16:26 +0000
reviewersmixedpuppy
bugs1367453
milestone55.0a1
Bug 1367453 - Do not throw an exception from ExtensionSettingsStore when trying to remove a setting that was not previously set, r=mixedpuppy This changes the behaviour of ExtensionSettingsStore so that attempting to remove a setting that was not previously set does not throw an exception. This allows things like privacy.network.webRTCIPHandlingPolicy.clear() to be called without having previously called privacy.network.webRTCIPHandlingPolicy.set(). MozReview-Commit-ID: FFCOFHk5lhb
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
@@ -127,24 +127,30 @@ function precedenceComparator(a, b) {
  *          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) {
+    if (action === "remove") {
+      return null;
+    }
     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) {
+    if (action === "remove") {
+      return null;
+    }
     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;
--- a/toolkit/components/extensions/test/xpcshell/test_ext_extensionSettingsStore.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_extensionSettingsStore.js
@@ -148,22 +148,19 @@ add_task(async function test_settings_st
       "getLevelOfControl returns correct levelOfControl when this extension is in control.");
   }
 
   for (let extension of extensions) {
     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 alter the setting for myType:unset_key as it does not exist/,
-    "removeSetting rejects with an unset key.");
+  // 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.");
 
   // Attempting to enable a setting that has not been set should throw an exception.
@@ -361,23 +358,19 @@ add_task(async function test_settings_st
       null,
       "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 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.");
+    // Attempting to remove a setting that has had all extensions removed should *not* throw an exception.
+    removeResult = await ExtensionSettingsStore.removeSetting(extensions[1], TEST_TYPE, key);
+    equal(removeResult, null, "Removing a setting that has had all extensions removed returns null.");
   }
 
   // Test adding a setting with a value in callbackArgument.
   let extensionIndex = 0;
   let testKey = "callbackArgumentKey";
   let callbackArgumentValue = Date.now();
   // Add the setting.
   let item = await ExtensionSettingsStore.addSetting(