Bug 1548700 support selecting what controls an extension setting r=robwu

This adds support for selecting a specific extension to control a setting,
as well as allowing a user to take control without requiring the extension
to be disabled.

Differential Revision: https://phabricator.services.mozilla.com/D52609

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Shane Caraveo 2019-12-02 22:00:34 +00:00
parent 71c9b69680
commit c15c20aa82
5 changed files with 396 additions and 83 deletions

View File

@ -569,7 +569,13 @@ var gSearchPane = {
await Services.search.setDefault(
document.getElementById("defaultEngine").selectedItem.engine
);
ExtensionSettingsStore.setByUser(SEARCH_TYPE, SEARCH_KEY);
if (ExtensionSettingsStore.getSetting(SEARCH_TYPE, SEARCH_KEY) !== null) {
ExtensionSettingsStore.select(
ExtensionSettingsStore.SETTING_USER_SET,
SEARCH_TYPE,
SEARCH_KEY
);
}
},
async setDefaultPrivateEngine() {

View File

@ -173,7 +173,8 @@ function setPrefs(name, setting, item) {
* of the prefs.
*
* @param {string} id
* The id of the extension for which a setting is being modified.
* The id of the extension for which a setting is being modified. Also
* see selectSetting.
* @param {string} name
* The name of the setting being processed.
* @param {string} action
@ -301,6 +302,24 @@ this.ExtensionPreferencesManager = {
return processSetting(id, name, "enable");
},
/**
* Specifically select an extension, the user, or the precedence order that will
* be in control of this setting.
*
* @param {string | null} id
* The id of the extension for which a setting is being selected, or
* ExtensionSettingStore.SETTING_USER_SET (null).
* @param {string} name
* The unique id of the setting.
*
* @returns {Promise}
* Resolves to true if the preferences were changed and to false if
* the preferences were not changed.
*/
selectSetting(id, name) {
return processSetting(id, name, "select");
},
/**
* Indicates that this extension no longer wants to set the given setting.
*

View File

@ -61,6 +61,12 @@ ChromeUtils.defineModuleGetter(
"resource://gre/modules/ExtensionParent.jsm"
);
// Defined for readability of precedence and selection code. keyInfo.selected will be
// one of these defines, or the id of an extension if an extension has been explicitly
// selected.
const SETTING_USER_SET = null;
const SETTING_PRECEDENCE_ORDER = undefined;
const JSON_FILE_NAME = "extension-settings.json";
const JSON_FILE_VERSION = 2;
const STORE_PATH = OS.Path.join(
@ -152,20 +158,28 @@ function getItem(type, key, id) {
return null;
}
// If no id was provided, the selected entry will have precedence.
if (!id && keyInfo.selected) {
id = keyInfo.selected;
}
if (id) {
// Return the item that corresponds to the extension with id of id.
let item = keyInfo.precedenceList.find(item => item.id === id);
return item ? { key, value: item.value, id } : null;
}
// Find the highest precedence, enabled setting.
for (let item of keyInfo.precedenceList) {
if (item.enabled) {
return { key, value: item.value, id: item.id };
// Find the highest precedence, enabled setting, if it has not been
// user set.
if (keyInfo.selected === SETTING_PRECEDENCE_ORDER) {
for (let item of keyInfo.precedenceList) {
if (item.enabled) {
return { key, value: item.value, id: item.id };
}
}
}
// Nothing found in the precedenceList, return the initialValue.
// Nothing found in the precedenceList or the setting is user-set,
// return the initialValue.
return { key, initialValue: keyInfo.initialValue };
}
@ -184,8 +198,9 @@ function precedenceComparator(a, b) {
* Helper method that alters a setting, either by changing its enabled status
* or by removing it.
*
* @param {string} id
* The id of the extension for which a setting is being removed/disabled.
* @param {string|null} id
* The id of the extension for which a setting is being altered, may also
* be SETTING_USER_SET (null).
* @param {string} type
* The type of setting to be altered.
* @param {string} key
@ -200,7 +215,7 @@ function precedenceComparator(a, b) {
* the current top precedent setting has not changed.
*/
function alterSetting(id, type, key, action) {
let returnItem;
let returnItem = null;
ensureType(type);
let keyInfo = _store.data[type][key];
@ -215,27 +230,54 @@ function alterSetting(id, type, key, action) {
let foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
if (foundIndex === -1) {
if (foundIndex === -1 && (action !== "select" || id !== SETTING_USER_SET)) {
if (action === "remove") {
return null;
}
throw new Error(
`Cannot alter the setting for ${type}:${key} as it does not exist.`
`Cannot alter the setting for ${type}:${key} as ${id} does not exist.`
);
}
let selected = keyInfo.selected;
switch (action) {
case "select":
if (foundIndex >= 0 && !keyInfo.precedenceList[foundIndex].enabled) {
throw new Error(
`Cannot select the setting for ${type}:${key} as ${id} is disabled.`
);
}
keyInfo.selected = id;
keyInfo.selectedDate = Date.now();
break;
case "remove":
// Removing a user-set setting reverts to precedence order.
if (id === keyInfo.selected) {
keyInfo.selected = SETTING_PRECEDENCE_ORDER;
delete keyInfo.selectedDate;
}
keyInfo.precedenceList.splice(foundIndex, 1);
break;
case "enable":
keyInfo.precedenceList[foundIndex].enabled = true;
keyInfo.precedenceList.sort(precedenceComparator);
// Enabling a setting does not change a user-set setting, so we
// save and bail early.
if (keyInfo.selected !== SETTING_PRECEDENCE_ORDER) {
_store.saveSoon();
return null;
}
foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
break;
case "disable":
// Disabling a user-set setting reverts to precedence order.
if (keyInfo.selected === id) {
keyInfo.selected = SETTING_PRECEDENCE_ORDER;
delete keyInfo.selectedDate;
}
keyInfo.precedenceList[foundIndex].enabled = false;
keyInfo.precedenceList.sort(precedenceComparator);
break;
@ -244,7 +286,7 @@ function alterSetting(id, type, key, action) {
throw new Error(`${action} is not a valid action for alterSetting.`);
}
if (foundIndex === 0) {
if (selected !== keyInfo.selected || foundIndex === 0) {
returnItem = getItem(type, key);
}
@ -264,6 +306,8 @@ function alterSetting(id, type, key, action) {
}
var ExtensionSettingsStore = {
SETTING_USER_SET,
/**
* Loads the JSON file for the SettingsStore into memory.
* The promise this returns must be resolved before asking the SettingsStore
@ -277,8 +321,7 @@ var ExtensionSettingsStore = {
},
/**
* Adds a setting to the store, possibly returning the current top precedent
* setting.
* Adds a setting to the store, returning the new setting if it changes.
*
* @param {string} id
* The id of the extension for which a setting is being added.
@ -302,7 +345,7 @@ var ExtensionSettingsStore = {
* value, which corresponds to the item that was
* just added, or null if the item that was just
* added does not need to be set because it is not
* at the top of the precedence list.
* selected or at the top of the precedence list.
*/
async addSetting(
id,
@ -334,6 +377,7 @@ var ExtensionSettingsStore = {
// Check for this item in the precedenceList.
let foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
let newInstall = false;
if (foundIndex === -1) {
// No item for this extension, so add a new one.
let addon = await AddonManager.getAddonByID(id);
@ -343,6 +387,7 @@ var ExtensionSettingsStore = {
value,
enabled: true,
});
newInstall = addon.installDate.valueOf() > keyInfo.selectedDate;
} else {
// Item already exists or this extension, so update it.
let item = keyInfo.precedenceList[foundIndex];
@ -353,19 +398,29 @@ var ExtensionSettingsStore = {
// Sort the list.
keyInfo.precedenceList.sort(precedenceComparator);
foundIndex = keyInfo.precedenceList.findIndex(item => item.id == id);
// If our new setting is top of precedence, then reset the selected entry.
if (foundIndex === 0 && newInstall) {
keyInfo.selected = SETTING_PRECEDENCE_ORDER;
delete keyInfo.selectedDate;
}
_store.saveSoon();
// Check whether this is currently the top item.
if (keyInfo.precedenceList[0].id == id) {
// Check whether this is currently selected item if one is
// selected, otherwise the top item has precedence.
if (
keyInfo.selected !== SETTING_USER_SET &&
(keyInfo.selected === id || foundIndex === 0)
) {
return { id, key, value };
}
return null;
},
/**
* Removes a setting from the store, possibly returning the current top
* precedent setting.
* Removes a setting from the store, returning the new setting if it changes.
*
* @param {string} id
* The id of the extension for which a setting is being removed.
@ -375,17 +430,14 @@ var ExtensionSettingsStore = {
* 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.
* Either an object with properties for key and value if the setting changes, or null.
*/
removeSetting(id, type, key) {
return alterSetting(id, type, key, "remove");
},
/**
* Enables a setting in the store, possibly returning the current top
* precedent setting.
* Enables a setting in the store, returning the new setting if it changes.
*
* @param {string} id
* The id of the extension for which a setting is being enabled.
@ -395,17 +447,14 @@ var ExtensionSettingsStore = {
* 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.
* Either an object with properties for key and value if the setting changes, or null.
*/
enable(id, type, key) {
return alterSetting(id, type, key, "enable");
},
/**
* Disables a setting in the store, possibly returning the current top
* precedent setting.
* Disables a setting in the store, returning the new setting if it changes.
*
* @param {string} id
* The id of the extension for which a setting is being disabled.
@ -415,39 +464,36 @@ var ExtensionSettingsStore = {
* 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.
* Either an object with properties for key and value if the setting changes, or null.
*/
disable(id, type, key) {
return alterSetting(id, type, key, "disable");
},
/**
* Mark a setting as being controlled by a user's choice. This will disable all of
* the extension defined values for the extension.
* Specifically select an extension, or no extension, that will be in control of
* this setting.
*
* @param {string} type The type of the setting.
* @param {string} key The key of the setting.
* To select a specific extension that controls this setting, pass the extension id.
*
* To select as user-set pass SETTING_USER_SET as the id. In this case, no extension
* will have control of the setting.
*
* Once a specific selection is made, precedence order will not be used again unless the selected
* extension is disabled, removed, or a new extension takes control of the setting.
*
* @param {string | null} id
* The id of the extension being selected or SETTING_USER_SET (null).
* @param {string} type
* The type of setting to be selected.
* @param {string} key
* A string that uniquely identifies the setting.
*
* @returns {object | null}
* Either an object with properties for key and value if the setting changes, or null.
*/
setByUser(type, key) {
let { precedenceList } =
(_store.data[type] && _store.data[type][key]) || {};
if (!precedenceList) {
// The setting for this key does not exist. Nothing to do.
return;
}
for (let item of precedenceList) {
item.enabled = false;
}
ExtensionParent.apiManager.emit("extension-setting-changed", {
action: "disable",
type,
key,
});
_store.saveSoon();
select(id, type, key) {
return alterSetting(id, type, key, "select");
},
/**
@ -535,6 +581,18 @@ var ExtensionSettingsStore = {
return "controllable_by_this_extension";
}
if (keyInfo.selected !== SETTING_PRECEDENCE_ORDER) {
if (id === keyInfo.selected) {
return "controlled_by_this_extension";
}
// When user set, the setting is never "controllable" unless the installDate
// is later than the user date.
let addon = await AddonManager.getAddonByID(id);
return keyInfo.selectedDate > addon.installDate.valueOf()
? "not_controllable"
: "controllable_by_this_extension";
}
let enabledItems = keyInfo.precedenceList.filter(item => item.enabled);
if (!enabledItems.length) {
return "controllable_by_this_extension";

View File

@ -694,3 +694,178 @@ add_task(async function test_preference_default_upgraded() {
await extension.unload();
await promiseShutdownManager();
});
add_task(async function test_preference_select() {
await promiseStartupManager();
let extensionData = {
useAddonManager: "temporary",
manifest: {
applications: { gecko: { id: "@one" } },
},
};
let one = ExtensionTestUtils.loadExtension(extensionData);
await one.startup();
// We set the default value for a pref here so it will be
// picked up by EPM.
let defaultPrefs = Services.prefs.getDefaultBranch(null);
defaultPrefs.setStringPref("bar", "initial default");
await ExtensionSettingsStore.initialize();
ExtensionPreferencesManager.addSetting("some-pref", {
prefNames: ["bar"],
setCallback(value) {
return { [this.prefNames[0]]: value };
},
});
ok(
await ExtensionPreferencesManager.setSetting(
one.id,
"some-pref",
"new value"
),
"setting was changed"
);
let item = await ExtensionPreferencesManager.getSetting("some-pref");
equal(item.value, "new value", "The value is set");
// User-set the setting.
await ExtensionPreferencesManager.selectSetting(null, "some-pref");
item = await ExtensionPreferencesManager.getSetting("some-pref");
deepEqual(
item,
{ key: "some-pref", initialValue: {} },
"The value is user-set"
);
// Extensions installed before cannot gain control again.
let levelOfControl = await ExtensionPreferencesManager.getLevelOfControl(
one.id,
"some-pref"
);
equal(
levelOfControl,
"not_controllable",
"getLevelOfControl returns correct levelOfControl when user-set."
);
// Enabling the top-precedence addon does not take over a user-set setting.
await ExtensionPreferencesManager.disableSetting(one.id, "some-pref");
await ExtensionPreferencesManager.enableSetting(one.id, "some-pref");
item = await ExtensionPreferencesManager.getSetting("some-pref");
deepEqual(
item,
{ key: "some-pref", initialValue: {} },
"The value is user-set"
);
// Upgrading does not override the user-set setting.
extensionData.manifest.version = "2.0";
extensionData.manifest.incognito = "not_allowed";
await one.upgrade(extensionData);
levelOfControl = await ExtensionPreferencesManager.getLevelOfControl(
one.id,
"some-pref"
);
equal(
levelOfControl,
"not_controllable",
"getLevelOfControl returns correct levelOfControl when user-set after addon upgrade."
);
// We can re-select the extension.
await ExtensionPreferencesManager.selectSetting(one.id, "some-pref");
item = await ExtensionPreferencesManager.getSetting("some-pref");
deepEqual(item.value, "new value", "The value is extension set");
// An extension installed after user-set can take over the setting.
await ExtensionPreferencesManager.selectSetting(null, "some-pref");
item = await ExtensionPreferencesManager.getSetting("some-pref");
deepEqual(
item,
{ key: "some-pref", initialValue: {} },
"The value is user-set"
);
let two = ExtensionTestUtils.loadExtension({
useAddonManager: "temporary",
manifest: {
applications: { gecko: { id: "@two" } },
},
});
await two.startup();
levelOfControl = await ExtensionPreferencesManager.getLevelOfControl(
two.id,
"some-pref"
);
equal(
levelOfControl,
"controllable_by_this_extension",
"getLevelOfControl returns correct levelOfControl when user-set after addon install."
);
await ExtensionPreferencesManager.setSetting(
two.id,
"some-pref",
"another value"
);
item = ExtensionSettingsStore.getSetting("prefs", "some-pref");
equal(item.value, "another value", "The value is set");
// A new installed extension can override a user selected extension.
let three = ExtensionTestUtils.loadExtension({
useAddonManager: "temporary",
manifest: {
applications: { gecko: { id: "@three" } },
},
});
// user selects specific extension to take control
await ExtensionPreferencesManager.selectSetting(one.id, "some-pref");
// two cannot control
levelOfControl = await ExtensionPreferencesManager.getLevelOfControl(
two.id,
"some-pref"
);
equal(
levelOfControl,
"not_controllable",
"getLevelOfControl returns correct levelOfControl when user-set after addon install."
);
// three can control after install
await three.startup();
levelOfControl = await ExtensionPreferencesManager.getLevelOfControl(
three.id,
"some-pref"
);
equal(
levelOfControl,
"controllable_by_this_extension",
"getLevelOfControl returns correct levelOfControl when user-set after addon install."
);
await ExtensionPreferencesManager.setSetting(
three.id,
"some-pref",
"third value"
);
item = ExtensionSettingsStore.getSetting("prefs", "some-pref");
equal(item.value, "third value", "The value is set");
// We have returned to precedence based settings.
await ExtensionPreferencesManager.removeSetting(three.id, "some-pref");
await ExtensionPreferencesManager.removeSetting(two.id, "some-pref");
item = await ExtensionPreferencesManager.getSetting("some-pref");
equal(item.value, "new value", "The value is extension set");
await one.unload();
await two.unload();
await three.unload();
await promiseShutdownManager();
});

View File

@ -732,12 +732,6 @@ add_task(async function test_settings_store_setByUser() {
applications: { gecko: { id: "@second" } },
},
}),
ExtensionTestUtils.loadExtension({
useAddonManager: "temporary",
manifest: {
applications: { gecko: { id: "@third" } },
},
}),
];
let type = "some_type";
@ -749,7 +743,7 @@ add_task(async function test_settings_store_setByUser() {
// Create an array actual Extension objects which correspond to the
// test framework extension wrappers.
let [one, two, three] = testExtensions.map(extension => extension.extension);
let [one, two] = testExtensions.map(extension => extension.extension);
let initialCallback = () => "initial";
// Initialize the SettingsStore.
@ -787,6 +781,27 @@ add_task(async function test_settings_store_setByUser() {
"addSetting returns the second set item"
);
// a user-set selection reverts to precedence order when new
// extension sets the setting.
ExtensionSettingsStore.select(
ExtensionSettingsStore.SETTING_USER_SET,
type,
key
);
deepEqual(
{ key, initialValue: "initial" },
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the initial value after being set by user"
);
let three = ExtensionTestUtils.loadExtension({
useAddonManager: "temporary",
manifest: {
applications: { gecko: { id: "@third" } },
},
});
await three.startup();
item = await ExtensionSettingsStore.addSetting(
three.id,
type,
@ -799,46 +814,86 @@ add_task(async function test_settings_store_setByUser() {
item,
"addSetting returns the third set item"
);
deepEqual(
item,
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the third set item"
);
ExtensionSettingsStore.setByUser(type, key);
ExtensionSettingsStore.select(
ExtensionSettingsStore.SETTING_USER_SET,
type,
key
);
deepEqual(
{ key, initialValue: "initial" },
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the initial value after being set by user"
);
item = ExtensionSettingsStore.enable(one.id, type, key);
item = ExtensionSettingsStore.select(one.id, type, key);
deepEqual(
{ key, value: "one", id: one.id },
item,
"enable returns the first set item after enable"
"selecting an extension returns the first set item after enable"
);
// Disabling a selected item returns to precedence order
ExtensionSettingsStore.disable(one.id, type, key);
deepEqual(
{ key, value: "three", id: three.id },
ExtensionSettingsStore.getSetting(type, key),
"returning to precedence order sets the third set item"
);
// Test that disabling all then enabling one does not take over a user-set setting.
ExtensionSettingsStore.select(
ExtensionSettingsStore.SETTING_USER_SET,
type,
key
);
deepEqual(
{ key, initialValue: "initial" },
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the initial value after being set by user"
);
ExtensionSettingsStore.disable(three.id, type, key);
ExtensionSettingsStore.disable(two.id, type, key);
deepEqual(
{ key, initialValue: "initial" },
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the initial value after disabling all extensions"
);
ExtensionSettingsStore.enable(three.id, type, key);
deepEqual(
{ key, initialValue: "initial" },
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the initial value after enabling one extension"
);
// Ensure that calling addSetting again will not reset a user-set value when
// the extension install date is older than the user-set date.
item = await ExtensionSettingsStore.addSetting(
three.id,
type,
key,
"three",
initialCallback
);
deepEqual(
{ key, initialValue: "initial" },
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the initial value after calling addSetting for old addon"
);
item = ExtensionSettingsStore.enable(three.id, type, key);
equal(undefined, item, "enabling the active item does not return an item");
deepEqual(
{ key, value: "three", id: three.id },
item,
"enable returns the third set item after enable"
);
item = ExtensionSettingsStore.enable(two.id, type, key);
deepEqual(
undefined,
item,
"enable returns undefined after enabling the second item"
);
item = ExtensionSettingsStore.getSetting(type, key);
deepEqual(
{ key, value: "three", id: three.id },
item,
"getSetting returns the third set item after enabling the second item"
{ key, initialValue: "initial" },
ExtensionSettingsStore.getSetting(type, key),
"getSetting returns the initial value after enabling one extension"
);
ExtensionSettingsStore.removeSetting(three.id, type, key);