Bug 1203330 Part 1 Fix SingletonEventManager r=kmag

This patch adds the ability to run SingletonEventManager handlers in
different modes: sync, async, raw (no exception handling, arg cloning,
or asynchrony), or asyncWithoutClone. When you call the handler,
you're required to specify which variant you want.

Existing uses of SingletonEventManager are all converted to async calls.
Note that some of them were previously synchronous, but it didn't appear
to be necessary.

Also added a callOnClose for SingletonEventManager when the last listener
is removed.

MozReview-Commit-ID: ATHO97dWf3X

--HG--
extra : rebase_source : bf02d79e3fbab84892be8a7e52ea7a1caf2e003d

browser/components/extensions/ext-bookmarks.js

@@ -320,7 +320,7 @@ extensions.registerSchemaAPI("bookmarks", "addon_parent", context => {
 
       onCreated: new SingletonEventManager(context, "bookmarks.onCreated", fire => {
         let listener = (event, bookmark) => {
-          context.runSafe(fire, bookmark.id, bookmark);
+          fire.sync(bookmark.id, bookmark);
         };
 
         observer.on("created", listener);
@@ -333,7 +333,7 @@ extensions.registerSchemaAPI("bookmarks", "addon_parent", context => {
 
       onRemoved: new SingletonEventManager(context, "bookmarks.onRemoved", fire => {
         let listener = (event, data) => {
-          context.runSafe(fire, data.guid, data.info);
+          fire.sync(data.guid, data.info);
         };
 
         observer.on("removed", listener);
@@ -346,7 +346,7 @@ extensions.registerSchemaAPI("bookmarks", "addon_parent", context => {
 
       onChanged: new SingletonEventManager(context, "bookmarks.onChanged", fire => {
         let listener = (event, data) => {
-          context.runSafe(fire, data.guid, data.info);
+          fire.sync(data.guid, data.info);
         };
 
         observer.on("changed", listener);
@@ -359,7 +359,7 @@ extensions.registerSchemaAPI("bookmarks", "addon_parent", context => {
 
       onMoved: new SingletonEventManager(context, "bookmarks.onMoved", fire => {
         let listener = (event, data) => {
-          context.runSafe(fire, data.guid, data.info);
+          fire.sync(data.guid, data.info);
         };
 
         observer.on("moved", listener);

browser/components/extensions/ext-c-omnibox.js

@@ -5,7 +5,6 @@
 Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 
 var {
-  runSafeSyncWithoutClone,
   SingletonEventManager,
 } = ExtensionUtils;
 
@@ -14,7 +13,7 @@ extensions.registerSchemaAPI("omnibox", "addon_child", context => {
     omnibox: {
       onInputChanged: new SingletonEventManager(context, "omnibox.onInputChanged", fire => {
         let listener = (text, id) => {
-          runSafeSyncWithoutClone(fire, text, suggestions => {
+          fire.asyncWithoutClone(text, suggestions => {
             // TODO: Switch to using callParentFunctionNoReturn once bug 1314903 is fixed.
             context.childManager.callParentAsyncFunction("omnibox_internal.addSuggestions", [
               id,

browser/components/extensions/ext-history.js

@@ -222,7 +222,7 @@ extensions.registerSchemaAPI("history", "addon_parent", context => {
 
       onVisited: new SingletonEventManager(context, "history.onVisited", fire => {
         let listener = (event, data) => {
-          context.runSafe(fire, data);
+          fire.sync(data);
         };
 
         getObserver().on("visited", listener);
@@ -233,7 +233,7 @@ extensions.registerSchemaAPI("history", "addon_parent", context => {
 
       onVisitRemoved: new SingletonEventManager(context, "history.onVisitRemoved", fire => {
         let listener = (event, data) => {
-          context.runSafe(fire, data);
+          fire.sync(data);
         };
 
         getObserver().on("visitRemoved", listener);

browser/components/extensions/ext-omnibox.js

@@ -50,7 +50,7 @@ extensions.registerSchemaAPI("omnibox", "addon_parent", context => {
 
       onInputStarted: new SingletonEventManager(context, "omnibox.onInputStarted", fire => {
         let listener = (eventName) => {
-          fire();
+          fire.sync();
         };
         extension.on(ExtensionSearchHandler.MSG_INPUT_STARTED, listener);
         return () => {
@@ -60,7 +60,7 @@ extensions.registerSchemaAPI("omnibox", "addon_parent", context => {
 
       onInputCancelled: new SingletonEventManager(context, "omnibox.onInputCancelled", fire => {
         let listener = (eventName) => {
-          fire();
+          fire.sync();
         };
         extension.on(ExtensionSearchHandler.MSG_INPUT_CANCELLED, listener);
         return () => {
@@ -70,7 +70,7 @@ extensions.registerSchemaAPI("omnibox", "addon_parent", context => {
 
       onInputEntered: new SingletonEventManager(context, "omnibox.onInputEntered", fire => {
         let listener = (eventName, text, disposition) => {
-          fire(text, disposition);
+          fire.sync(text, disposition);
         };
         extension.on(ExtensionSearchHandler.MSG_INPUT_ENTERED, listener);
         return () => {
@@ -92,7 +92,7 @@ extensions.registerSchemaAPI("omnibox", "addon_parent", context => {
 
       onInputChanged: new SingletonEventManager(context, "omnibox_internal.onInputChanged", fire => {
         let listener = (eventName, text, id) => {
-          fire(text, id);
+          fire.sync(text, id);
         };
         extension.on(ExtensionSearchHandler.MSG_INPUT_CHANGED, listener);
         return () => {

browser/components/extensions/ext-sessions.js

@@ -94,7 +94,7 @@ extensions.registerSchemaAPI("sessions", "addon_parent", context => {
 
       onChanged: new SingletonEventManager(context, "sessions.onChanged", fire => {
         let observer = () => {
-          context.runSafe(fire);
+          fire.async();
         };
 
         Services.obs.addObserver(observer, SS_ON_CLOSED_OBJECTS_CHANGED, false);

mobile/android/components/extensions/ext-pageAction.js

@@ -132,7 +132,7 @@ extensions.registerSchemaAPI("pageAction", "addon_parent", context => {
     pageAction: {
       onClicked: new SingletonEventManager(context, "pageAction.onClicked", fire => {
         let listener = (event) => {
-          fire();
+          fire.async();
         };
         pageActionMap.get(extension).on("click", listener);
         return () => {

toolkit/components/extensions/ExtensionChild.jsm

@@ -330,7 +330,7 @@ class Messenger {
   }
 
   onMessage(name) {
-    return new SingletonEventManager(this.context, name, callback => {
+    return new SingletonEventManager(this.context, name, fire => {
       let listener = {
         messageFilterPermissive: this.optionalFilter,
         messageFilterStrict: this.filter,
@@ -360,7 +360,7 @@ class Messenger {
 
           // Note: We intentionally do not use runSafe here so that any
           // errors are propagated to the message sender.
-          let result = callback(message, sender, sendResponse);
+          let result = fire.raw(message, sender, sendResponse);
           if (result instanceof this.context.cloneScope.Promise) {
             return result;
           } else if (result === true) {
@@ -412,7 +412,7 @@ class Messenger {
   }
 
   onConnect(name) {
-    return new SingletonEventManager(this.context, name, callback => {
+    return new SingletonEventManager(this.context, name, fire => {
       let listener = {
         messageFilterPermissive: this.optionalFilter,
         messageFilterStrict: this.filter,
@@ -431,7 +431,7 @@ class Messenger {
             delete recipient.tab;
           }
           let port = new Port(this.context, mm, this.messageManagers, name, portId, sender, recipient);
-          this.context.runSafeWithoutClone(callback, port.api());
+          fire.asyncWithoutClone(port.api());
           return true;
         },
       };

toolkit/components/extensions/ExtensionUtils.jsm

@@ -700,15 +700,51 @@ function SingletonEventManager(context, name, register) {
 
 SingletonEventManager.prototype = {
   addListener(callback, ...args) {
-    let wrappedCallback = (...args) => {
+    if (this.unregister.has(callback)) {
+      return;
+    }
+
+    let shouldFire = () => {
       if (this.context.unloaded) {
         dump(`${this.name} event fired after context unloaded.\n`);
+      } else if (!this.context.active) {
+        dump(`${this.name} event fired while context is inactive.\n`);
       } else if (this.unregister.has(callback)) {
-        return callback(...args);
+        return true;
       }
+      return false;
     };
 
-    let unregister = this.register(wrappedCallback, ...args);
+    let fire = {
+      sync: (...args) => {
+        if (shouldFire()) {
+          return this.context.runSafe(callback, ...args);
+        }
+      },
+      async: (...args) => {
+        return Promise.resolve().then(() => {
+          if (shouldFire()) {
+            return this.context.runSafe(callback, ...args);
+          }
+        });
+      },
+      raw: (...args) => {
+        if (!shouldFire()) {
+          throw new Error("Called raw() on unloaded/inactive context");
+        }
+        return callback(...args);
+      },
+      asyncWithoutClone: (...args) => {
+        return Promise.resolve().then(() => {
+          if (shouldFire()) {
+            return this.context.runSafeWithoutClone(callback, ...args);
+          }
+        });
+      },
+    };
+
+
+    let unregister = this.register(fire, ...args);
     this.unregister.set(callback, unregister);
     this.context.callOnClose(this);
   },
@@ -721,6 +757,9 @@ SingletonEventManager.prototype = {
     let unregister = this.unregister.get(callback);
     this.unregister.delete(callback);
     unregister();
+    if (this.unregister.size == 0) {
+      this.context.forgetOnClose(this);
+    }
   },
 
   hasListener(callback) {

toolkit/components/extensions/ext-c-test.js

@@ -171,7 +171,7 @@ function makeTestAPI(context) {
 
       onMessage: new SingletonEventManager(context, "test.onMessage", fire => {
         let handler = (event, ...args) => {
-          context.runSafe(fire, ...args);
+          fire.async(...args);
         };
 
         extension.on("test-harness-message", handler);

toolkit/components/extensions/ext-downloads.js

@@ -21,7 +21,6 @@ Cu.import("resource://gre/modules/ExtensionUtils.jsm");
 const {
   ignoreEvent,
   normalizeTime,
-  runSafeSync,
   SingletonEventManager,
   PlatformInfo,
 } = ExtensionUtils;
@@ -751,7 +750,7 @@ extensions.registerSchemaAPI("downloads", "addon_parent", context => {
           });
           if (Object.keys(changes).length > 0) {
             changes.id = item.id;
-            runSafeSync(context, fire, changes);
+            fire.async(changes);
           }
         };
 
@@ -767,7 +766,7 @@ extensions.registerSchemaAPI("downloads", "addon_parent", context => {
 
       onCreated: new SingletonEventManager(context, "downloads.onCreated", fire => {
         const handler = (what, item) => {
-          runSafeSync(context, fire, item.serialize());
+          fire.async(item.serialize());
         };
         let registerPromise = DownloadMap.getDownloadList().then(() => {
           DownloadMap.on("create", handler);
@@ -781,7 +780,7 @@ extensions.registerSchemaAPI("downloads", "addon_parent", context => {
 
       onErased: new SingletonEventManager(context, "downloads.onErased", fire => {
         const handler = (what, item) => {
-          runSafeSync(context, fire, item.id);
+          fire.async(item.id);
         };
         let registerPromise = DownloadMap.getDownloadList().then(() => {
           DownloadMap.on("erase", handler);

toolkit/components/extensions/ext-idle.js

@@ -81,7 +81,7 @@ extensions.registerSchemaAPI("idle", "addon_parent", context => {
       },
       onStateChanged: new SingletonEventManager(context, "idle.onStateChanged", fire => {
         let listener = (event, data) => {
-          context.runSafe(fire, data);
+          fire.sync(data);
         };
 
         getObserver(extension, context).on("stateChanged", listener);

toolkit/components/extensions/ext-runtime.js

@@ -28,7 +28,7 @@ extensions.registerSchemaAPI("runtime", "addon_parent", context => {
         }
         let listener = () => {
           if (extension.startupReason === "APP_STARTUP") {
-            fire();
+            fire.sync();
           }
         };
         extension.on("startup", listener);
@@ -42,14 +42,14 @@ extensions.registerSchemaAPI("runtime", "addon_parent", context => {
           switch (extension.startupReason) {
             case "APP_STARTUP":
               if (Extension.browserUpdated) {
-                fire({reason: "browser_update"});
+                fire.sync({reason: "browser_update"});
               }
               break;
             case "ADDON_INSTALL":
-              fire({reason: "install"});
+              fire.sync({reason: "install"});
               break;
             case "ADDON_UPGRADE":
-              fire({reason: "update"});
+              fire.sync({reason: "update"});
               break;
           }
         };
@@ -66,7 +66,7 @@ extensions.registerSchemaAPI("runtime", "addon_parent", context => {
           let details = {
             version: upgrade.version,
           };
-          context.runSafe(fire, details);
+          fire.sync(details);
         });
         return () => {
           AddonManager.removeUpgradeListener(instanceID);

toolkit/components/extensions/ext-webNavigation.js

@@ -98,7 +98,7 @@ function fillTransitionProperties(eventName, src, dst) {
 // Similar to WebRequestEventManager but for WebNavigation.
 function WebNavigationEventManager(context, eventName) {
   let name = `webNavigation.${eventName}`;
-  let register = (callback, urlFilters) => {
+  let register = (fire, urlFilters) => {
     // Don't create a MatchURLFilters instance if the listener does not include any filter.
     let filters = urlFilters ?
           new MatchURLFilters(urlFilters.url) : null;
@@ -127,7 +127,7 @@ function WebNavigationEventManager(context, eventName) {
 
       fillTransitionProperties(eventName, data, data2);
 
-      context.runSafe(callback, data2);
+      fire.async(data2);
     };
 
     WebNavigation[eventName].addListener(listener, filters);

toolkit/components/extensions/ext-webRequest.js

@@ -20,7 +20,7 @@ var {
 // when invoking listeners.
 function WebRequestEventManager(context, eventName) {
   let name = `webRequest.${eventName}`;
-  let register = (callback, filter, info) => {
+  let register = (fire, filter, info) => {
     let listener = data => {
       // Prevent listening in on requests originating from system principal to
       // prevent tinkering with OCSP, app and addon updates, etc.
@@ -65,7 +65,7 @@ function WebRequestEventManager(context, eventName) {
         }
       }
 
-      return context.runSafe(callback, data2);
+      return fire.sync(data2);
     };
 
     let filter2 = {};

toolkit/components/extensions/test/xpcshell/test_ext_contexts.js

@@ -72,9 +72,9 @@ add_task(function* test_post_unload_listeners() {
   });
 
   let fireSingleton;
-  let onSingleton = new SingletonEventManager(context, "onSingleton", callback => {
+  let onSingleton = new SingletonEventManager(context, "onSingleton", fire => {
     fireSingleton = () => {
-      Promise.resolve().then(callback);
+      fire.async();
     };
     return () => {};
   });