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
--- 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(