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.

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?
+                                        // TODO(agrieve): Should this use popAll() instead?
-                                		js = jsMessageQueue.pop();
+                                		payload = jsMessageQueue.popAndEncode();
-                                	    if (js != null) {
+                                	    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 {

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

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);
     }
 
     /**

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 */);
+        appView.pluginManager.exec(service, action, callbackId, jsonArgs, true /* async */);
-        if (r != null) {
-            String callbackString = r.toCallbackString(callbackId);
-            appView.sendJavascript(callbackString);
-        }
 	}    
 	
     /**

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);
         }
     }
 

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 */);
+        pluginManager.exec(service, action, callbackId, arguments, true /* async */);
-        return r == null ? "" : r.getJSONString();
+        // 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.
+        // TODO(agrieve): Should this use popAll() instead?
-        return jsMessageQueue.pop();
+        return jsMessageQueue.popAndEncode();
     }
 }

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 {
         }
     }
 
-    /**
+    public String popAndEncode() {
-     * Removes and returns the last statement in the queue.
-     * Returns null if the queue is empty.
-     */
-    public String pop() {
         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 (queue.isEmpty()) {
-            if (length == 0) {
                 return null;
             }
-            StringBuffer sb = new StringBuffer();
+            int totalMessageLen = 0;
-            // Wrap each statement in a try/finally so that if one throws it does 
-            // not affect the next.
-            int i = 0;
             for (String message : queue) {
-                if (++i == length) {
+                totalMessageLen += message.length();
-                    sb.append(message);
-                } else {
-                    sb.append("try{")
-                      .append(message)
-                      .append("}finally{");
-                }
             }
-            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);

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.
+     * sendPluginResult() should be used instead where possible.
-     *
-     * @param statement
      */
     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);
     }
 }

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);
+                                app.sendPluginResult(cr, callbackId);
-                                if (callbackString != null) {
-                                    app.sendJavascript(callbackString);
-                                }
                             } 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);
     }
 
     /**

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(status, PluginResult.StatusMessages[status.ordinal()]);
-        this.message = "\"" + PluginResult.StatusMessages[this.status] + "\"";
     }
 
     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",