Bug 1476268 - Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort. r=mixedpuppy draft
authorLuca Greco <lgreco@mozilla.com>
Mon, 16 Jul 2018 17:43:57 +0200
changeset 819373 35799c0755aa7089c5f537ef520184d3ff65bda3
parent 819372 7533158d59e148065c90950c6e565b814a97bf8c
push id116537
push userluca.greco@alcacoop.it
push dateTue, 17 Jul 2018 18:45:27 +0000
reviewersmixedpuppy
bugs1476268
milestone63.0a1
Bug 1476268 - Fix uncaught storage.local IDBTransaction error on explicit IDBTransaction abort. r=mixedpuppy MozReview-Commit-ID: F4gA0V0eq8U
toolkit/components/extensions/ExtensionStorageIDB.jsm
toolkit/components/extensions/test/xpcshell/test_ext_storage_tab.js
--- a/toolkit/components/extensions/ExtensionStorageIDB.jsm
+++ b/toolkit/components/extensions/ExtensionStorageIDB.jsm
@@ -199,35 +199,42 @@ class ExtensionStorageLocalIDB extends I
   async set(items, {serialize} = {}) {
     const changes = {};
     let changed = false;
 
     // Explicitly create a transaction, so that we can explicitly abort it
     // as soon as one of the put requests fails.
     const transaction = this.transaction(IDB_DATA_STORENAME, "readwrite");
     const objectStore = transaction.objectStore(IDB_DATA_STORENAME, "readwrite");
+    const transactionCompleted = transaction.promiseComplete();
 
     for (let key of Object.keys(items)) {
       try {
         let oldValue = await objectStore.get(key);
 
         await objectStore.put(items[key], key);
 
         changes[key] = {
           oldValue: oldValue && serialize ? serialize(oldValue) : oldValue,
           newValue: serialize ? serialize(items[key]) : items[key],
         };
         changed = true;
       } catch (err) {
+        transactionCompleted.catch(err => {
+          // We ignore this rejection because we are explicitly aborting the transaction,
+          // the transaction.error will be null, and we throw the original error below.
+        });
         transaction.abort();
 
         throw err;
       }
     }
 
+    await transactionCompleted;
+
     return changed ? changes : null;
   }
 
   /**
    * Asynchronously retrieves the values for the given storage items.
    *
    * @param {Array<string>|object|null} [keysOrItems]
    *        The storage items to get. If an array, the value of each key
--- a/toolkit/components/extensions/test/xpcshell/test_ext_storage_tab.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_storage_tab.js
@@ -194,27 +194,27 @@ add_task(async function test_storage_loc
         <html>
           <head>
             <meta charset="utf-8">
             <script src="tab.js"></script>
           </head>
         </html>`,
 
         "tab.js"() {
-          browser.test.onMessage.addListener(async ({msg, expectErrorOnSet}) => {
+          browser.test.onMessage.addListener(async ({msg, expectErrorOnSet, errorShouldInclude}) => {
             if (msg !== "call-storage-local") {
               return;
             }
 
             const expectedValue = "newvalue";
 
             try {
               await browser.storage.local.set({"newkey": expectedValue});
             } catch (err) {
-              if (expectErrorOnSet) {
+              if (expectErrorOnSet && err.message.includes(errorShouldInclude)) {
                 browser.test.sendMessage("storage-local-set-rejected");
                 return;
               }
 
               browser.test.fail(`Got an unexpected exception on storage.local.get: ${err}`);
               throw err;
             }
 
@@ -245,33 +245,48 @@ add_task(async function test_storage_loc
     const url = await extension.awaitMessage("ext-page-url");
 
     let contentPage = await ExtensionTestUtils.loadContentPage(url, {extension});
     await extension.awaitMessage("extension-tab-ready");
 
     // Turn the low disk mode on (so that opening an IndexedDB connection raises a
     // QuotaExceededError).
     setLowDiskMode(true);
-
-    extension.sendMessage({msg: "call-storage-local", expectErrorOnSet: true});
+    extension.sendMessage({
+      msg: "call-storage-local",
+      expectErrorOnSet: true,
+      errorShouldInclude: "QuotaExceededError",
+    });
     info(`Wait the storage.local.set API call to reject while the disk is full`);
     await extension.awaitMessage("storage-local-set-rejected");
     info("Got the a rejection on storage.local.set while the disk is full as expected");
 
     setLowDiskMode(false);
     extension.sendMessage({msg: "call-storage-local", expectErrorOnSet: false});
     info(`Wait the storage.local API calls to resolve successfully once the disk is free again`);
     await extension.awaitMessage("storage-local-get-resolved");
     info("storage.local.set and storage.local.get resolve successfully once the disk is free again");
 
+    // Turn the low disk mode on again (so that we can trigger an aborted transaction in the
+    // ExtensionStorageIDB set method, now that there is an open IndexedDb connection).
+    setLowDiskMode(true);
+    extension.sendMessage({
+      msg: "call-storage-local",
+      expectErrorOnSet: true,
+      errorShouldInclude: "QuotaExceededError",
+    });
+    info(`Wait the storage.local.set transaction to be aborted while the disk is full`);
+    await extension.awaitMessage("storage-local-set-rejected");
+    info("Got the a rejection on storage.local.set while the disk is full as expected");
+
+    setLowDiskMode(false);
     contentPage.close();
     await extension.unload();
   }
 
   return runWithPrefs([
     [ExtensionStorageIDB.BACKEND_ENABLED_PREF, true],
     // Set the migrated preference for the test extension to prevent the extension
     // from falling back to the JSONFile storage because of an QuotaExceededError
     // raised while migrating to the IndexedDB backend.
     [`${ExtensionStorageIDB.IDB_MIGRATED_PREF_BRANCH}.${EXTENSION_ID}`, true],
   ], test_storage_local_on_idb_disk_full_rejection);
 });
-