cordova-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From agri...@apache.org
Subject js commit: [android] CB-2963 No longer return a value from exec() when calls are synchronous
Date Tue, 16 Apr 2013 19:01:37 GMT
Updated Branches:
  refs/heads/master 62288781c -> e80f4afdc


[android] CB-2963 No longer return a value from exec() when calls are synchronous

Fixes a quirk in behaviour where nested callbacks are called before
outer callbacks finish.


Project: http://git-wip-us.apache.org/repos/asf/cordova-js/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-js/commit/e80f4afd
Tree: http://git-wip-us.apache.org/repos/asf/cordova-js/tree/e80f4afd
Diff: http://git-wip-us.apache.org/repos/asf/cordova-js/diff/e80f4afd

Branch: refs/heads/master
Commit: e80f4afdc14a5f28e41d075f3c3dbdbf3fbdbb34
Parents: 6228878
Author: Andrew Grieve <agrieve@chromium.org>
Authored: Tue Apr 16 14:59:59 2013 -0400
Committer: Andrew Grieve <agrieve@chromium.org>
Committed: Tue Apr 16 14:59:59 2013 -0400

----------------------------------------------------------------------
 lib/android/exec.js       |   44 +++++++++++----------------------
 test/android/test.exec.js |   53 ++++++++++++++++++++-------------------
 2 files changed, 42 insertions(+), 55 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-js/blob/e80f4afd/lib/android/exec.js
----------------------------------------------------------------------
diff --git a/lib/android/exec.js b/lib/android/exec.js
index be58dff..206c09a 100644
--- a/lib/android/exec.js
+++ b/lib/android/exec.js
@@ -79,18 +79,12 @@ function androidExec(success, fail, service, action, args) {
     }
 
     var callbackId = service + cordova.callbackId++,
-        argsJson = JSON.stringify(args),
-        returnValue;
+        argsJson = JSON.stringify(args);
 
-    // TODO: Returning the payload of a synchronous call was deprecated in 2.2.0.
-    // Remove it after 6 months.
-    function captureReturnValue(value) {
-        returnValue = value;
-        success && success(value);
+    if (success || fail) {
+        cordova.callbacks[callbackId] = {success:success, fail:fail};
     }
 
-    cordova.callbacks[callbackId] = {success:captureReturnValue, fail:fail};
-
     if (jsToNativeBridgeMode == jsToNativeModes.LOCATION_CHANGE) {
         window.location = 'http://cdv_exec/' + service + '#' + action + '#' + callbackId
+ '#' + argsJson;
     } else {
@@ -106,14 +100,6 @@ function androidExec(success, fail, service, action, args) {
             androidExec.processMessages(messages);
         }
     }
-    if (cordova.callbacks[callbackId]) {
-        if (success || fail) {
-            cordova.callbacks[callbackId].success = success;
-        } else {
-            delete cordova.callbacks[callbackId];
-        }
-    }
-    return returnValue;
 }
 
 function pollOnce() {
@@ -229,30 +215,30 @@ function processMessage(message) {
 androidExec.processMessages = function(messages) {
     if (messages) {
         messagesFromNative.push(messages);
+        // Check for the reentrant case, and enqueue the message if that's the case.
+        if (messagesFromNative.length > 1) {
+            return;
+        }
         while (messagesFromNative.length) {
-            messages = messagesFromNative.shift();
+            // Don't unshift until the end so that reentrancy can be detected.
+            messages = messagesFromNative[0];
             // The Java side can send a * message to indicate that it
             // still has messages waiting to be retrieved.
-            // TODO(agrieve): This is currently disabled on the Java side
-            // since it breaks returning the result in exec of synchronous
-            // plugins. Once we remove this ability, we can remove this comment.
             if (messages == '*') {
+                messagesFromNative.shift();
                 window.setTimeout(pollOnce, 0);
-                continue;
+                return;
             }
 
             var spaceIdx = messages.indexOf(' ');
             var msgLen = +messages.slice(0, spaceIdx);
             var message = messages.substr(spaceIdx + 1, msgLen);
             messages = messages.slice(spaceIdx + msgLen + 1);
-            // Put the remaining messages back into queue in case an exec()
-            // is made by the callback.
+            processMessage(message);
             if (messages) {
-                messagesFromNative.unshift(messages);
-            }
-
-            if (message) {
-                processMessage(message);
+                messagesFromNative[0] = messages;
+            } else {
+                messagesFromNative.shift();
             }
         }
     }

http://git-wip-us.apache.org/repos/asf/cordova-js/blob/e80f4afd/test/android/test.exec.js
----------------------------------------------------------------------
diff --git a/test/android/test.exec.js b/test/android/test.exec.js
index 3e185d9..7d6f2a7 100644
--- a/test/android/test.exec.js
+++ b/test/android/test.exec.js
@@ -56,39 +56,40 @@ describe('exec.processMessages', function () {
     }
 
     describe('exec', function() {
-        it('should return payload value when plugin is synchronous', function() {
-            var winSpy = jasmine.createSpy('win');
+        it('should process messages in order even when called recursively', function() {
+            var firstCallbackId = null;
+            var callCount = 0;
             nativeApi.exec.andCallFake(function(service, action, callbackId, argsJson) {
-                return createCallbackMessage(true, true, 1, callbackId, 't');
+                ++callCount;
+                if (callCount == 1) {
+                    firstCallbackId = callbackId;
+                    return '';
+                }
+                if (callCount == 2) {
+                    return createCallbackMessage(true, false, 1, firstCallbackId, 't') +
+                           createCallbackMessage(true, false, 1, callbackId, 'stwo');
+                }
+                return createCallbackMessage(true, false, 1, callbackId, 'sthree');
             });
 
-            var result = exec(winSpy, null, 'Service', 'action', []);
-            expect(winSpy).toHaveBeenCalledWith(true);
-            expect(result).toBe(true);
-        });
-        it('should return payload value when plugin is synchronous and no callbacks are used',
function() {
-            nativeApi.exec.andCallFake(function(service, action, callbackId, argsJson) {
-                return createCallbackMessage(true, true, 1, callbackId, 't');
-            });
+            var win2Called = false;
+            var winSpy3 = jasmine.createSpy('win3');
 
-            var result = exec(null, null, 'Service', 'action', []);
-            expect(result).toBe(true);
-        });
-        it('should return payload value when plugin is synchronous even when called recursively',
function() {
-            var winSpy = jasmine.createSpy('win');
-            nativeApi.exec.andCallFake(function(service, action, callbackId, argsJson) {
-                return createCallbackMessage(true, true, 1, callbackId, 't');
-            });
-
-            function firstWin(value) {
+            function win1(value) {
                 expect(value).toBe(true);
-                var result = exec(winSpy, null, 'Service', 'action', []);
-                expect(result).toBe(true);
+                exec(winSpy3, null, 'Service', 'action', []);
+                expect(win2Called).toBe(false, 'win1 should finish before win2 starts');
+            }
+
+            function win2(value) {
+                win2Called = true;
+                expect(value).toBe('two');
+                expect(winSpy3).not.toHaveBeenCalled();
             }
 
-            var result2 = exec(firstWin, null, 'Service', 'action', []);
-            expect(winSpy).toHaveBeenCalledWith(true);
-            expect(result2).toBe(true);
+            exec(win1, null, 'Service', 'action', []);
+            exec(win2, null, 'Service', 'action', []);
+            expect(winSpy3).toHaveBeenCalledWith('three');
         });
     });
 


Mime
View raw message