Bug 1250469 - Reduce number of ObserverService observers subscribed by the SDK Disposable class.
MozReview-Commit-ID: 32X9M07FbHO
--- a/addon-sdk/source/lib/sdk/core/disposable.js
+++ b/addon-sdk/source/lib/sdk/core/disposable.js
@@ -3,31 +3,29 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
"use strict";
module.metadata = {
"stability": "experimental"
};
-
const { Class } = require("./heritage");
const { Observer, subscribe, unsubscribe, observe } = require("./observer");
-const { isWeak, WeakReference } = require("./reference");
+const { isWeak } = require("./reference");
+const SDKWeakSet = require("../lang/weak-set");
+
const method = require("../../method/core");
const unloadSubject = require('@loader/unload');
const addonUnloadTopic = "sdk:loader:destroy";
-
-
const uninstall = method("disposable/uninstall");
exports.uninstall = uninstall;
-
const shutdown = method("disposable/shutdown");
exports.shutdown = shutdown;
const disable = method("disposable/disable");
exports.disable = disable;
const upgrade = method("disposable/upgrade");
exports.upgrade = upgrade;
@@ -37,38 +35,104 @@ exports.downgrade = downgrade;
const unload = method("disposable/unload");
exports.unload = unload;
const dispose = method("disposable/dispose");
exports.dispose = dispose;
dispose.define(Object, object => object.dispose());
-
const setup = method("disposable/setup");
exports.setup = setup;
setup.define(Object, (object, ...args) => object.setup(...args));
+// DisposablesUnloadObserver is the class which subscribe the
+// Observer Service to be notified when the add-on loader is
+// unloading to be able to dispose all the existent disposables.
+const DisposablesUnloadObserver = Class({
+ implements: [Observer],
+ initialize: function(...args) {
+ // Set of the non-weak disposables registered to be disposed.
+ this.disposables = new Set();
+ // Target of the weak disposables registered to be disposed
+ // (and tracked on this target using the SDK weak-set module).
+ this.weakDisposables = {};
+ },
+ subscribe(disposable) {
+ if (isWeak(disposable)) {
+ SDKWeakSet.add(this.weakDisposables, disposable);
+ } else {
+ this.disposables.add(disposable);
+ }
+ },
+ unsubscribe(disposable) {
+ if (isWeak(disposable)) {
+ SDKWeakSet.remove(this.weakDisposables, disposable);
+ } else {
+ this.disposables.delete(disposable);
+ }
+ },
+ tryUnloadDisposable(disposable) {
+ try {
+ if (disposable) {
+ unload(disposable);
+ }
+ } catch(e) {
+ console.error("Error unloading a",
+ isWeak(disposable) ? "weak disposable" : "disposable",
+ disposable, e);
+ }
+ },
+ unloadAll() {
+ // Remove all the subscribed disposables.
+ for (let disposable of this.disposables) {
+ this.tryUnloadDisposable(disposable);
+ }
+
+ this.disposables.clear();
+
+ // Remove all the subscribed weak disposables.
+ for (let disposable of SDKWeakSet.iterator(this.weakDisposables)) {
+ this.tryUnloadDisposable(disposable);
+ }
+
+ SDKWeakSet.clear(this.weakDisposables);
+ }
+});
+const disposablesUnloadObserver = new DisposablesUnloadObserver();
+
+// The DisposablesUnloadObserver instance is the only object which subscribes
+// the Observer Service directly, it observes add-on unload notifications in
+// order to trigger `unload` on all its subscribed disposables.
+observe.define(DisposablesUnloadObserver, (obj, subject, topic, data) => {
+ const isUnloadTopic = topic === addonUnloadTopic;
+ const isUnloadSubject = subject.wrappedJSObject === unloadSubject;
+ if (isUnloadTopic && isUnloadSubject) {
+ unsubscribe(disposablesUnloadObserver, addonUnloadTopic);
+ disposablesUnloadObserver.unloadAll();
+ }
+});
+
+subscribe(disposablesUnloadObserver, addonUnloadTopic, false);
// Set's up disposable instance.
const setupDisposable = disposable => {
- subscribe(disposable, addonUnloadTopic, isWeak(disposable));
+ disposablesUnloadObserver.subscribe(disposable);
};
exports.setupDisposable = setupDisposable;
// Tears down disposable instance.
const disposeDisposable = disposable => {
- unsubscribe(disposable, addonUnloadTopic);
+ disposablesUnloadObserver.unsubscribe(disposable);
};
exports.disposeDisposable = disposeDisposable;
// Base type that takes care of disposing it's instances on add-on unload.
// Also makes sure to remove unload listener if it's already being disposed.
const Disposable = Class({
- implements: [Observer],
initialize: function(...args) {
// First setup instance before initializing it's disposal. If instance
// fails to initialize then there is no instance to be disposed at the
// unload.
setup(this, ...args);
setupDisposable(this);
},
destroy: function(reason) {
@@ -81,50 +145,39 @@ const Disposable = Class({
// Implement your initialize logic here.
},
dispose: function() {
// Implement your cleanup logic here.
}
});
exports.Disposable = Disposable;
-// Disposable instances observe add-on unload notifications in
-// order to trigger `unload` on them.
-observe.define(Disposable, (disposable, subject, topic, data) => {
- const isUnloadTopic = topic === addonUnloadTopic;
- const isUnloadSubject = subject.wrappedJSObject === unloadSubject;
- if (isUnloadTopic && isUnloadSubject) {
- unsubscribe(disposable, topic);
- unload(disposable);
- }
-});
-
const unloaders = {
destroy: dispose,
uninstall: uninstall,
shutdown: shutdown,
disable: disable,
upgrade: upgrade,
downgrade: downgrade
-}
+};
+
const unloaded = new WeakMap();
unload.define(Disposable, (disposable, reason) => {
if (!unloaded.get(disposable)) {
unloaded.set(disposable, true);
// Pick an unload handler associated with an unload
// reason (falling back to destroy if not found) and
// delegate unloading to it.
const unload = unloaders[reason] || unloaders.destroy;
unload(disposable);
}
});
-
-// If add-on is disabled munally, it's being upgraded, downgraded
-// or uniststalled `dispose` is invoked to undo any changes that
+// If add-on is disabled manually, it's being upgraded, downgraded
+// or uninstalled `dispose` is invoked to undo any changes that
// has being done by it in this session.
disable.define(Disposable, dispose);
downgrade.define(Disposable, dispose);
upgrade.define(Disposable, dispose);
uninstall.define(Disposable, dispose);
// If application is shut down no dispose is invoked as undo-ing
// changes made by instance is likely to just waste of resources &
--- a/addon-sdk/source/test/test-disposable.js
+++ b/addon-sdk/source/test/test-disposable.js
@@ -4,25 +4,53 @@
"use strict";
const { Loader } = require("sdk/test/loader");
const { Class } = require("sdk/core/heritage");
const { Disposable } = require("sdk/core/disposable");
const { Cc, Ci, Cu } = require("chrome");
const { setTimeout } = require("sdk/timers");
+exports["test disposeDisposable"] = assert => {
+ let loader = Loader(module);
+
+ const { Disposable, disposeDisposable } = loader.require("sdk/core/disposable");
+ const { isWeak, WeakReference } = loader.require("sdk/core/reference");
+
+ let disposals = 0;
+
+ const Foo = Class({
+ extends: Disposable,
+ implements: [WeakReference],
+ dispose(...params) {
+ disposeDisposable(this);
+ disposals = disposals + 1;
+ }
+ });
+
+ const f1 = new Foo();
+ assert.equal(isWeak(f1), true, "f1 has WeakReference support");
+
+ f1.dispose();
+ assert.equal(disposals, 1, "disposed on dispose");
+
+ loader.unload("uninstall");
+ assert.equal(disposals, 1, "after disposeDisposable, dispose is not called anymore");
+};
+
exports["test destroy reasons"] = assert => {
+ let disposals = 0;
+
const Foo = Class({
extends: Disposable,
dispose: function() {
disposals = disposals + 1;
}
});
- let disposals = 0;
const f1 = new Foo();
f1.destroy();
assert.equal(disposals, 1, "disposed on destroy");
f1.destroy();
assert.equal(disposals, 1, "second destroy is ignored");
disposals = 0;
@@ -33,19 +61,20 @@ exports["test destroy reasons"] = assert
f2.destroy("uninstall")
f2.destroy();
assert.equal(disposals, 1, "disposal happens just once");
disposals = 0;
const f3 = new Foo();
f3.destroy("shutdown");
- assert.equal(disposals, 0, "shutdown doesn't invoke disposal");
+ assert.equal(disposals, 0, "shutdown invoke disposal");
+ f3.destroy("shutdown");
f3.destroy();
- assert.equal(disposals, 0, "shutdown already skipped disposal");
+ assert.equal(disposals, 0, "shutdown disposal happens just once");
disposals = 0;
const f4 = new Foo();
f4.destroy("disable");
assert.equal(disposals, 1, "disable invokes disposal");
f4.destroy("disable")
f4.destroy();
@@ -144,17 +173,17 @@ exports["test different unload hooks"] =
const u7 = new UberUnload();
u7.destroy("whatever");
u7.destroy();
u7.destroy("uninstall");
assert.deepEqual(u7.log, ["dispose"], "dispose hook invoked");
};
-exports["test disposables are desposed on unload"] = function(assert) {
+exports["test disposables are disposed on unload"] = function(assert) {
let loader = Loader(module);
let { Disposable } = loader.require("sdk/core/disposable");
let arg1 = {}
let arg2 = 2
let disposals = 0
let Foo = Class({