Bug 1474557 - Prevent ExtensionStorageIDB and child/ext-storage from caching a stale or rejected selectBackend promise. r=mixedpuppy
MozReview-Commit-ID: Kgwtm7QXW9o
--- a/toolkit/components/extensions/ExtensionStorageIDB.jsm
+++ b/toolkit/components/extensions/ExtensionStorageIDB.jsm
@@ -367,45 +367,47 @@ this.ExtensionStorageIDB = {
*/
selectBackend(context) {
const {extension} = context;
if (!this.selectedBackendPromises.has(extension)) {
let promise;
if (context.childManager) {
- // Create a promise object that is not tied to the current extension context, because
- // we are caching it for the entire life of the extension in the current process (and
- // the promise returned by context.childManager.callParentAsyncFunction would become
- // a dead object when the context.cloneScope has been destroyed).
- promise = (async () => {
- // Ask the parent process if the new backend is enabled for the
- // running extension.
- let result = await context.childManager.callParentAsyncFunction(
- "storage.local.IDBBackend.selectBackend", []
- );
+ return context.childManager.callParentAsyncFunction(
+ "storage.local.IDBBackend.selectBackend", []
+ ).then(parentResult => {
+ let result;
- if (!result.backendEnabled) {
- return {backendEnabled: false};
+ if (!parentResult.backendEnabled) {
+ result = {backendEnabled: false};
+ } else {
+ result = {
+ ...parentResult,
+ // In the child process, we need to deserialize the storagePrincipal
+ // from the StructuredCloneHolder used to send it across the processes.
+ storagePrincipal: parentResult.storagePrincipal.deserialize(this),
+ };
}
- return {
- ...result,
- // In the child process, we need to deserialize the storagePrincipal
- // from the StructuredCloneHolder used to send it across the processes.
- storagePrincipal: result.storagePrincipal.deserialize(this),
- };
- })();
+ // Cache the result once we know that it has been resolved. The promise returned by
+ // context.childManager.callParentAsyncFunction will be dead when context.cloneScope
+ // is destroyed. To keep a promise alive in the cache, we wrap the result in an
+ // independent promise.
+ this.selectedBackendPromises.set(extension, Promise.resolve(result));
+
+ return result;
+ });
+ }
+
+ // If migrating to the IDB backend is not enabled by the preference, then we
+ // don't need to migrate any data and the new backend is not enabled.
+ if (!this.isBackendEnabled) {
+ promise = Promise.resolve({backendEnabled: false});
} else {
- // If migrating to the IDB backend is not enabled by the preference, then we
- // don't need to migrate any data and the new backend is not enabled.
- if (!this.isBackendEnabled) {
- return Promise.resolve({backendEnabled: false});
- }
-
// In the main process, lazily create a storagePrincipal isolated in a
// reserved user context id (its purpose is ensuring that the IndexedDB storage used
// by the browser.storage.local API is not directly accessible from the extension code).
const storagePrincipal = this.getStoragePrincipal(extension);
// Serialize the nsIPrincipal object into a StructuredCloneHolder related to the privileged
// js global, ready to be sent to the child processes.
const serializedPrincipal = new StructuredCloneHolder(storagePrincipal, this);
--- a/toolkit/components/extensions/child/ext-storage.js
+++ b/toolkit/components/extensions/child/ext-storage.js
@@ -167,17 +167,21 @@ this.storage = class extends ExtensionAP
// Generate the backend-agnostic local API wrapped methods.
const local = {};
for (let method of ["get", "set", "remove", "clear"]) {
local[method] = async function(...args) {
if (!promiseStorageLocalBackend) {
promiseStorageLocalBackend = getStorageLocalBackend();
}
- const backend = await promiseStorageLocalBackend;
+ const backend = await promiseStorageLocalBackend.catch(err => {
+ // Clear the cached promise if it has been rejected.
+ promiseStorageLocalBackend = null;
+ throw err;
+ });
return backend[method](...args);
};
}
return {
storage: {
local,
--- a/toolkit/components/extensions/test/xpcshell/test_ext_storage_tab.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage_tab.js
@@ -95,8 +95,88 @@ add_task(async function test_storage_loc
return runWithPrefs([[ExtensionStorageIDB.BACKEND_ENABLED_PREF, false]],
test_multiple_pages);
});
add_task(async function test_storage_local_idb_backend_from_tab() {
return runWithPrefs([[ExtensionStorageIDB.BACKEND_ENABLED_PREF, true]],
test_multiple_pages);
});
+
+async function test_storage_local_call_from_destroying_context() {
+ let extension = ExtensionTestUtils.loadExtension({
+ async background() {
+ browser.test.onMessage.addListener(async ({msg, values}) => {
+ switch (msg) {
+ case "storage-set": {
+ await browser.storage.local.set(values);
+ browser.test.sendMessage("storage-set:done");
+ break;
+ }
+ case "storage-get": {
+ const res = await browser.storage.local.get();
+ browser.test.sendMessage("storage-get:done", res);
+ break;
+ }
+ default:
+ browser.test.fail(`Received unexpected message: ${msg}`);
+ }
+ });
+
+ browser.test.sendMessage("ext-page-url", browser.runtime.getURL("tab.html"));
+ },
+ files: {
+ "tab.html": `<!DOCTYPE html>
+ <html>
+ <head>
+ <meta charset="utf-8">
+ <script src="tab.js"></script>
+ </head>
+ </html>`,
+
+ "tab.js"() {
+ browser.test.log("Extension tab - calling storage.local API method");
+ // Call the storage.local API from a tab that is going to be quickly closed.
+ browser.storage.local.get({}).then(() => {
+ // This call should never be reached (because the tab should have been
+ // destroyed in the meantime).
+ browser.test.fail("Extension tab - Unexpected storage.local promise resolved");
+ });
+ // Navigate away from the extension page, so that the storage.local API call will be unable
+ // to send the call to the caller context (because it has been destroyed in the meantime).
+ window.location = "about:blank";
+ },
+ },
+ manifest: {
+ permissions: ["storage"],
+ },
+ });
+
+ await extension.startup();
+ const url = await extension.awaitMessage("ext-page-url");
+
+ let contentPage = await ExtensionTestUtils.loadContentPage(url, {extension});
+ let expectedData = {"test-key": "test-value"};
+
+ info("Call storage.local.set from the background page and wait it to be completed");
+ extension.sendMessage({msg: "storage-set", values: expectedData});
+ await extension.awaitMessage("storage-set:done");
+
+ info("Call storage.local.get from the background page and wait it to be completed");
+ extension.sendMessage({msg: "storage-get"});
+ let res = await extension.awaitMessage("storage-get:done");
+
+ Assert.deepEqual(res, expectedData, "Got the expected data set in the storage.local backend");
+
+ contentPage.close();
+
+ await extension.unload();
+}
+
+add_task(async function test_storage_local_file_backend_destroyed_context_promise() {
+ return runWithPrefs([[ExtensionStorageIDB.BACKEND_ENABLED_PREF, false]],
+ test_storage_local_call_from_destroying_context);
+});
+
+add_task(async function test_storage_local_idb_backend_destroyed_context_promise() {
+ return runWithPrefs([[ExtensionStorageIDB.BACKEND_ENABLED_PREF, true]],
+ test_storage_local_call_from_destroying_context);
+});