Bug 1203330 Part 3 Remove EventManager
Also clean up some test warnings related to event listener IPC
along the way.
MozReview-Commit-ID: KOBQX3TynV
--- a/toolkit/components/extensions/ExtensionUtils.jsm
+++ b/toolkit/components/extensions/ExtensionUtils.jsm
@@ -575,127 +575,35 @@ LocaleData.prototype = {
// Return the browser locale, but convert it to a Chrome-style
// locale code.
return Locale.getLocale().replace(/-/g, "_");
},
};
// 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);
// return () => {
// // Return a way to unregister the listener.
// SomehowUnregisterListener(listener);
// };
// }).api()
//
// The result is an object with addListener, removeListener, and
// 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;
this.register = register;
this.unregister = new Map();
}
SingletonEventManager.prototype = {
@@ -1237,17 +1145,16 @@ this.ExtensionUtils = {
runSafe,
runSafeSync,
runSafeSyncWithoutClone,
runSafeWithoutClone,
stylesheetMap,
DefaultMap,
DefaultWeakMap,
EventEmitter,
- EventManager,
ExtensionError,
IconDetails,
LocaleData,
MessageManagerProxy,
PlatformInfo,
SingletonEventManager,
SpreadArgs,
};
--- a/toolkit/components/extensions/test/xpcshell/test_ext_alarms.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_alarms.js
@@ -47,20 +47,23 @@ add_task(function* test_alarm_fires() {
background: `(${backgroundScript})()`,
manifest: {
permissions: ["alarms"],
},
});
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";
let timer;
browser.alarms.onAlarm.addListener(alarm => {
browser.test.assertEq(ALARM_NAME, alarm.name, "alarm has the expected name");
clearTimeout(timer);
@@ -81,20 +84,23 @@ add_task(function* test_alarm_fires_with
background: `(${backgroundScript})()`,
manifest: {
permissions: ["alarms"],
},
});
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";
browser.alarms.create(ALARM_NAME, {when: Date.now() + 2000});
let wasCleared = await browser.alarms.clear(ALARM_NAME + "1");
browser.test.assertFalse(wasCleared, "alarm was not cleared");
@@ -111,17 +117,16 @@ add_task(function* test_alarm_clear_non_
},
});
yield extension.startup();
yield extension.awaitFinish("alarm-clear");
yield extension.unload();
});
-
add_task(function* test_alarm_get_and_clear_single_argument() {
async function backgroundScript() {
browser.alarms.create({when: Date.now() + 2000});
let alarm = await browser.alarms.get();
browser.test.assertEq("", alarm.name, "expected alarm returned");
let wasCleared = await browser.alarms.clear();
--- a/toolkit/components/extensions/test/xpcshell/test_ext_contexts.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_contexts.js
@@ -7,17 +7,16 @@ Cu.import("resource://gre/modules/Timer.
Cu.import("resource://gre/modules/ExtensionCommon.jsm");
Cu.import("resource://gre/modules/ExtensionUtils.jsm");
var {
BaseContext,
} = ExtensionCommon;
var {
- EventManager,
SingletonEventManager,
} = ExtensionUtils;
class StubContext extends BaseContext {
constructor() {
let fakeExtension = {id: "test@web.extension"};
super("testEnv", fakeExtension);
this.sandbox = Cu.Sandbox(global);
@@ -60,70 +59,54 @@ add_task(function* test_post_unload_prom
// any micro-tasks that get enqueued by the resolution handlers above.
yield new Promise(resolve => setTimeout(resolve, 0));
});
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 = () => {
fire.async();
};
return () => {};
});
let fail = event => {
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);
context.unload();
// The `setTimeout` ensures that we return to the event loop after
// promise resolution, which means we're guaranteed to return after
// any micro-tasks that get enqueued by the resolution handlers above.
--- a/toolkit/components/extensions/test/xpcshell/test_ext_idle.js
+++ b/toolkit/components/extensions/test/xpcshell/test_ext_idle.js
@@ -140,16 +140,19 @@ add_task(function* testSetDetectionInter
});
idleService._reset();
yield extension.startup();
yield extension.awaitMessage("listenerAdded");
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();
});
add_task(function* testSetDetectionIntervalAfterAddingListener() {
function background() {
browser.idle.onStateChanged.addListener(newState => {
browser.test.assertEq("idle", newState, "listener fired with the expected state");
browser.test.sendMessage("listenerFired");
@@ -166,16 +169,20 @@ add_task(function* testSetDetectionInter
});
idleService._reset();
yield extension.startup();
yield extension.awaitMessage("detectionIntervalSet");
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();
});
add_task(function* testOnlyAddingListener() {
function background() {
browser.idle.onStateChanged.addListener(newState => {
browser.test.assertEq("active", newState, "listener fired with the expected state");
browser.test.sendMessage("listenerFired");
@@ -193,10 +200,14 @@ add_task(function* testOnlyAddingListene
idleService._reset();
yield extension.startup();
yield extension.awaitMessage("listenerAdded");
idleService._fireObservers("active");
yield extension.awaitMessage("listenerFired");
// 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();
});