Bug 1203330 Part 3 Remove EventManager r=kmag

Also clean up some test warnings related to event listener IPC
along the way.

MozReview-Commit-ID: KOBQX3TynV

--HG--
extra : rebase_source : d833b580c7a55abf1618fcbefdc8e11d78a1651e

toolkit/components/extensions/ExtensionUtils.jsm

@@ -580,10 +580,10 @@ LocaleData.prototype = {
 
 // This is a generic class for managing event listeners. Example usage:
 //
-// new EventManager(context, "api.subAPI", fire => {
+// new SingletonEventManager(context, "api.subAPI", fire => {
 //   let listener = (...) => {
 //     // Fire any listeners registered with addListener.
-//     fire(arg1, arg2);
+//     fire.async(arg1, arg2);
 //   };
 //   // Register the listener.
 //   SomehowRegisterListener(listener);
@@ -597,100 +597,8 @@ LocaleData.prototype = {
 // hasListener methods. |context| is an add-on scope (either an
 // ExtensionContext in the chrome process or ExtensionContext in a
 // content process). |name| is for debugging. |register| is a function
-// to register the listener. |register| is only called once, even if
-// multiple listeners are registered. |register| should return an
+// to register the listener. |register| should return an
 // unregister function that will unregister the listener.
-function EventManager(context, name, register) {
-  this.context = context;
-  this.name = name;
-  this.register = register;
-  this.unregister = null;
-  this.callbacks = new Set();
-}
-
-EventManager.prototype = {
-  addListener(callback) {
-    if (typeof(callback) != "function") {
-      dump(`Expected function\n${Error().stack}`);
-      return;
-    }
-    if (this.context.unloaded) {
-      dump(`Cannot add listener to ${this.name} after context unloaded`);
-      return;
-    }
-
-    if (!this.callbacks.size) {
-      this.context.callOnClose(this);
-
-      let fireFunc = this.fire.bind(this);
-      let fireWithoutClone = this.fireWithoutClone.bind(this);
-      fireFunc.withoutClone = fireWithoutClone;
-      this.unregister = this.register(fireFunc);
-    }
-    this.callbacks.add(callback);
-  },
-
-  removeListener(callback) {
-    if (!this.callbacks.size) {
-      return;
-    }
-
-    this.callbacks.delete(callback);
-    if (this.callbacks.size == 0) {
-      this.unregister();
-      this.unregister = null;
-
-      this.context.forgetOnClose(this);
-    }
-  },
-
-  hasListener(callback) {
-    return this.callbacks.has(callback);
-  },
-
-  fire(...args) {
-    this._fireCommon("runSafe", args);
-  },
-
-  fireWithoutClone(...args) {
-    this._fireCommon("runSafeWithoutClone", args);
-  },
-
-  _fireCommon(runSafeMethod, args) {
-    for (let callback of this.callbacks) {
-      Promise.resolve(callback).then(callback => {
-        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.callbacks.has(callback)) {
-          this.context[runSafeMethod](callback, ...args);
-        }
-      });
-    }
-  },
-
-  close() {
-    if (this.callbacks.size) {
-      this.unregister();
-    }
-    this.callbacks.clear();
-    this.register = null;
-    this.unregister = null;
-  },
-
-  api() {
-    return {
-      addListener: callback => this.addListener(callback),
-      removeListener: callback => this.removeListener(callback),
-      hasListener: callback => this.hasListener(callback),
-    };
-  },
-};
-
-// Similar to EventManager, but it doesn't try to consolidate event
-// notifications. Each addListener call causes us to register once. It
-// allows extra arguments to be passed to addListener.
 function SingletonEventManager(context, name, register) {
   this.context = context;
   this.name = name;
@@ -1242,7 +1150,6 @@ this.ExtensionUtils = {
   DefaultMap,
   DefaultWeakMap,
   EventEmitter,
-  EventManager,
   ExtensionError,
   IconDetails,
   LocaleData,

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

@@ -52,10 +52,13 @@ add_task(function* test_alarm_fires() {
 
   yield extension.startup();
   yield extension.awaitFinish("alarm-fires");
+
+  // Defer unloading the extension so the asynchronous event listener
+  // reply finishes.
+  yield new Promise(resolve => setTimeout(resolve, 0));
   yield extension.unload();
 });
 
-
 add_task(function* test_alarm_fires_with_when() {
   function backgroundScript() {
     let ALARM_NAME = "test_ext_alarms";
@@ -86,10 +89,13 @@ add_task(function* test_alarm_fires_with_when() {
 
   yield extension.startup();
   yield extension.awaitFinish("alarm-when");
+
+  // Defer unloading the extension so the asynchronous event listener
+  // reply finishes.
+  yield new Promise(resolve => setTimeout(resolve, 0));
   yield extension.unload();
 });
 
-
 add_task(function* test_alarm_clear_non_matching_name() {
   async function backgroundScript() {
     let ALARM_NAME = "test_ext_alarms";
@@ -116,7 +122,6 @@ add_task(function* test_alarm_clear_non_matching_name() {
   yield extension.unload();
 });
 
-
 add_task(function* test_alarm_get_and_clear_single_argument() {
   async function backgroundScript() {
     browser.alarms.create({when: Date.now() + 2000});

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

@@ -12,7 +12,6 @@ var {
 } = ExtensionCommon;
 
 var {
-  EventManager,
   SingletonEventManager,
 } = ExtensionUtils;
 
@@ -65,12 +64,6 @@ add_task(function* test_post_unload_promises() {
 add_task(function* test_post_unload_listeners() {
   let context = new StubContext();
 
-  let fireEvent;
-  let onEvent = new EventManager(context, "onEvent", fire => {
-    fireEvent = fire;
-    return () => {};
-  });
-
   let fireSingleton;
   let onSingleton = new SingletonEventManager(context, "onSingleton", fire => {
     fireSingleton = () => {
@@ -83,42 +76,32 @@ add_task(function* test_post_unload_listeners() {
     ok(false, `Unexpected event: ${event}`);
   };
 
-  // Check that event listeners aren't called after they've been removed.
-  onEvent.addListener(fail);
+  // Check that event listeners isn't called after it has been removed.
   onSingleton.addListener(fail);
 
-  let promises = [
-    new Promise(resolve => onEvent.addListener(resolve)),
-    new Promise(resolve => onSingleton.addListener(resolve)),
-  ];
+  let promise = new Promise(resolve => onSingleton.addListener(resolve));
 
-  fireEvent("onEvent");
   fireSingleton("onSingleton");
 
-  // Both `fireEvent` calls are dispatched asynchronously, so they won't
-  // have fired by this point. The `fail` listeners that we remove now
-  // should not be called, even though the events have already been
+  // The `fireSingleton` call ia dispatched asynchronously, so it won't
+  // have fired by this point. The `fail` listener that we remove now
+  // should not be called, even though the event has already been
   // enqueued.
-  onEvent.removeListener(fail);
   onSingleton.removeListener(fail);
 
-  // Wait for the remaining listeners to be called, which should always
-  // happen after the `fail` listeners would normally be called.
-  yield Promise.all(promises);
+  // Wait for the remaining listener to be called, which should always
+  // happen after the `fail` listener would normally be called.
+  yield promise;
 
-  // Check that event listeners aren't called after the context has
+  // Check that the event listener isn't called after the context has
   // unloaded.
-  onEvent.addListener(fail);
   onSingleton.addListener(fail);
 
-  // The EventManager `fire` callback always dispatches events
+  // The `fire` callback always dispatches events
   // asynchronously, so we need to test that any pending event callbacks
   // aren't fired after the context unloads. We also need to test that
   // any `fire` calls that happen *after* the context is unloaded also
   // do not trigger callbacks.
-  fireEvent("onEvent");
-  Promise.resolve("onEvent").then(fireEvent);
-
   fireSingleton("onSingleton");
   Promise.resolve("onSingleton").then(fireSingleton);
 

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

@@ -145,6 +145,9 @@ add_task(function* testSetDetectionIntervalBeforeAddingListener() {
   idleService._fireObservers("idle");
   yield extension.awaitMessage("listenerFired");
   checkActivity({expectedAdd: [99], expectedRemove: [], expectedFires: ["idle"]});
+  // Defer unloading the extension so the asynchronous event listener
+  // reply finishes.
+  yield new Promise(resolve => setTimeout(resolve, 0));
   yield extension.unload();
 });
 
@@ -171,6 +174,10 @@ add_task(function* testSetDetectionIntervalAfterAddingListener() {
   idleService._fireObservers("idle");
   yield extension.awaitMessage("listenerFired");
   checkActivity({expectedAdd: [60, 99], expectedRemove: [60], expectedFires: ["idle"]});
+
+  // Defer unloading the extension so the asynchronous event listener
+  // reply finishes.
+  yield new Promise(resolve => setTimeout(resolve, 0));
   yield extension.unload();
 });
 
@@ -198,5 +205,9 @@ add_task(function* testOnlyAddingListener() {
   // check that "idle-daily" topic does not cause a listener to fire
   idleService._fireObservers("idle-daily");
   checkActivity({expectedAdd: [60], expectedRemove: [], expectedFires: ["active", "idle-daily"]});
+
+  // Defer unloading the extension so the asynchronous event listener
+  // reply finishes.
+  yield new Promise(resolve => setTimeout(resolve, 0));
   yield extension.unload();
 });