You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cordova.apache.org by ag...@apache.org on 2012/09/18 16:43:25 UTC

[2/2] js commit: Change onSubscribe & onUnsubscribe into a single onHasSubscribersChange.

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 <ag...@chromium.org>
Authored: Mon Aug 27 22:58:07 2012 -0400
Committer: Andrew Grieve <ag...@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);
+        });
+    });
 });