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 360810 bff7c4b4d2d9e9437085f34d20c344a39280be22
parent 360809 715ae99bc12638aa2b6a9b89325a2d4b2e579bb8
child 360811 81c038f7f40ca05b5329eaa5d7ce05b306934ca0
push id43491
push userbsilverberg@mozilla.com
push dateFri, 26 May 2017 12:25:49 +0000
treeherderautoland@bff7c4b4d2d9 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1367453
milestone55.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 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(