author | Rob Wu <rob@robwu.nl> |
Tue, 12 Feb 2019 14:04:39 +0000 | |
changeset 459067 | 5542b48b6dfe |
parent 459066 | 84d60f483f42 |
child 459068 | 788e4338d31f |
push id | 111913 |
push user | shindli@mozilla.com |
push date | Thu, 14 Feb 2019 05:01:59 +0000 |
treeherder | mozilla-inbound@a0752d7e8073 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | rpl |
bugs | 1522918 |
milestone | 67.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
|
--- a/toolkit/components/extensions/ExtensionParent.jsm +++ b/toolkit/components/extensions/ExtensionParent.jsm @@ -1776,18 +1776,19 @@ class CacheStore { let [store] = await this.getStore(); return new Map(store); } async delete(path) { let [store, key] = await this.getStore(path); - store.delete(key); - StartupCache.save(); + if (store.delete(key)) { + StartupCache.save(); + } } } for (let name of StartupCache.STORE_NAMES) { StartupCache[name] = new CacheStore(name); } var ExtensionParent = {
--- a/toolkit/components/extensions/ExtensionPermissions.jsm +++ b/toolkit/components/extensions/ExtensionPermissions.jsm @@ -58,27 +58,36 @@ var ExtensionPermissions = { }, async _get(extensionId) { await lazyInit(); let perms = prefs.data[extensionId]; if (!perms) { perms = emptyPermissions(); - prefs.data[extensionId] = perms; } return perms; }, async _getCached(extensionId) { return StartupCache.permissions.get(extensionId, () => this._get(extensionId)); }, + /** + * Retrieves the optional permissions for the given extension. + * The information may be retrieved from the StartupCache, and otherwise fall + * back to data from the disk (and cache the result in the StartupCache). + * + * @param {string} extensionId The extensionId + * @returns {object} An object with "permissions" and "origins" array. + * The object may be a direct reference to the storage or cache, so its + * value should immediately be used and not be modified by callers. + */ get(extensionId) { return this._getCached(extensionId); }, /** * Add new permissions for the given extension. `permissions` is * in the format that is passed to browser.permissions.request(). * @@ -149,20 +158,20 @@ var ExtensionPermissions = { this._saveSoon(extensionId); if (emitter) { emitter.emit("remove-permissions", removed); } } }, async removeAll(extensionId) { - let perms = await this._getCached(extensionId); - - if (perms.permissions.length || perms.origins.length) { - Object.assign(perms, emptyPermissions()); + await lazyInit(); + StartupCache.permissions.delete(extensionId); + if (prefs.data[extensionId]) { + delete prefs.data[extensionId]; prefs.saveSoon(); } }, // This is meant for tests only async _uninit() { if (!_initPromise) { return;
--- a/toolkit/components/extensions/test/xpcshell/test_ext_permissions_uninstall.js +++ b/toolkit/components/extensions/test/xpcshell/test_ext_permissions_uninstall.js @@ -12,16 +12,50 @@ const observer = { observe(subject, topic, data) { if (topic == "webextension-optional-permission-prompt") { let {resolve} = subject.wrappedJSObject; resolve(true); } }, }; +// Look up the cached permissions, if any. +async function getCachedPermissions(extensionId) { + const NotFound = Symbol("extension ID not found in permissions cache"); + try { + return await ExtensionParent.StartupCache.permissions.get(extensionId, () => { + // Throw error to prevent the key from being created. + throw NotFound; + }); + } catch (e) { + if (e === NotFound) { + return null; + } + throw e; + } +} + +// Look up the permissions from the file. Internal methods are used to avoid +// inadvertently changing the permissions in the cache or JSON file. +async function getStoredPermissions(extensionId) { + // The two _get calls that follow are expected to return the same object if + // the entry exists in the JSON file, otherwise we expect two different + // objects (with the same default properties). + let perms1 = await ExtensionPermissions._get(extensionId); + let perms2 = await ExtensionPermissions._get(extensionId); + if (perms1 === perms2) { + // There is an entry in the file. + return perms1; + } + // Sanity check: The returned object should be empty. + Assert.deepEqual(perms1, perms2, "Expected same permission values"); + Assert.deepEqual(perms1, {origins: [], permissions: []}, "Expected empty permissions"); + return null; +} + add_task(async function setup() { Services.prefs.setBoolPref("extensions.webextOptionalPermissionPrompts", true); Services.obs.addObserver(observer, "webextension-optional-permission-prompt"); await AddonTestUtils.promiseStartupManager(); registerCleanupFunction(async () => { await AddonTestUtils.promiseShutdownManager(); Services.obs.removeObserver(observer, "webextension-optional-permission-prompt"); Services.prefs.clearUserPref("extensions.webextOptionalPermissionPrompts"); @@ -57,14 +91,36 @@ add_task(async function test_permissions let result = await extension.awaitMessage("request.result"); equal(result, true, "request() for optional permissions succeeded"); }); let id = extension.id; let perms = await ExtensionPermissions.get(id); equal(perms.permissions.length, 1, "optional permission added"); + Assert.deepEqual(await getCachedPermissions(id), { + permissions: ["idle"], + origins: [], + }, "Optional permission added to cache"); + Assert.deepEqual(await getStoredPermissions(id), { + permissions: ["idle"], + origins: [], + }, "Optional permission added to persistent file"); + await extension.unload(); + // Directly read from the internals instead of using ExtensionPermissions.get, + // because the latter will lazily cache the extension ID. + Assert.deepEqual(await getCachedPermissions(id), null, "Cached permissions removed"); + Assert.deepEqual(await getStoredPermissions(id), null, "Stored permissions removed"); + perms = await ExtensionPermissions.get(id); equal(perms.permissions.length, 0, "no permissions after uninstall"); equal(perms.origins.length, 0, "no origin permissions after uninstall"); + + // The public ExtensionPermissions.get method should not store (empty) + // permissions in the persistent JSON file. Polluting the cache is not ideal, + // but acceptable since the cache will eventually be cleared, and non-test + // code is not likely to call ExtensionPermissions.get() for non-installed + // extensions anyway. + Assert.deepEqual(await getCachedPermissions(id), perms, "Permissions cached"); + Assert.deepEqual(await getStoredPermissions(id), null, "Permissions not saved"); });