From ae9047a70854d60a2fdd476aa8241067b8a17bd6 Mon Sep 17 00:00:00 2001 From: Andrew Grieve Date: Fri, 7 Sep 2012 15:19:24 -0400 Subject: [PATCH] Refactor how PluginResults are sent to JS. There is now a sendPluginResult() as well as a sendJavascript() on CordovaWebview. sendPluginResult() sends the result so that it can be parsed without using eval(), when the active bridge allows it. --- .../org/apache/cordova/CallbackServer.java | 12 +- .../apache/cordova/CordovaChromeClient.java | 6 +- .../org/apache/cordova/CordovaWebView.java | 12 +- .../apache/cordova/CordovaWebViewClient.java | 6 +- .../src/org/apache/cordova/DroidGap.java | 2 +- .../src/org/apache/cordova/ExposedJsApi.java | 9 +- .../cordova/NativeToJsMessageQueue.java | 141 ++++++++++++++---- .../src/org/apache/cordova/api/Plugin.java | 23 +-- .../org/apache/cordova/api/PluginManager.java | 19 +-- .../org/apache/cordova/api/PluginResult.java | 55 +++++-- 10 files changed, 203 insertions(+), 82 deletions(-) diff --git a/framework/src/org/apache/cordova/CallbackServer.java b/framework/src/org/apache/cordova/CallbackServer.java index d0296a30..7cefe3b5 100755 --- a/framework/src/org/apache/cordova/CallbackServer.java +++ b/framework/src/org/apache/cordova/CallbackServer.java @@ -207,16 +207,16 @@ public class CallbackServer implements Runnable { // Must have security token if ((requestParts.length == 3) && (requestParts[1].substring(1).equals(this.token))) { //Log.d(LOG_TAG, "CallbackServer -- Processing GET request"); - String js = null; + String payload = null; // Wait until there is some data to send, or send empty data every 10 sec // to prevent XHR timeout on the client synchronized (this) { while (this.active) { if (jsMessageQueue != null) { - // TODO(agrieve): Should this use popAll() instead? - js = jsMessageQueue.pop(); - if (js != null) { + // TODO(agrieve): Should this use popAll() instead? + payload = jsMessageQueue.popAndEncode(); + if (payload != null) { break; } } @@ -233,14 +233,14 @@ public class CallbackServer implements Runnable { if (this.active) { // If no data, then send 404 back to client before it times out - if (js == null) { + if (payload == null) { //Log.d(LOG_TAG, "CallbackServer -- sending data 0"); response = "HTTP/1.1 404 NO DATA\r\n\r\n "; // need to send content otherwise some Android devices fail, so send space } else { //Log.d(LOG_TAG, "CallbackServer -- sending item"); response = "HTTP/1.1 200 OK\r\n\r\n"; - response += encode(js, "UTF-8"); + response += encode(payload, "UTF-8"); } } else { diff --git a/framework/src/org/apache/cordova/CordovaChromeClient.java b/framework/src/org/apache/cordova/CordovaChromeClient.java index 747265a9..398286d2 100755 --- a/framework/src/org/apache/cordova/CordovaChromeClient.java +++ b/framework/src/org/apache/cordova/CordovaChromeClient.java @@ -202,7 +202,8 @@ public class CordovaChromeClient extends WebChromeClient { String service = array.getString(0); String action = array.getString(1); String callbackId = array.getString(2); - result.confirm(this.appView.exposedJsApi.exec(service, action, callbackId, message)); + String r = this.appView.exposedJsApi.exec(service, action, callbackId, message); + result.confirm(r == null ? "" : r); } catch (JSONException e) { e.printStackTrace(); } @@ -216,7 +217,8 @@ public class CordovaChromeClient extends WebChromeClient { // Polling for JavaScript messages else if (reqOk && defaultValue != null && defaultValue.equals("gap_poll:")) { - result.confirm(this.appView.exposedJsApi.retrieveJsMessages()); + String r = this.appView.exposedJsApi.retrieveJsMessages(); + result.confirm(r == null ? "" : r); } // Do NO-OP so older code doesn't display dialog diff --git a/framework/src/org/apache/cordova/CordovaWebView.java b/framework/src/org/apache/cordova/CordovaWebView.java index 53640b8d..ddae1dda 100755 --- a/framework/src/org/apache/cordova/CordovaWebView.java +++ b/framework/src/org/apache/cordova/CordovaWebView.java @@ -517,7 +517,17 @@ public class CordovaWebView extends WebView { * @param message */ public void sendJavascript(String statement) { - this.jsMessageQueue.add(statement); + this.jsMessageQueue.addJavaScript(statement); + } + + /** + * Send a plugin result back to JavaScript. + * (This is a convenience method) + * + * @param message + */ + public void sendPluginResult(PluginResult result, String callbackId) { + this.jsMessageQueue.addPluginResult(result, callbackId); } /** diff --git a/framework/src/org/apache/cordova/CordovaWebViewClient.java b/framework/src/org/apache/cordova/CordovaWebViewClient.java index 75d3590c..8229cf7a 100755 --- a/framework/src/org/apache/cordova/CordovaWebViewClient.java +++ b/framework/src/org/apache/cordova/CordovaWebViewClient.java @@ -110,11 +110,7 @@ public class CordovaWebViewClient extends WebViewClient { String action = url.substring(idx2 + 1, idx3); String callbackId = url.substring(idx3 + 1, idx4); String jsonArgs = url.substring(idx4 + 1); - PluginResult r = appView.pluginManager.exec(service, action, callbackId, jsonArgs, true /* async */); - if (r != null) { - String callbackString = r.toCallbackString(callbackId); - appView.sendJavascript(callbackString); - } + appView.pluginManager.exec(service, action, callbackId, jsonArgs, true /* async */); } /** diff --git a/framework/src/org/apache/cordova/DroidGap.java b/framework/src/org/apache/cordova/DroidGap.java index 0fe5b8ab..b36320fa 100755 --- a/framework/src/org/apache/cordova/DroidGap.java +++ b/framework/src/org/apache/cordova/DroidGap.java @@ -718,7 +718,7 @@ public class DroidGap extends Activity implements CordovaInterface { */ public void sendJavascript(String statement) { if (this.appView != null) { - this.appView.jsMessageQueue.add(statement); + this.appView.jsMessageQueue.addJavaScript(statement); } } diff --git a/framework/src/org/apache/cordova/ExposedJsApi.java b/framework/src/org/apache/cordova/ExposedJsApi.java index 92aff9af..461f3201 100755 --- a/framework/src/org/apache/cordova/ExposedJsApi.java +++ b/framework/src/org/apache/cordova/ExposedJsApi.java @@ -38,8 +38,9 @@ import org.json.JSONException; } public String exec(String service, String action, String callbackId, String arguments) throws JSONException { - PluginResult r = pluginManager.exec(service, action, callbackId, arguments, true /* async */); - return r == null ? "" : r.getJSONString(); + pluginManager.exec(service, action, callbackId, arguments, true /* async */); + // TODO(agrieve): Should this use popAll() instead? + return jsMessageQueue.popAndEncode(); } public void setNativeToJsBridgeMode(int value) { @@ -47,7 +48,7 @@ import org.json.JSONException; } public String retrieveJsMessages() { - // TODO(agrieve): Use popAll() here. - return jsMessageQueue.pop(); + // TODO(agrieve): Should this use popAll() instead? + return jsMessageQueue.popAndEncode(); } } diff --git a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java index 703baf14..08cadd3a 100755 --- a/framework/src/org/apache/cordova/NativeToJsMessageQueue.java +++ b/framework/src/org/apache/cordova/NativeToJsMessageQueue.java @@ -25,6 +25,8 @@ import java.util.ArrayList; import java.util.LinkedList; import org.apache.cordova.api.CordovaInterface; +import org.apache.cordova.api.PluginResult; +import org.json.JSONObject; import android.os.Message; import android.util.Log; @@ -44,6 +46,12 @@ public class NativeToJsMessageQueue { */ private int activeListenerIndex; + /** + * When true, the active listener is not fired upon enqueue. When set to false, + * the active listener will be fired if the queue is non-empty. + */ + private boolean paused; + /** * The list of JavaScript statements to be sent to JavaScript. */ @@ -99,60 +107,133 @@ public class NativeToJsMessageQueue { } } - /** - * Removes and returns the last statement in the queue. - * Returns null if the queue is empty. - */ - public String pop() { + public String popAndEncode() { synchronized (this) { if (queue.isEmpty()) { return null; } - return queue.remove(0); - } + String message = queue.removeFirst(); + StringBuffer sb = new StringBuffer(message.length() + 8) + .append(message.length()) + .append(' ') + .append(message); + return sb.toString(); + } } - + /** - * Combines and returns all statements. Clears the queue. + * Combines and returns all messages combined into a single string. + * Clears the queue. * Returns null if the queue is empty. */ - public String popAll() { + public String popAllAndEncode() { synchronized (this) { - int length = queue.size(); - if (length == 0) { + if (queue.isEmpty()) { return null; } - StringBuffer sb = new StringBuffer(); - // Wrap each statement in a try/finally so that if one throws it does - // not affect the next. - int i = 0; + int totalMessageLen = 0; for (String message : queue) { - if (++i == length) { - sb.append(message); - } else { - sb.append("try{") - .append(message) - .append("}finally{"); - } + totalMessageLen += message.length(); } - for ( i = 1; i < length; ++i) { - sb.append('}'); + + StringBuffer sb = new StringBuffer(totalMessageLen + 8 * queue.size()); + for (String message : queue) { + sb.append(message.length()) + .append(' ') + .append(message); } queue.clear(); return sb.toString(); } - } + } + + private String popAllAndEncodeAsJs() { + String ret = popAllAndEncode(); + if (ret != null) { + ret = "cordova.require('cordova/exec').processMessages(\"" + JSONObject.quote(ret) + "\")"; + } + return ret; + } + + private String encodePluginResult(PluginResult result, String callbackId) { + int status = result.getStatus(); + boolean noResult = status == PluginResult.Status.NO_RESULT.ordinal(); + boolean resultOk = status == PluginResult.Status.OK.ordinal(); + boolean keepCallback = result.getKeepCallback(); + if (noResult && keepCallback) { + return null; + } + + StringBuilder sb = new StringBuilder(result.getMessage().length() + 50); + sb.append((noResult || resultOk) ? 'S' : 'F') + .append(keepCallback ? '1' : '0') + .append(status) + .append(' ') + .append(callbackId) + .append(' '); + switch (result.getMessageType()) { + case PluginResult.MESSAGE_TYPE_BOOLEAN: + sb.append(result.getMessage().charAt(0)); // t or f. + break; + case PluginResult.MESSAGE_TYPE_NUMBER: // n + sb.append('n') + .append(result.getMessage()); + break; + case PluginResult.MESSAGE_TYPE_STRING: // s + sb.append('s') + .append(result.getStrMessage()); + break; + case PluginResult.MESSAGE_TYPE_JSON: + default: + sb.append(result.getMessage()); // [ or { + } + return sb.toString(); + } /** * Add a JavaScript statement to the list. */ - public void add(String statement) { + public void addJavaScript(String statement) { + enqueueMessage("J" + statement); + } + + /** + * Add a JavaScript statement to the list. + */ + public void addPluginResult(PluginResult result, String callbackId) { + String message = encodePluginResult(result, callbackId); + if (message != null) { + enqueueMessage(message); + } + } + + private void enqueueMessage(String encodedMessage) { synchronized (this) { - queue.add(statement); + queue.add(encodedMessage); if (registeredListeners[activeListenerIndex] != null) { registeredListeners[activeListenerIndex].onNativeToJsMessageAvailable(); } + } + } + + public void setPaused(boolean value) { + if (paused && value) { + // This should never happen. If a use-case for it comes up, we should + // change pause to be a counter. + Log.e(LOG_TAG, "nested call to setPaused detected.", new Throwable()); } + paused = value; + if (!value) { + synchronized (this) { + if (!queue.isEmpty() && registeredListeners[activeListenerIndex] != null) { + registeredListeners[activeListenerIndex].onNativeToJsMessageAvailable(); + } + } + } + } + + public boolean getPaused() { + return paused; } private interface BridgeMode { @@ -171,7 +252,7 @@ public class NativeToJsMessageQueue { /** Uses webView.loadUrl("javascript:") to execute messages. */ private class LoadUrlBridgeMode implements BridgeMode { public void onNativeToJsMessageAvailable() { - webView.loadUrlNow("javascript:" + popAll()); + webView.loadUrlNow("javascript:" + popAllAndEncodeAsJs()); } } @@ -245,7 +326,7 @@ public class NativeToJsMessageQueue { } // webViewCore is lazily initialized, and so may not be available right away. if (sendMessageMethod != null) { - String js = popAll(); + String js = popAllAndEncodeAsJs(); Message execJsMessage = Message.obtain(null, EXECUTE_JS, js); try { sendMessageMethod.invoke(webViewCore, execJsMessage); diff --git a/framework/src/org/apache/cordova/api/Plugin.java b/framework/src/org/apache/cordova/api/Plugin.java index c7c2be79..84b67e00 100755 --- a/framework/src/org/apache/cordova/api/Plugin.java +++ b/framework/src/org/apache/cordova/api/Plugin.java @@ -139,14 +139,19 @@ public abstract class Plugin implements IPlugin { /** * Send generic JavaScript statement back to JavaScript. - * success(...) and error(...) should be used instead where possible. - * - * @param statement + * sendPluginResult() should be used instead where possible. */ public void sendJavascript(String statement) { this.webView.sendJavascript(statement); } + /** + * Send generic JavaScript statement back to JavaScript. + */ + public void sendPluginResult(PluginResult pluginResult, String callbackId) { + this.webView.sendPluginResult(pluginResult, callbackId); + } + /** * Call the JavaScript success callback for this plugin. * @@ -158,7 +163,7 @@ public abstract class Plugin implements IPlugin { * @param callbackId The callback id used when calling back into JavaScript. */ public void success(PluginResult pluginResult, String callbackId) { - this.webView.sendJavascript(pluginResult.toSuccessCallbackString(callbackId)); + this.webView.sendPluginResult(pluginResult, callbackId); } /** @@ -168,7 +173,7 @@ public abstract class Plugin implements IPlugin { * @param callbackId The callback id used when calling back into JavaScript. */ public void success(JSONObject message, String callbackId) { - this.webView.sendJavascript(new PluginResult(PluginResult.Status.OK, message).toSuccessCallbackString(callbackId)); + this.webView.sendPluginResult(new PluginResult(PluginResult.Status.OK, message), callbackId); } /** @@ -178,7 +183,7 @@ public abstract class Plugin implements IPlugin { * @param callbackId The callback id used when calling back into JavaScript. */ public void success(String message, String callbackId) { - this.webView.sendJavascript(new PluginResult(PluginResult.Status.OK, message).toSuccessCallbackString(callbackId)); + this.webView.sendPluginResult(new PluginResult(PluginResult.Status.OK, message), callbackId); } /** @@ -188,7 +193,7 @@ public abstract class Plugin implements IPlugin { * @param callbackId The callback id used when calling back into JavaScript. */ public void error(PluginResult pluginResult, String callbackId) { - this.webView.sendJavascript(pluginResult.toErrorCallbackString(callbackId)); + this.webView.sendPluginResult(pluginResult, callbackId); } /** @@ -198,7 +203,7 @@ public abstract class Plugin implements IPlugin { * @param callbackId The callback id used when calling back into JavaScript. */ public void error(JSONObject message, String callbackId) { - this.webView.sendJavascript(new PluginResult(PluginResult.Status.ERROR, message).toErrorCallbackString(callbackId)); + this.webView.sendPluginResult(new PluginResult(PluginResult.Status.ERROR, message), callbackId); } /** @@ -208,6 +213,6 @@ public abstract class Plugin implements IPlugin { * @param callbackId The callback id used when calling back into JavaScript. */ public void error(String message, String callbackId) { - this.webView.sendJavascript(new PluginResult(PluginResult.Status.ERROR, message).toErrorCallbackString(callbackId)); + this.webView.sendPluginResult(new PluginResult(PluginResult.Status.ERROR, message), callbackId); } } diff --git a/framework/src/org/apache/cordova/api/PluginManager.java b/framework/src/org/apache/cordova/api/PluginManager.java index 0a09027e..99b8aeb8 100755 --- a/framework/src/org/apache/cordova/api/PluginManager.java +++ b/framework/src/org/apache/cordova/api/PluginManager.java @@ -210,10 +210,8 @@ public class PluginManager { * @param async Boolean indicating whether the calling JavaScript code is expecting an * immediate return value. If true, either Cordova.callbackSuccess(...) or * Cordova.callbackError(...) is called once the plugin code has executed. - * - * @return PluginResult to send to the page, or null if no response is ready yet. */ - public PluginResult exec(final String service, final String action, final String callbackId, final String jsonArgs, final boolean async) { + public void exec(final String service, final String action, final String callbackId, final String jsonArgs, final boolean async) { PluginResult cr = null; boolean runAsync = async; try { @@ -229,25 +227,22 @@ public class PluginManager { try { // Call execute on the plugin so that it can do it's thing PluginResult cr = plugin.execute(action, args, callbackId); - String callbackString = cr.toCallbackString(callbackId); - if (callbackString != null) { - app.sendJavascript(callbackString); - } + app.sendPluginResult(cr, callbackId); } catch (Exception e) { PluginResult cr = new PluginResult(PluginResult.Status.ERROR, e.getMessage()); - app.sendJavascript(cr.toErrorCallbackString(callbackId)); + app.sendPluginResult(cr, callbackId); } } }); thread.start(); - return null; + return; } else { // Call execute on the plugin so that it can do it's thing cr = plugin.execute(action, args, callbackId); // If no result to be sent and keeping callback, then no need to sent back to JavaScript if ((cr.getStatus() == PluginResult.Status.NO_RESULT.ordinal()) && cr.getKeepCallback()) { - return null; + return; } } } @@ -260,12 +255,12 @@ public class PluginManager { if (cr == null) { cr = new PluginResult(PluginResult.Status.CLASS_NOT_FOUND_EXCEPTION); } - app.sendJavascript(cr.toErrorCallbackString(callbackId)); + app.sendPluginResult(cr, callbackId); } if (cr == null) { cr = new PluginResult(PluginResult.Status.NO_RESULT); } - return cr; + app.sendPluginResult(cr, callbackId); } /** diff --git a/framework/src/org/apache/cordova/api/PluginResult.java b/framework/src/org/apache/cordova/api/PluginResult.java index eacf6d5c..28c7b980 100755 --- a/framework/src/org/apache/cordova/api/PluginResult.java +++ b/framework/src/org/apache/cordova/api/PluginResult.java @@ -23,43 +23,49 @@ import org.json.JSONObject; public class PluginResult { private final int status; - private final String message; + private final int messageType; private boolean keepCallback = false; - + private String strMessage; + private String encodedMessage; public PluginResult(Status status) { - this.status = status.ordinal(); - this.message = "\"" + PluginResult.StatusMessages[this.status] + "\""; + this(status, PluginResult.StatusMessages[status.ordinal()]); } public PluginResult(Status status, String message) { this.status = status.ordinal(); - this.message = JSONObject.quote(message); + this.messageType = MESSAGE_TYPE_STRING; + this.strMessage = message; } public PluginResult(Status status, JSONArray message) { this.status = status.ordinal(); - this.message = message.toString(); + this.messageType = MESSAGE_TYPE_JSON; + encodedMessage = message.toString(); } public PluginResult(Status status, JSONObject message) { this.status = status.ordinal(); - this.message = message.toString(); + this.messageType = MESSAGE_TYPE_JSON; + encodedMessage = message.toString(); } public PluginResult(Status status, int i) { this.status = status.ordinal(); - this.message = ""+i; + this.messageType = MESSAGE_TYPE_NUMBER; + this.encodedMessage = ""+i; } public PluginResult(Status status, float f) { this.status = status.ordinal(); - this.message = ""+f; + this.messageType = MESSAGE_TYPE_NUMBER; + this.encodedMessage = ""+f; } public PluginResult(Status status, boolean b) { this.status = status.ordinal(); - this.message = ""+b; + this.messageType = MESSAGE_TYPE_BOOLEAN; + this.encodedMessage = Boolean.toString(b); } public void setKeepCallback(boolean b) { @@ -70,18 +76,35 @@ public class PluginResult { return status; } + public int getMessageType() { + return messageType; + } + public String getMessage() { - return message; + if (encodedMessage == null) { + encodedMessage = JSONObject.quote(strMessage); + } + return encodedMessage; + } + + /** + * If messageType == MESSAGE_TYPE_STRING, then returns the message string. + * Otherwise, returns null. + */ + public String getStrMessage() { + return strMessage; } public boolean getKeepCallback() { return this.keepCallback; } + @Deprecated // Use sendPluginResult instead of sendJavascript. public String getJSONString() { - return "{\"status\":" + this.status + ",\"message\":" + this.message + ",\"keepCallback\":" + this.keepCallback + "}"; + return "{\"status\":" + this.status + ",\"message\":" + this.getMessage() + ",\"keepCallback\":" + this.keepCallback + "}"; } + @Deprecated // Use sendPluginResult instead of sendJavascript. public String toCallbackString(String callbackId) { // If no result to be sent and keeping callback, then no need to sent back to JavaScript if ((status == PluginResult.Status.NO_RESULT.ordinal()) && keepCallback) { @@ -95,14 +118,22 @@ public class PluginResult { return toErrorCallbackString(callbackId); } + + @Deprecated // Use sendPluginResult instead of sendJavascript. public String toSuccessCallbackString(String callbackId) { return "cordova.callbackSuccess('"+callbackId+"',"+this.getJSONString()+");"; } + @Deprecated // Use sendPluginResult instead of sendJavascript. public String toErrorCallbackString(String callbackId) { return "cordova.callbackError('"+callbackId+"', " + this.getJSONString()+ ");"; } + public static final int MESSAGE_TYPE_STRING = 1; + public static final int MESSAGE_TYPE_JSON = 2; + public static final int MESSAGE_TYPE_NUMBER = 3; + public static final int MESSAGE_TYPE_BOOLEAN = 4; + public static String[] StatusMessages = new String[] { "No result", "OK",