Bug 1395215: remove asynchrony from addListener, r?kmag draft
authorEthan Glasser-Camp <eglassercamp@mozilla.com>
Tue, 05 Sep 2017 16:11:39 -0400
changeset 660902 8594464094d1570f7b6133b30ded660a257e3ae7
parent 659065 3ecda4678c49ca255c38b1697142b9118cdd27e7
child 730426 4290e4db5d0032d53cd56053c4ac13cf54a4f9f8
push id78602
push usereglassercamp@mozilla.com
push dateThu, 07 Sep 2017 20:48:37 +0000
reviewerskmag
bugs1395215
milestone57.0a1
Bug 1395215: remove asynchrony from addListener, r?kmag MozReview-Commit-ID: HzjfFiIR7hE
toolkit/components/extensions/ExtensionStorageSync.jsm
--- a/toolkit/components/extensions/ExtensionStorageSync.jsm
+++ b/toolkit/components/extensions/ExtensionStorageSync.jsm
@@ -66,22 +66,23 @@ XPCOMUtils.defineLazyPreferenceGetter(th
                                       STORAGE_SYNC_SERVER_URL_PREF,
                                       KINTO_DEFAULT_SERVER_URL);
 XPCOMUtils.defineLazyGetter(this, "WeaveCrypto", function() {
   let {WeaveCrypto} = Cu.import("resource://services-crypto/WeaveCrypto.js", {});
   return new WeaveCrypto();
 });
 
 const {
+  DefaultMap,
   runSafeSyncWithoutClone,
 } = ExtensionUtils;
 
 // Map of Extensions to Set<Contexts> to track contexts that are still
 // "live" and use storage.sync.
-const extensionContexts = new Map();
+const extensionContexts = new DefaultMap(() => new Set());
 // Borrow logger from Sync.
 const log = Log.repository.getLogger("Sync.Engine.Extension-Storage");
 
 // A global that is fxAccounts, or null if (as on android) fxAccounts
 // isn't available.
 let _fxaService = null;
 if (AppConstants.platform != "android") {
   _fxaService = fxAccounts;
@@ -659,19 +660,16 @@ global.CollectionKeyEncryptionRemoteTran
  *
  * @param {Extension} extension
  *                    The extension whose context just ended.
  * @param {Context} context
  *                  The context that just ended.
  */
 function cleanUpForContext(extension, context) {
   const contexts = extensionContexts.get(extension);
-  if (!contexts) {
-    Cu.reportError(new Error(`Internal error: cannot find any contexts for extension ${extension.id}`));
-  }
   contexts.delete(context);
   if (contexts.size === 0) {
     // Nobody else is using this collection. Clean up.
     extensionContexts.delete(extension);
   }
 }
 
 /**
@@ -1080,46 +1078,46 @@ class ExtensionStorageSync {
           // could have uploaded another keyring in the meantime.
           return this.cryptoCollection.sync(this);
         }
       }
       throw e;
     }
   }
 
+  registerInUse(extension, context) {
+    // Register that the extension and context are in use.
+    const contexts = extensionContexts.get(extension);
+    if (!contexts.has(context)) {
+      // New context. Register it and make sure it cleans itself up
+      // when it closes.
+      contexts.add(context);
+      context.callOnClose({
+        close: () => cleanUpForContext(extension, context),
+      });
+    }
+  }
+
   /**
    * Get the collection for an extension, and register the extension
    * as being "in use".
    *
    * @param {Extension} extension
    *                    The extension for which we are seeking
    *                    a collection.
    * @param {Context} context
    *                  The context of the extension, so that we can
    *                  stop syncing the collection when the extension ends.
    * @returns {Promise<Collection>}
    */
   getCollection(extension, context) {
     if (prefPermitsStorageSync !== true) {
       return Promise.reject({message: `Please set ${STORAGE_SYNC_ENABLED_PREF} to true in about:config`});
     }
-    // Register that the extension and context are in use.
-    if (!extensionContexts.has(extension)) {
-      extensionContexts.set(extension, new Set());
-    }
-    const contexts = extensionContexts.get(extension);
-    if (!contexts.has(context)) {
-      // New context. Register it and make sure it cleans itself up
-      // when it closes.
-      contexts.add(context);
-      context.callOnClose({
-        close: () => cleanUpForContext(extension, context),
-      });
-    }
-
+    this.registerInUse(extension, context);
     return openCollection(this.cryptoCollection, extension, context);
   }
 
   async set(extension, items, context) {
     const coll = await this.getCollection(extension, context);
     const keys = Object.keys(items);
     const ids = keys.map(keyToId);
     const histogramSize = this._telemetry.getKeyedHistogramById(HISTOGRAM_SET_OPS_SIZE);
@@ -1218,28 +1216,17 @@ class ExtensionStorageSync {
     return records;
   }
 
   addOnChangedListener(extension, listener, context) {
     let listeners = this.listeners.get(extension) || new Set();
     listeners.add(listener);
     this.listeners.set(extension, listeners);
 
-    // Force opening the collection so that we will sync for this extension.
-    // This happens asynchronously, even though the surface API is synchronous.
-    return this.getCollection(extension, context)
-      .catch((e) => {
-        // We can ignore failures that happen during shutdown here. First, we
-        // can't report in any way. And second, a failure to open the collection
-        // does not matter, because there won't be any message to listen to.
-        // See Bug 1395215.
-        if (!(/Kinto storage adapter connection closing/.test(e.message))) {
-          throw e;
-        }
-      });
+    this.registerInUse(extension, context);
   }
 
   removeOnChangedListener(extension, listener) {
     let listeners = this.listeners.get(extension);
     listeners.delete(listener);
     if (listeners.size == 0) {
       this.listeners.delete(extension);
     }