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

js commit: [common] [CB-1462] bad logic in geo watchPosition. changed internal timeout var to object to be able to pass-by-reference with it to fix issue.

Updated Branches:
  refs/heads/master 143f5221a -> dab8e8485


[common] [CB-1462] bad logic in geo watchPosition. changed internal timeout var to object to be able to pass-by-reference with it to fix issue.


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/dab8e848
Tree: http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/tree/dab8e848
Diff: http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/diff/dab8e848

Branch: refs/heads/master
Commit: dab8e8485f7c8801a5344aaf95aad6c53ca57eba
Parents: 143f522
Author: Fil Maj <ma...@gmail.com>
Authored: Mon Sep 17 12:20:32 2012 -0700
Committer: Fil Maj <ma...@gmail.com>
Committed: Mon Sep 17 12:20:32 2012 -0700

----------------------------------------------------------------------
 lib/common/plugin/geolocation.js |   24 ++++++++++++------------
 test/test.geolocation.js         |   13 +++++++++++++
 2 files changed, 25 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/dab8e848/lib/common/plugin/geolocation.js
----------------------------------------------------------------------
diff --git a/lib/common/plugin/geolocation.js b/lib/common/plugin/geolocation.js
index 050df62..f6e3221 100644
--- a/lib/common/plugin/geolocation.js
+++ b/lib/common/plugin/geolocation.js
@@ -62,11 +62,11 @@ var geolocation = {
 
         // Timer var that will fire an error callback if no position is retrieved from native
         // before the "timeout" param provided expires
-        var timeoutTimer = null;
+        var timeoutTimer = {timer:null};
 
         var win = function(p) {
-            clearTimeout(timeoutTimer);
-            if (!timeoutTimer) {
+            clearTimeout(timeoutTimer.timer);
+            if (!(timeoutTimer.timer)) {
                 // Timeout already happened, or native fired error callback for
                 // this geo request.
                 // Don't continue with success callback.
@@ -88,8 +88,8 @@ var geolocation = {
             successCallback(pos);
         };
         var fail = function(e) {
-            clearTimeout(timeoutTimer);
-            timeoutTimer = null;
+            clearTimeout(timeoutTimer.timer);
+            timeoutTimer.timer = null;
             var err = new PositionError(e.code, e.message);
             if (errorCallback) {
                 errorCallback(err);
@@ -112,12 +112,12 @@ var geolocation = {
                 // If the timeout value was not set to Infinity (default), then
                 // set up a timeout function that will fire the error callback
                 // if no successful position was retrieved before timeout expired.
-                timeoutTimer = createTimeout(fail, options.timeout);
+                timeoutTimer.timer = createTimeout(fail, options.timeout);
             } else {
                 // This is here so the check in the win function doesn't mess stuff up
                 // may seem weird but this guarantees timeoutTimer is
                 // always truthy before we call into native
-                timeoutTimer = true;
+                timeoutTimer.timer = true;
             }
             exec(win, fail, "Geolocation", "getLocation", [options.enableHighAccuracy, options.maximumAge]);
         }
@@ -144,7 +144,7 @@ var geolocation = {
         timers[id] = geolocation.getCurrentPosition(successCallback, errorCallback, options);
 
         var fail = function(e) {
-            clearTimeout(timers[id]);
+            clearTimeout(timers[id].timer);
             var err = new PositionError(e.code, e.message);
             if (errorCallback) {
                 errorCallback(err);
@@ -152,9 +152,9 @@ var geolocation = {
         };
 
         var win = function(p) {
-            clearTimeout(timers[id]);
+            clearTimeout(timers[id].timer);
             if (options.timeout !== Infinity) {
-                timers[id] = createTimeout(fail, options.timeout);
+                timers[id].timer = createTimeout(fail, options.timeout);
             }
             var pos = new Position(
                 {
@@ -183,8 +183,8 @@ var geolocation = {
      */
     clearWatch:function(id) {
         if (id && timers[id] !== undefined) {
-            clearTimeout(timers[id]);
-            delete timers[id];
+            clearTimeout(timers[id].timer);
+            timers[id].timer = false;
             exec(null, null, "Geolocation", "clearWatch", [id]);
         }
     }

http://git-wip-us.apache.org/repos/asf/incubator-cordova-js/blob/dab8e848/test/test.geolocation.js
----------------------------------------------------------------------
diff --git a/test/test.geolocation.js b/test/test.geolocation.js
index 3eae38f..070968e 100644
--- a/test/test.geolocation.js
+++ b/test/test.geolocation.js
@@ -253,5 +253,18 @@ describe("geolocation", function () {
             geo.clearWatch(id);
             expect(exec).not.toHaveBeenCalled();
         });
+        it("watchPosition followed by clearWatch should not invoke any success callbacks", function() {
+            // watchPosition calls native getLocation then addWatch method in order.
+            var id = geo.watchPosition(s, e);
+            // Get ref to getLocation success callback, to fake the native interaction.
+            var getLocationWin = exec.argsForCall[0][0];
+
+            // Next clear the watch id, but call the get location success callback as if the native framework has fired it after a clear watch.
+            geo.clearWatch(id);
+            getLocationWin({});
+
+            // End result: expect the success callback not to have been called.
+            expect(s).not.toHaveBeenCalled();
+        });
     });
 });