Bug 1395215: remove asynchrony from addListener, r=kmag
authorEthan Glasser-Camp <eglassercamp@mozilla.com>
Tue, 05 Sep 2017 16:11:39 -0400
changeset 429195 192a10e664c726add22ea36d1b050bc6daf81f54
parent 429194 a02bd6e1c55e3ef0575427831ad655ff7656ee62
child 429196 1fe86d8df2cd7172c66b0506bf0b0e789e86cf01
push id7761
push userjlund@mozilla.com
push dateFri, 15 Sep 2017 00:19:52 +0000
treeherdermozilla-beta@c38455951db4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskmag
bugs1395215
milestone57.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
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);
     }