Return-Path: X-Original-To: apmail-cordova-commits-archive@www.apache.org Delivered-To: apmail-cordova-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 3F22010F4C for ; Tue, 13 Jan 2015 03:04:16 +0000 (UTC) Received: (qmail 27973 invoked by uid 500); 13 Jan 2015 03:04:18 -0000 Delivered-To: apmail-cordova-commits-archive@cordova.apache.org Received: (qmail 27947 invoked by uid 500); 13 Jan 2015 03:04:18 -0000 Mailing-List: contact commits-help@cordova.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Delivered-To: mailing list commits@cordova.apache.org Received: (qmail 27938 invoked by uid 99); 13 Jan 2015 03:04:18 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 13 Jan 2015 03:04:18 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id AD437A05570; Tue, 13 Jan 2015 03:04:17 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: agrieve@apache.org To: commits@cordova.apache.org Message-Id: <547e08ef78e64dc6bfaab54a168590ad@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: js commit: CB-8298 android: Execute exec callbacks within their own stack frames Date: Tue, 13 Jan 2015 03:04:17 +0000 (UTC) Repository: cordova-js Updated Branches: refs/heads/master 24ab68554 -> f6ec6b13c CB-8298 android: Execute exec callbacks within their own stack frames This allow us to remove a try/catch that was messing with remote inspector being able to effectively log exceptions in user callbacks Project: http://git-wip-us.apache.org/repos/asf/cordova-js/repo Commit: http://git-wip-us.apache.org/repos/asf/cordova-js/commit/f6ec6b13 Tree: http://git-wip-us.apache.org/repos/asf/cordova-js/tree/f6ec6b13 Diff: http://git-wip-us.apache.org/repos/asf/cordova-js/diff/f6ec6b13 Branch: refs/heads/master Commit: f6ec6b13c5c8c2de231740f20c73e16026188414 Parents: 24ab685 Author: Andrew Grieve Authored: Mon Jan 12 21:48:47 2015 -0500 Committer: Andrew Grieve Committed: Mon Jan 12 22:03:59 2015 -0500 ---------------------------------------------------------------------- src/android/exec.js | 97 ++++++++++++++++++++---------------------- test/android/test.exec.js | 87 +++++++++++++++++++++++++------------ 2 files changed, 106 insertions(+), 78 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cordova-js/blob/f6ec6b13/src/android/exec.js ---------------------------------------------------------------------- diff --git a/src/android/exec.js b/src/android/exec.js index 87cd52c..f0d395d 100644 --- a/src/android/exec.js +++ b/src/android/exec.js @@ -60,9 +60,13 @@ var cordova = require('cordova'), jsToNativeBridgeMode, // Set lazily. nativeToJsBridgeMode = nativeToJsModes.ONLINE_EVENT, pollEnabled = false, - messagesFromNative = [], bridgeSecret = -1; +var messagesFromNative = []; +var isProcessing = false; +var resolvedPromise = typeof Promise == 'undefined' ? null : Promise.resolve(); +var nextTick = resolvedPromise ? function(fn) { resolvedPromise.then(fn); } : function(fn) { setTimeout(fn) }; + function androidExec(success, fail, service, action, args) { if (bridgeSecret < 0) { // If we ever catch this firing, we'll need to queue up exec()s @@ -91,16 +95,17 @@ function androidExec(success, fail, service, action, args) { cordova.callbacks[callbackId] = {success:success, fail:fail}; } - var messages = nativeApiProvider.get().exec(bridgeSecret, service, action, callbackId, argsJson); + var msgs = nativeApiProvider.get().exec(bridgeSecret, service, action, callbackId, argsJson); // If argsJson was received by Java as null, try again with the PROMPT bridge mode. // This happens in rare circumstances, such as when certain Unicode characters are passed over the bridge on a Galaxy S2. See CB-2666. - if (jsToNativeBridgeMode == jsToNativeModes.JS_OBJECT && messages === "@Null arguments.") { + if (jsToNativeBridgeMode == jsToNativeModes.JS_OBJECT && msgs === "@Null arguments.") { androidExec.setJsToNativeBridgeMode(jsToNativeModes.PROMPT); androidExec(success, fail, service, action, args); androidExec.setJsToNativeBridgeMode(jsToNativeModes.JS_OBJECT); - return; - } else { - androidExec.processMessages(messages, true); + } else if (msgs) { + messagesFromNative.push(msgs); + // Always process async to avoid exceptions messing up stack. + nextTick(processMessages); } } @@ -119,8 +124,12 @@ function pollOnce(opt_fromOnlineEvent) { // We know there's nothing to retrieve, so no need to poll. return; } - var msg = nativeApiProvider.get().retrieveJsMessages(bridgeSecret, !!opt_fromOnlineEvent); - androidExec.processMessages(msg); + var msgs = nativeApiProvider.get().retrieveJsMessages(bridgeSecret, !!opt_fromOnlineEvent); + if (msgs) { + messagesFromNative.push(msgs); + // Process sync since we know we're already top-of-stack. + processMessages(); + } } function pollingTimerFunc() { @@ -213,63 +222,51 @@ function buildPayload(payload, message) { // Processes a single message, as encoded by NativeToJsMessageQueue.java. function processMessage(message) { - try { - var firstChar = message.charAt(0); - if (firstChar == 'J') { - eval(message.slice(1)); - } else if (firstChar == 'S' || firstChar == 'F') { - var success = firstChar == 'S'; - var keepCallback = message.charAt(1) == '1'; - var spaceIdx = message.indexOf(' ', 2); - var status = +message.slice(2, spaceIdx); - var nextSpaceIdx = message.indexOf(' ', spaceIdx + 1); - var callbackId = message.slice(spaceIdx + 1, nextSpaceIdx); - var payloadMessage = message.slice(nextSpaceIdx + 1); - var payload = []; - buildPayload(payload, payloadMessage); - cordova.callbackFromNative(callbackId, success, status, payload, keepCallback); - } else { - console.log("processMessage failed: invalid message: " + JSON.stringify(message)); - } - } catch (e) { - console.log("processMessage failed: Error: " + e); - console.log("processMessage failed: Stack: " + e.stack); - console.log("processMessage failed: Message: " + message); + var firstChar = message.charAt(0); + if (firstChar == 'J') { + // This is deprecated on the .java side. It doesn't work with CSP enabled. + eval(message.slice(1)); + } else if (firstChar == 'S' || firstChar == 'F') { + var success = firstChar == 'S'; + var keepCallback = message.charAt(1) == '1'; + var spaceIdx = message.indexOf(' ', 2); + var status = +message.slice(2, spaceIdx); + var nextSpaceIdx = message.indexOf(' ', spaceIdx + 1); + var callbackId = message.slice(spaceIdx + 1, nextSpaceIdx); + var payloadMessage = message.slice(nextSpaceIdx + 1); + var payload = []; + buildPayload(payload, payloadMessage); + cordova.callbackFromNative(callbackId, success, status, payload, keepCallback); + } else { + console.log("processMessage failed: invalid message: " + JSON.stringify(message)); } } -var isProcessing = false; - -// This is called from the NativeToJsMessageQueue.java. -androidExec.processMessages = function(messages, opt_useTimeout) { - if (messages) { - messagesFromNative.push(messages); - } +function processMessages() { // Check for the reentrant case. if (isProcessing) { return; } - if (opt_useTimeout) { - window.setTimeout(androidExec.processMessages, 0); + if (messagesFromNative.length === 0) { return; } isProcessing = true; try { - // TODO: add setImmediate polyfill and process only one message at a time. - while (messagesFromNative.length) { - var msg = popMessageFromQueue(); - // The Java side can send a * message to indicate that it - // still has messages waiting to be retrieved. - if (msg == '*' && messagesFromNative.length === 0) { - setTimeout(pollOnce, 0); - return; - } - processMessage(msg); + var msg = popMessageFromQueue(); + // The Java side can send a * message to indicate that it + // still has messages waiting to be retrieved. + if (msg == '*' && messagesFromNative.length === 0) { + nextTick(pollOnce); + return; } + processMessage(msg); } finally { isProcessing = false; + if (messagesFromNative.length > 0) { + nextTick(processMessages); + } } -}; +} function popMessageFromQueue() { var messageBatch = messagesFromNative.shift(); http://git-wip-us.apache.org/repos/asf/cordova-js/blob/f6ec6b13/test/android/test.exec.js ---------------------------------------------------------------------- diff --git a/test/android/test.exec.js b/test/android/test.exec.js index 551f161..6541f3c 100644 --- a/test/android/test.exec.js +++ b/test/android/test.exec.js @@ -125,64 +125,93 @@ describe('exec.processMessages', function () { afterEach(function() { cordova.callbackFromNative = origCallbackFromNative; }); + + function performExecAndReturn(messages) { + nativeApi.exec.andCallFake(function(secret, service, action, callbackId, argsJson) { + return messages; + }); + + var winSpy = jasmine.createSpy('win'); + + exec(null, null, 'Service', 'action', []); + waitsFor(function() { return callbackSpy.wasCalled }, 200); + } + it('should handle payloads of false', function() { var messages = createCallbackMessage(true, true, 1, 'id', 'f'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [false], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [false], true); + }); }); it('should handle payloads of true', function() { var messages = createCallbackMessage(true, true, 1, 'id', 't'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [true], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [true], true); + }); }); it('should handle payloads of null', function() { var messages = createCallbackMessage(true, true, 1, 'id', 'N'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [null], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [null], true); + }); }); it('should handle payloads of numbers', function() { var messages = createCallbackMessage(true, true, 1, 'id', 'n-3.3'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [-3.3], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [-3.3], true); + }); }); it('should handle payloads of strings', function() { var messages = createCallbackMessage(true, true, 1, 'id', 'sHello world'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, ['Hello world'], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, ['Hello world'], true); + }); }); it('should handle payloads of JSON objects', function() { var messages = createCallbackMessage(true, true, 1, 'id', '{"a":1}'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [{a:1}], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [{a:1}], true); + }); }); it('should handle payloads of JSON arrays', function() { var messages = createCallbackMessage(true, true, 1, 'id', '[1]'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [[1]], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [[1]], true); + }); }); it('should handle other callback opts', function() { var messages = createCallbackMessage(false, false, 3, 'id', 'sfoo'); - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', false, 3, ['foo'], false); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', false, 3, ['foo'], false); + }); }); it('should handle multiple messages', function() { var message1 = createCallbackMessage(false, false, 3, 'id', 'sfoo'); var message2 = createCallbackMessage(true, true, 1, 'id', 'f'); var messages = message1 + message2; - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', false, 3, ['foo'], false); - expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [false], true); + performExecAndReturn(messages); + runs(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', false, 3, ['foo'], false); + expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [false], true); + }); }); it('should poll for more messages when hitting an *', function() { var message1 = createCallbackMessage(false, false, 3, 'id', 'sfoo'); var message2 = createCallbackMessage(true, true, 1, 'id', 'f'); nativeApi.retrieveJsMessages.andCallFake(function() { + expect(callbackSpy).toHaveBeenCalledWith('id', false, 3, ['foo'], false); callbackSpy.reset(); return message2; }); - var messages = message1 + '*'; - exec.processMessages(messages); - expect(callbackSpy).toHaveBeenCalledWith('id', false, 3, ['foo'], false); + performExecAndReturn(message1 + '*'); waitsFor(function() { return nativeApi.retrieveJsMessages.wasCalled }, 500); runs(function() { expect(callbackSpy).toHaveBeenCalledWith('id', true, 1, [false], true); @@ -195,14 +224,16 @@ describe('exec.processMessages', function () { callbackSpy.andCallFake(function() { if (callbackSpy.calls.length == 1) { - exec.processMessages(message3); + performExecAndReturn(message3); } }); - exec.processMessages(message1 + message2); - expect(callbackSpy.argsForCall.length).toEqual(3); - expect(callbackSpy.argsForCall[0]).toEqual(['id', false, 3, ['call1'], false]); - expect(callbackSpy.argsForCall[1]).toEqual(['id', false, 3, ['call2'], false]); - expect(callbackSpy.argsForCall[2]).toEqual(['id', false, 3, ['call3'], false]); + performExecAndReturn(message1 + message2); + runs(function() { + expect(callbackSpy.argsForCall.length).toEqual(3); + expect(callbackSpy.argsForCall[0]).toEqual(['id', false, 3, ['call1'], false]); + expect(callbackSpy.argsForCall[1]).toEqual(['id', false, 3, ['call2'], false]); + expect(callbackSpy.argsForCall[2]).toEqual(['id', false, 3, ['call3'], false]); + }); }); }); }); --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org For additional commands, e-mail: commits-help@cordova.apache.org