Bug 1474557 - Fix storage.local JSONFile backend unable to store data successfully from a destroying context. r?mixedpuppy
MozReview-Commit-ID: 3aQGrkD6Gfd
--- a/toolkit/components/extensions/ExtensionStorageIDB.jsm
+++ b/toolkit/components/extensions/ExtensionStorageIDB.jsm
@@ -363,16 +363,25 @@ this.ExtensionStorageIDB = {
* the IDB backend and the object also includes a `storagePrincipal` property
* of type nsIPrincipal, otherwise `backendEnabled` will be false when the
* extension should use the old JSONFile backend (e.g. because the IDB backend has
* not been enabled from the preference).
*/
selectBackend(context) {
const {extension} = context;
+ // If 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) {
+ // Resolve immediately, so that the storage.local.set has a chance to be completed even
+ // when called from a context that is destroyed immediately after a storage.local.set
+ // API call.
+ return Promise.resolve({backendEnabled: false});
+ }
+
if (!this.selectedBackendPromises.has(extension)) {
let promise;
if (context.childManager) {
return context.childManager.callParentAsyncFunction(
"storage.local.IDBBackend.selectBackend", []
).then(parentResult => {
let result;
@@ -393,22 +402,16 @@ this.ExtensionStorageIDB = {
// 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) {
- 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/test/xpcshell/test_ext_storage_tab.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage_tab.js
@@ -130,17 +130,17 @@ async function test_storage_local_call_f
<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(() => {
+ browser.storage.local.set({"test-key": "tab-set-value"}).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");
});
},
},
manifest: {
permissions: ["storage"],
@@ -151,27 +151,41 @@ async function test_storage_local_call_f
const url = await extension.awaitMessage("ext-page-url");
let contentPage = await ExtensionTestUtils.loadContentPage(url, {extension});
// Close the content 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).
contentPage.close();
- let expectedData = {"test-key": "test-value"};
+
+ let expectedTabSetData = {"test-key": "tab-set-value"};
+ let expectedData = {"test-key": "bg-set-value"};
+
+ // The JSONFile backend used to be able to store values in the file backend from a
+ // context that is destroyed immediately after a storage.local.set API call,
+ // this block make some additional assertions to ensure that is still working
+ // as it used to (unfortunately if doesn't currently work with the IndexedDB backend
+ // because we currently need to ask the main process to try to migrate the extension
+ // data and then return the selected backend).
+ if (!Services.prefs.getBoolPref(ExtensionStorageIDB.BACKEND_ENABLED_PREF, false)) {
+ 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, expectedTabSetData, "Got the expected data set from the extension tab");
+ }
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");
+ let res2 = await extension.awaitMessage("storage-get:done");
+ Assert.deepEqual(res2, expectedData, "Got the expected data set from the background page");
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);
});