Change onSubscribe & onUnsubscribe into a single onHasSubscribersChange.
- This better captures the use-case of the callback and reduces code. - Updates all affected usages. Github search of plugins repo shows no usages to update. Project: http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/commit/7ea6d82e Tree: http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/tree/7ea6d82e Diff: http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/diff/7ea6d82e Branch: refs/heads/master Commit: 7ea6d82e3a454e1874d62b5a5e2121641d3475de Parents: 60666b0 Author: Andrew Grieve <agri...@chromium.org> Authored: Mon Aug 27 22:58:07 2012 -0400 Committer: Andrew Grieve <agri...@chromium.org> Committed: Tue Sep 18 10:41:30 2012 -0400 ---------------------------------------------------------------------- lib/android/platform.js | 18 +++--------- lib/common/channel.js | 29 +++++++-------------- lib/common/plugin/battery.js | 30 ++++++++-------------- lib/cordova.js | 8 +++--- lib/webworks/java/platform.js | 48 ++++++++++++----------------------- lib/wp7/platform.js | 14 ++-------- test/test.channel.js | 21 +++++++++++++++- 7 files changed, 70 insertions(+), 98 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/7ea6d82e/lib/android/platform.js ---------------------------------------------------------------------- diff --git a/lib/android/platform.js b/lib/android/platform.js index bb38536..af5ab4f 100644 --- a/lib/android/platform.js +++ b/lib/android/platform.js @@ -27,19 +27,11 @@ module.exports = { exec = require('cordova/exec'); // Inject a listener for the backbutton on the document. - var backButtonChannel = cordova.addDocumentEventHandler('backbutton', { - onSubscribe:function() { - // If we just attached the first handler, let native know we need to override the back button. - if (this.numHandlers === 1) { - exec(null, null, "App", "overrideBackbutton", [true]); - } - }, - onUnsubscribe:function() { - // If we just detached the last handler, let native know we no longer override the back button. - if (this.numHandlers === 0) { - exec(null, null, "App", "overrideBackbutton", [false]); - } - } + var backButtonChannel = cordova.addDocumentEventHandler('backbutton'); + backButtonChannel.onHasSubscribersChange = function() { + // If we just attached the first handler or detached the last handler, + // let native know we need to override the back button. + exec(null, null, "App", "overrideBackbutton", [this.numHandlers == 1]); }); // Add hardware MENU and SEARCH button handlers http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/7ea6d82e/lib/common/channel.js ---------------------------------------------------------------------- diff --git a/lib/common/channel.js b/lib/common/channel.js index 4990d6d..21f0e6e 100644 --- a/lib/common/channel.js +++ b/lib/common/channel.js @@ -59,29 +59,16 @@ var utils = require('cordova/utils'), * Channel * @constructor * @param type String the channel name - * @param opts Object options to pass into the channel, currently - * supports: - * onSubscribe: callback that fires when - * something subscribes to the Channel. Sets - * context to the Channel. - * onUnsubscribe: callback that fires when - * something unsubscribes to the Channel. Sets - * context to the Channel. */ -var Channel = function(type, opts) { +var Channel = function(type) { this.type = type; this.handlers = {}; this.numHandlers = 0; this.fired = false; this.enabled = true; - this.events = { - onSubscribe:null, - onUnsubscribe:null - }; - if (opts) { - if (opts.onSubscribe) this.events.onSubscribe = opts.onSubscribe; - if (opts.onUnsubscribe) this.events.onUnsubscribe = opts.onUnsubscribe; - } + // Function that is called when the first listener is subscribed, or when + // the last listener is unsubscribed. + this.onHasSubscribersChange = null; }, channel = { /** @@ -174,7 +161,9 @@ Channel.prototype.subscribe = function(f, c, g) { if (!this.handlers[g]) { this.handlers[g] = func; this.numHandlers++; - if (this.events.onSubscribe) this.events.onSubscribe.call(this); + if (this.numHandlers == 1) { + this.onHasSubscribersChange && this.onHasSubscribersChange(); + } if (this.fired) func.apply(this, this.fireArgs); } return g; @@ -215,7 +204,9 @@ Channel.prototype.unsubscribe = function(g) { if (handler.observer_guid) handler.observer_guid=null; delete this.handlers[g]; this.numHandlers--; - if (this.events.onUnsubscribe) this.events.onUnsubscribe.call(this); + if (this.numHandlers == 0) { + this.onHasSubscribersChange && this.onHasSubscribersChange(); + } } }; http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/7ea6d82e/lib/common/plugin/battery.js ---------------------------------------------------------------------- diff --git a/lib/common/plugin/battery.js b/lib/common/plugin/battery.js index 9236d84..7dddd4b 100644 --- a/lib/common/plugin/battery.js +++ b/lib/common/plugin/battery.js @@ -36,34 +36,26 @@ var Battery = function() { this._level = null; this._isPlugged = null; // Create new event handlers on the window (returns a channel instance) - var subscriptionEvents = { - onSubscribe:this.onSubscribe, - onUnsubscribe:this.onUnsubscribe - }; this.channels = { - batterystatus:cordova.addWindowEventHandler("batterystatus", subscriptionEvents), - batterylow:cordova.addWindowEventHandler("batterylow", subscriptionEvents), - batterycritical:cordova.addWindowEventHandler("batterycritical", subscriptionEvents) + batterystatus:cordova.addWindowEventHandler("batterystatus"), + batterylow:cordova.addWindowEventHandler("batterylow"), + batterycritical:cordova.addWindowEventHandler("batterycritical") }; + for (var key in this.channels) { + this.channels[key].onHasSubscribersChange = Battery.onHasSubscribersChange; + } + }; /** * Event handlers for when callbacks get registered for the battery. * Keep track of how many handlers we have so we can start and stop the native battery listener * appropriately (and hopefully save on battery life!). */ -Battery.prototype.onSubscribe = function() { - var me = battery; +Battery.onHasSubscribersChange = function() { // If we just registered the first handler, make sure native listener is started. - if (handlers() === 1) { - exec(me._status, me._error, "Battery", "start", []); - } -}; - -Battery.prototype.onUnsubscribe = function() { - var me = battery; - - // If we just unregistered the last handler, make sure native listener is stopped. - if (handlers() === 0) { + if (this.numHandlers === 1 && handlers() === 1) { + exec(battery._status, battery._error, "Battery", "start", []); + } else if (handlers() === 0) { exec(null, null, "Battery", "stop", []); } }; http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/7ea6d82e/lib/cordova.js ---------------------------------------------------------------------- diff --git a/lib/cordova.js b/lib/cordova.js index 9cdbab4..0a7e92e 100644 --- a/lib/cordova.js +++ b/lib/cordova.js @@ -114,11 +114,11 @@ var cordova = { /** * Methods to add/remove your own addEventListener hijacking on document + window. */ - addWindowEventHandler:function(event, opts) { - return (windowEventHandlers[event] = channel.create(event, opts)); + addWindowEventHandler:function(event) { + return (windowEventHandlers[event] = channel.create(event)); }, - addDocumentEventHandler:function(event, opts) { - return (documentEventHandlers[event] = channel.create(event, opts)); + addDocumentEventHandler:function(event) { + return (documentEventHandlers[event] = channel.create(event)); }, removeWindowEventHandler:function(event) { delete windowEventHandlers[event]; http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/7ea6d82e/lib/webworks/java/platform.js ---------------------------------------------------------------------- diff --git a/lib/webworks/java/platform.js b/lib/webworks/java/platform.js index 4a62ebe..54eefed 100644 --- a/lib/webworks/java/platform.js +++ b/lib/webworks/java/platform.js @@ -65,28 +65,27 @@ module.exports = { }; var eventHandler = function(event) { - return { onSubscribe : function() { + return function() { // If we just attached the first handler, let native know we // need to override the hardware button. - if (this.numHandlers === 1) { + if (this.numHandlers) { blackberry.system.event.onHardwareKey( buttonMapping[event], fireEvent(event)); } - }, - onUnsubscribe : function() { // If we just detached the last handler, let native know we // no longer override the hardware button. - if (this.numHandlers === 0) { + else { blackberry.system.event.onHardwareKey( buttonMapping[event], null); } - }}; + }; }; // Inject listeners for buttons on the document. for (var button in buttonMapping) { if (buttonMapping.hasOwnProperty(button)) { - cordova.addDocumentEventHandler(button, eventHandler(button)); + var channel = cordova.addDocumentEventHandler(button); + channel.onHasSubscribersChange = eventHandler(button); } } @@ -106,8 +105,13 @@ module.exports = { // Unsubscribe handler - turns off native backlight change // listener - var onUnsubscribe = function() { - if (channel.onResume.numHandlers === 0 && channel.onPause.numHandlers === 0) { + var onHasSubscribersChange = function() { + // If we just attached the first handler and there are + // no pause handlers, start the backlight system + // listener on the native side. + if (this.numHandlers && (channel.onResume.numHandlers + channel.onPause.numHandlers === 1)) { + exec(backlightWin, backlightFail, "App", "detectBacklight", []); + } else if (channel.onResume.numHandlers === 0 && channel.onPause.numHandlers === 0) { exec(null, null, 'App', 'ignoreBacklight', []); } }; @@ -126,28 +130,10 @@ module.exports = { // Override stock resume and pause listeners so we can trigger // some native methods during attach/remove - channel.onResume = cordova.addDocumentEventHandler('resume', { - onSubscribe:function() { - // If we just attached the first handler and there are - // no pause handlers, start the backlight system - // listener on the native side. - if (channel.onResume.numHandlers === 1 && channel.onPause.numHandlers === 0) { - exec(backlightWin, backlightFail, "App", "detectBacklight", []); - } - }, - onUnsubscribe:onUnsubscribe - }); - channel.onPause = cordova.addDocumentEventHandler('pause', { - onSubscribe:function() { - // If we just attached the first handler and there are - // no resume handlers, start the backlight system - // listener on the native side. - if (channel.onResume.numHandlers === 0 && channel.onPause.numHandlers === 1) { - exec(backlightWin, backlightFail, "App", "detectBacklight", []); - } - }, - onUnsubscribe:onUnsubscribe - }); + channel.onResume = cordova.addDocumentEventHandler('resume'); + channel.onResume.onHasSubscribersChange = onHasSubscribersChange; + channel.onPause = cordova.addDocumentEventHandler('pause'); + channel.onPause.onHasSubscribersChange = onHasSubscribersChange; // Fire resume event when application brought to foreground. blackberry.app.event.onForeground(resume); http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/7ea6d82e/lib/wp7/platform.js ---------------------------------------------------------------------- diff --git a/lib/wp7/platform.js b/lib/wp7/platform.js index bf3dbb9..2903c54 100644 --- a/lib/wp7/platform.js +++ b/lib/wp7/platform.js @@ -37,17 +37,9 @@ module.exports = { window.alert = require("cordova/plugin/notification").alert; // Inject a listener for the backbutton, and tell native to override the flag (true/false) when we have 1 or more, or 0, listeners - var backButtonChannel = cordova.addDocumentEventHandler('backbutton', { - onSubscribe:function() { - if (this.numHandlers === 1) { - exec(null, null, "CoreEvents", "overridebackbutton", [true]); - } - }, - onUnsubscribe:function() { - if (this.numHandlers === 0) { - exec(null, null, "CoreEvents", "overridebackbutton", [false]); - } - } + var backButtonChannel = cordova.addDocumentEventHandler('backbutton'); + backButtonChannel.onHasSubscribersChange = function() { + exec(null, null, "CoreEvents", "overridebackbutton", [this.numHandlers == 1]); }); }, objects: { http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/7ea6d82e/test/test.channel.js ---------------------------------------------------------------------- diff --git a/test/test.channel.js b/test/test.channel.js index dc9e661..e71f993 100644 --- a/test/test.channel.js +++ b/test/test.channel.js @@ -24,7 +24,6 @@ describe("channel", function () { c; beforeEach(function() { - c = null; c = channel.create('masterexploder'); }); @@ -264,4 +263,24 @@ describe("channel", function () { expect(count).toEqual(2); }); }); + describe("onHasSubscribersChange", function() { + it("should be called only when the first subscriber is added and last subscriber is removed.", function() { + var handler = jasmine.createSpy().andCallFake(function() { + var callCount = handler.argsForCall.length; + if (callCount == 1) { + expect(this.numHandlers).toEqual(1); + } else { + expect(this.numHandlers).toEqual(0); + } + }); + c.onHasSubscribersChange = handler; + function foo1() {} + function foo2() {} + c.subscribe(foo1); + c.subscribe(foo2); + c.unsubscribe(foo1); + c.unsubscribe(foo2); + expect(handler.argsForCall.length).toEqual(2); + }); + }); });