Use callbacks directly as fields in connector object (#8888)

12 files changed, 193 insertions, 50 deletions

diff --git a/WebContent/statictestfiles/jsconnector.js b/WebContent/statictestfiles/jsconnector.js
index 1432c20698..e08c748aa6 100644
--- a/WebContent/statictestfiles/jsconnector.js
+++ b/WebContent/statictestfiles/jsconnector.js
@@ -5,6 +5,7 @@ window.com_vaadin_tests_components_javascriptcomponent_BasicJavaScriptComponent_
 	rootElement.innerHTML = 'Hello world!';
 	rootElement.onclick = function() {
 		connector.getRpcProxyFunction("com.vaadin.tests.components.javascriptcomponent.BasicJavaScriptComponent$ExampleClickRpc", "onClick")("message");
+		connector.onclick("another message");
 	}
 	connector.onStateChange = function() {
 		console.log('state change:', this.getState());
diff --git a/WebContent/statictestfiles/jsextension.js b/WebContent/statictestfiles/jsextension.js
index be551746ca..d824218560 100644
--- a/WebContent/statictestfiles/jsextension.js
+++ b/WebContent/statictestfiles/jsextension.js
@@ -1,4 +1,5 @@
 window.com_vaadin_tests_extensions_SimpleJavaScriptExtensionTest_SimpleJavascriptExtension = function() {
+	var self = this;
 	var state = this.getState();
 	var greetBack = this.getRpcProxyFunction('com.vaadin.tests.extensions.SimpleJavaScriptExtensionTest$SimpleJavaScriptExtensionServerRpc', 'greet');
 
@@ -10,4 +11,11 @@ window.com_vaadin_tests_extensions_SimpleJavaScriptExtensionTest_SimpleJavascrip
 			}
 		}
 	});
+	
+	this.greetToClient = function(greeting) {
+		var response = window.prompt(state.prefix + greeting);
+		if (response !== null) {
+			self.greetToServer(response);
+		}
+	} 
 }
\ No newline at end of file
diff --git a/src/com/vaadin/terminal/AbstractJavaScriptExtension.java b/src/com/vaadin/terminal/AbstractJavaScriptExtension.java
index 49f361f343..22c8dd4561 100644
--- a/src/com/vaadin/terminal/AbstractJavaScriptExtension.java
+++ b/src/com/vaadin/terminal/AbstractJavaScriptExtension.java
@@ -4,6 +4,7 @@
 
 package com.vaadin.terminal;
 
+import com.vaadin.terminal.gwt.client.JavaScriptExtensionState;
 import com.vaadin.ui.JavaScriptCallback;
 
 public class AbstractJavaScriptExtension extends AbstractExtension {
@@ -18,4 +19,9 @@ public class AbstractJavaScriptExtension extends AbstractExtension {
     protected void invokeCallback(String name, Object... arguments) {
         callbackHelper.invokeCallback(name, arguments);
     }
+
+    @Override
+    public JavaScriptExtensionState getState() {
+        return (JavaScriptExtensionState) super.getState();
+    }
 }
diff --git a/src/com/vaadin/terminal/JavaScriptCallbackHelper.java b/src/com/vaadin/terminal/JavaScriptCallbackHelper.java
index 5ea5e56255..c1da8b36ba 100644
--- a/src/com/vaadin/terminal/JavaScriptCallbackHelper.java
+++ b/src/com/vaadin/terminal/JavaScriptCallbackHelper.java
@@ -11,6 +11,7 @@ import java.util.Map;
 
 import com.vaadin.external.json.JSONArray;
 import com.vaadin.external.json.JSONException;
+import com.vaadin.terminal.gwt.client.JavaScriptConnectorHelper.JavaScriptConnectorState;
 import com.vaadin.tools.ReflectTools;
 import com.vaadin.ui.JavaScript.JavaScriptCallbackRpc;
 import com.vaadin.ui.JavaScriptCallback;
@@ -31,9 +32,19 @@ public class JavaScriptCallbackHelper {
     public void registerCallback(String functionName,
             JavaScriptCallback javaScriptCallback) {
         callbacks.put(functionName, javaScriptCallback);
+        JavaScriptConnectorState state = getConnectorState();
+        if (state.getCallbackNames().add(functionName)) {
+            connector.requestRepaint();
+        }
         ensureRpc();
     }
 
+    private JavaScriptConnectorState getConnectorState() {
+        JavaScriptConnectorState state = (JavaScriptConnectorState) connector
+                .getState();
+        return state;
+    }
+
     private void ensureRpc() {
         if (javascriptCallbackRpc == null) {
             javascriptCallbackRpc = new JavaScriptCallbackRpc() {
@@ -51,6 +62,12 @@ public class JavaScriptCallbackHelper {
     }
 
     public void invokeCallback(String name, Object... arguments) {
+        if (callbacks.containsKey(name)) {
+            throw new IllegalStateException(
+                    "Can't call callback "
+                            + name
+                            + " on the client because a callback with the same name is registered on the server.");
+        }
         JSONArray args = new JSONArray(Arrays.asList(arguments));
         connector.addMethodInvocationToQueue(
                 JavaScriptCallbackRpc.class.getName(), CALL_METHOD,
diff --git a/src/com/vaadin/terminal/gwt/client/JavaScriptConnectorHelper.java b/src/com/vaadin/terminal/gwt/client/JavaScriptConnectorHelper.java
index 2c7ca22157..d07cf0036c 100644
--- a/src/com/vaadin/terminal/gwt/client/JavaScriptConnectorHelper.java
+++ b/src/com/vaadin/terminal/gwt/client/JavaScriptConnectorHelper.java
@@ -5,14 +5,21 @@
 package com.vaadin.terminal.gwt.client;
 
 import java.util.ArrayList;
+import java.util.Set;
 
 import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.core.client.JsArray;
 import com.google.gwt.json.client.JSONArray;
 import com.vaadin.terminal.gwt.client.communication.MethodInvocation;
+import com.vaadin.terminal.gwt.client.communication.StateChangeEvent;
+import com.vaadin.terminal.gwt.client.communication.StateChangeEvent.StateChangeHandler;
 
 public class JavaScriptConnectorHelper {
 
+    public interface JavaScriptConnectorState {
+        public Set<String> getCallbackNames();
+    }
+
     private final ServerConnector connector;
     private final JavaScriptObject nativeState = JavaScriptObject
             .createObject();
@@ -23,6 +30,18 @@ public class JavaScriptConnectorHelper {
 
     public JavaScriptConnectorHelper(ServerConnector connector) {
         this.connector = connector;
+        connector.addStateChangeHandler(new StateChangeHandler() {
+            public void onStateChanged(StateChangeEvent stateChangeEvent) {
+                JavaScriptObject wrapper = getConnectorWrapper();
+
+                for (String callback : getConnectorState().getCallbackNames()) {
+                    ensureCallback(JavaScriptConnectorHelper.this, wrapper,
+                            callback);
+                }
+
+                fireNativeStateChange(wrapper);
+            }
+        });
     }
 
     public boolean init() {
@@ -78,10 +97,6 @@ public class JavaScriptConnectorHelper {
                 connector.getConnectorId());
     }
 
-    public void fireNativeStateChange() {
-        fireNativeStateChange(getConnectorWrapper());
-    }
-
     private static native void fireNativeStateChange(
             JavaScriptObject connectorWrapper)
     /*-{
@@ -106,21 +121,6 @@ public class JavaScriptConnectorHelper {
                         h.@com.vaadin.terminal.gwt.client.JavaScriptConnectorHelper::fireRpc(Ljava/lang/String;Ljava/lang/String;Lcom/google/gwt/core/client/JsArray;)(iface, method, arguments);
                     });
             },
-            'getCallback': function(name) {
-                return $entry(function() {
-                    var args = [name, Array.prototype.slice.call(arguments, 0)];
-                    var iface = "com.vaadin.ui.JavaScript$JavaScriptCallbackRpc";
-                    var method = "call";
-                    h.@com.vaadin.terminal.gwt.client.JavaScriptConnectorHelper::fireRpc(Ljava/lang/String;Ljava/lang/String;Lcom/google/gwt/core/client/JsArray;)(iface, method, args);
-                });
-            },
-            'registerCallback': function(name, callback) {
-                //TODO maintain separate map
-                if (!registeredRpc[name]) {
-                    registeredRpc[name] = [];
-                }
-                registeredRpc[name].push(callback);
-            },
             'registerRpc': function(iface, rpcHandler) {
                 if (!registeredRpc[iface]) {
                     registeredRpc[iface] = [];
@@ -142,6 +142,14 @@ public class JavaScriptConnectorHelper {
                         parameters), true);
     }
 
+    private void fireCallback(String name, JsArray<JavaScriptObject> arguments) {
+        MethodInvocation invocation = new MethodInvocation(
+                connector.getConnectorId(),
+                "com.vaadin.ui.JavaScript$JavaScriptCallbackRpc", "call",
+                new Object[] { name, new JSONArray(arguments) });
+        connector.getConnection().addMethodInvocationToQueue(invocation, true);
+    }
+
     public void setNativeState(JavaScriptObject state) {
         updateNativeState(nativeState, state);
     }
@@ -176,8 +184,11 @@ public class JavaScriptConnectorHelper {
         if ("com.vaadin.ui.JavaScript$JavaScriptCallbackRpc".equals(invocation
                 .getInterfaceName())
                 && "call".equals(invocation.getMethodName())) {
-            invokeJsRpc(rpcMap, parametersJson.get(0).isString().stringValue(),
-                    null, parametersJson.get(1).isArray().getJavaScriptObject());
+            String callbackName = parametersJson.get(0).isString()
+                    .stringValue();
+            JavaScriptObject arguments = parametersJson.get(1).isArray()
+                    .getJavaScriptObject();
+            invokeCallback(getConnectorWrapper(), callbackName, arguments);
         } else {
             invokeJsRpc(rpcMap, invocation.getInterfaceName(),
                     invocation.getMethodName(),
@@ -185,6 +196,12 @@ public class JavaScriptConnectorHelper {
         }
     }
 
+    private static native void invokeCallback(JavaScriptObject connector,
+            String name, JavaScriptObject arguments)
+    /*-{
+        connector[name](arguments);
+    }-*/;
+
     private static native void invokeJsRpc(JavaScriptObject rpcMap,
             String interfaceName, String methodName, JavaScriptObject parameters)
     /*-{
@@ -194,12 +211,21 @@ public class JavaScriptConnectorHelper {
         }
         for(var i = 0; i < targets.length; i++) {
             var target = targets[i];
-            if (methodName === null && typeof target === 'function') {
-                target.apply($wnd, parameters);
-            } else {
-                target[methodName].apply(target, parameters);
-            }
+            target[methodName].apply(target, parameters);
         }
     }-*/;
 
+    private static native void ensureCallback(JavaScriptConnectorHelper h,
+            JavaScriptObject connector, String name)
+    /*-{
+        connector[name] = $entry(function() {
+            var args = Array.prototype.slice.call(arguments, 0);
+            h.@com.vaadin.terminal.gwt.client.JavaScriptConnectorHelper::fireCallback(Ljava/lang/String;Lcom/google/gwt/core/client/JsArray;)(name, args);
+        });
+    }-*/;
+
+    private JavaScriptConnectorState getConnectorState() {
+        return (JavaScriptConnectorState) connector.getState();
+    }
+
 }
diff --git a/src/com/vaadin/terminal/gwt/client/JavaScriptExtension.java b/src/com/vaadin/terminal/gwt/client/JavaScriptExtension.java
index 121208d754..47aa7eab28 100644
--- a/src/com/vaadin/terminal/gwt/client/JavaScriptExtension.java
+++ b/src/com/vaadin/terminal/gwt/client/JavaScriptExtension.java
@@ -6,7 +6,6 @@ package com.vaadin.terminal.gwt.client;
 
 import com.vaadin.terminal.AbstractJavaScriptExtension;
 import com.vaadin.terminal.gwt.client.communication.HasJavaScriptConnectorHelper;
-import com.vaadin.terminal.gwt.client.communication.StateChangeEvent;
 import com.vaadin.terminal.gwt.client.extensions.AbstractExtensionConnector;
 import com.vaadin.terminal.gwt.client.ui.Connect;
 
@@ -21,13 +20,12 @@ public class JavaScriptExtension extends AbstractExtensionConnector implements
         helper.init();
     }
 
-    @Override
-    public void onStateChanged(StateChangeEvent stateChangeEvent) {
-        super.onStateChanged(stateChangeEvent);
-        helper.fireNativeStateChange();
-    }
-
     public JavaScriptConnectorHelper getJavascriptConnectorHelper() {
         return helper;
     }
+
+    @Override
+    public JavaScriptExtensionState getState() {
+        return (JavaScriptExtensionState) super.getState();
+    }
 }
diff --git a/src/com/vaadin/terminal/gwt/client/JavaScriptExtensionState.java b/src/com/vaadin/terminal/gwt/client/JavaScriptExtensionState.java
new file mode 100644
index 0000000000..fecf24d450
--- /dev/null
+++ b/src/com/vaadin/terminal/gwt/client/JavaScriptExtensionState.java
@@ -0,0 +1,26 @@
+/* 
+@VaadinApache2LicenseForJavaFiles@
+ */
+
+package com.vaadin.terminal.gwt.client;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import com.vaadin.terminal.gwt.client.JavaScriptConnectorHelper.JavaScriptConnectorState;
+import com.vaadin.terminal.gwt.client.communication.SharedState;
+
+public class JavaScriptExtensionState extends SharedState implements
+        JavaScriptConnectorState {
+
+    private Set<String> callbackNames = new HashSet<String>();
+
+    public Set<String> getCallbackNames() {
+        return callbackNames;
+    }
+
+    public void setCallbackNames(Set<String> callbackNames) {
+        this.callbackNames = callbackNames;
+    }
+
+}
diff --git a/src/com/vaadin/terminal/gwt/client/ui/JavaScriptComponentConnector.java b/src/com/vaadin/terminal/gwt/client/ui/JavaScriptComponentConnector.java
index c8b22a4239..ecab58bb9a 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/JavaScriptComponentConnector.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/JavaScriptComponentConnector.java
@@ -7,7 +7,6 @@ import com.google.gwt.core.client.JavaScriptObject;
 import com.google.gwt.user.client.Element;
 import com.vaadin.terminal.gwt.client.JavaScriptConnectorHelper;
 import com.vaadin.terminal.gwt.client.communication.HasJavaScriptConnectorHelper;
-import com.vaadin.terminal.gwt.client.communication.StateChangeEvent;
 import com.vaadin.ui.AbstractJavaScriptComponent;
 
 @Connect(AbstractJavaScriptComponent.class)
@@ -35,12 +34,6 @@ public class JavaScriptComponentConnector extends AbstractComponentConnector
         helper.init();
     }
 
-    @Override
-    public void onStateChanged(StateChangeEvent stateChangeEvent) {
-        super.onStateChanged(stateChangeEvent);
-        helper.fireNativeStateChange();
-    }
-
     private static native void addGetWidgetElement(
             JavaScriptObject connectorWrapper, Element element)
     /*-{
@@ -57,4 +50,9 @@ public class JavaScriptComponentConnector extends AbstractComponentConnector
     public JavaScriptConnectorHelper getJavascriptConnectorHelper() {
         return helper;
     }
+
+    @Override
+    public JavaScriptComponentState getState() {
+        return (JavaScriptComponentState) super.getState();
+    }
 }
diff --git a/src/com/vaadin/terminal/gwt/client/ui/JavaScriptComponentState.java b/src/com/vaadin/terminal/gwt/client/ui/JavaScriptComponentState.java
new file mode 100644
index 0000000000..bc8ed08bba
--- /dev/null
+++ b/src/com/vaadin/terminal/gwt/client/ui/JavaScriptComponentState.java
@@ -0,0 +1,26 @@
+/* 
+@VaadinApache2LicenseForJavaFiles@
+ */
+
+package com.vaadin.terminal.gwt.client.ui;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import com.vaadin.terminal.gwt.client.ComponentState;
+import com.vaadin.terminal.gwt.client.JavaScriptConnectorHelper.JavaScriptConnectorState;
+
+public class JavaScriptComponentState extends ComponentState implements
+        JavaScriptConnectorState {
+
+    private Set<String> callbackNames = new HashSet<String>();
+
+    public Set<String> getCallbackNames() {
+        return callbackNames;
+    }
+
+    public void setCallbackNames(Set<String> callbackNames) {
+        this.callbackNames = callbackNames;
+    }
+
+}
diff --git a/src/com/vaadin/ui/AbstractJavaScriptComponent.java b/src/com/vaadin/ui/AbstractJavaScriptComponent.java
index 881ac3f9d9..e5759a1efc 100644
--- a/src/com/vaadin/ui/AbstractJavaScriptComponent.java
+++ b/src/com/vaadin/ui/AbstractJavaScriptComponent.java
@@ -4,6 +4,7 @@
 package com.vaadin.ui;
 
 import com.vaadin.terminal.JavaScriptCallbackHelper;
+import com.vaadin.terminal.gwt.client.ui.JavaScriptComponentState;
 
 public class AbstractJavaScriptComponent extends AbstractComponent {
     private JavaScriptCallbackHelper callbackHelper = new JavaScriptCallbackHelper(
@@ -17,4 +18,9 @@ public class AbstractJavaScriptComponent extends AbstractComponent {
     protected void invokeCallback(String name, Object... arguments) {
         callbackHelper.invokeCallback(name, arguments);
     }
+
+    @Override
+    public JavaScriptComponentState getState() {
+        return (JavaScriptComponentState) super.getState();
+    }
 }
diff --git a/tests/testbench/com/vaadin/tests/components/javascriptcomponent/BasicJavaScriptComponent.java b/tests/testbench/com/vaadin/tests/components/javascriptcomponent/BasicJavaScriptComponent.java
index 4de80aaf9d..6519e4ae74 100644
--- a/tests/testbench/com/vaadin/tests/components/javascriptcomponent/BasicJavaScriptComponent.java
+++ b/tests/testbench/com/vaadin/tests/components/javascriptcomponent/BasicJavaScriptComponent.java
@@ -7,11 +7,14 @@ import java.util.Arrays;
 import java.util.List;
 
 import com.vaadin.annotations.LoadScripts;
+import com.vaadin.external.json.JSONArray;
+import com.vaadin.external.json.JSONException;
 import com.vaadin.terminal.WrappedRequest;
-import com.vaadin.terminal.gwt.client.ComponentState;
 import com.vaadin.terminal.gwt.client.communication.ServerRpc;
+import com.vaadin.terminal.gwt.client.ui.JavaScriptComponentState;
 import com.vaadin.tests.components.AbstractTestRoot;
 import com.vaadin.ui.AbstractJavaScriptComponent;
+import com.vaadin.ui.JavaScriptCallback;
 import com.vaadin.ui.Root;
 
 @LoadScripts({ "/statictestfiles/jsconnector.js" })
@@ -21,7 +24,7 @@ public class BasicJavaScriptComponent extends AbstractTestRoot {
         public void onClick(String message);
     }
 
-    public static class SpecialState extends ComponentState {
+    public static class SpecialState extends JavaScriptComponentState {
         private List<String> data;
 
         public List<String> getData() {
@@ -41,6 +44,12 @@ public class BasicJavaScriptComponent extends AbstractTestRoot {
                             "Got a click: " + message);
                 }
             });
+            registerCallback("onclick", new JavaScriptCallback() {
+                public void call(JSONArray arguments) throws JSONException {
+                    Root.getCurrentRoot().showNotification(
+                            "Got a callback: " + arguments.getString(0));
+                }
+            });
             getState().setData(Arrays.asList("a", "b", "c"));
         }
 
diff --git a/tests/testbench/com/vaadin/tests/extensions/SimpleJavaScriptExtensionTest.java b/tests/testbench/com/vaadin/tests/extensions/SimpleJavaScriptExtensionTest.java
index 983520ee93..8bcf873cd5 100644
--- a/tests/testbench/com/vaadin/tests/extensions/SimpleJavaScriptExtensionTest.java
+++ b/tests/testbench/com/vaadin/tests/extensions/SimpleJavaScriptExtensionTest.java
@@ -5,20 +5,24 @@
 package com.vaadin.tests.extensions;
 
 import com.vaadin.annotations.LoadScripts;
+import com.vaadin.external.json.JSONArray;
+import com.vaadin.external.json.JSONException;
 import com.vaadin.terminal.AbstractJavaScriptExtension;
 import com.vaadin.terminal.WrappedRequest;
+import com.vaadin.terminal.gwt.client.JavaScriptExtensionState;
 import com.vaadin.terminal.gwt.client.communication.ClientRpc;
 import com.vaadin.terminal.gwt.client.communication.ServerRpc;
-import com.vaadin.terminal.gwt.client.communication.SharedState;
 import com.vaadin.tests.components.AbstractTestRoot;
 import com.vaadin.ui.Button;
 import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.JavaScriptCallback;
 import com.vaadin.ui.Root;
 
 @LoadScripts({ "/statictestfiles/jsextension.js" })
 public class SimpleJavaScriptExtensionTest extends AbstractTestRoot {
 
-    public static class SimpleJavaScriptExtensionState extends SharedState {
+    public static class SimpleJavaScriptExtensionState extends
+            JavaScriptExtensionState {
         private String prefix;
 
         public void setPrefix(String prefix) {
@@ -50,6 +54,12 @@ public class SimpleJavaScriptExtensionTest extends AbstractTestRoot {
                             getState().getPrefix() + message);
                 }
             });
+            registerCallback("greetToServer", new JavaScriptCallback() {
+                public void call(JSONArray arguments) throws JSONException {
+                    Root.getCurrentRoot().showNotification(
+                            getState().getPrefix() + arguments.getString(0));
+                }
+            });
         }
 
         @Override
@@ -62,10 +72,14 @@ public class SimpleJavaScriptExtensionTest extends AbstractTestRoot {
             requestRepaint();
         }
 
-        public void greet(String message) {
+        public void greetRpc(String message) {
             getRpcProxy(SimpleJavaScriptExtensionClientRpc.class)
                     .greet(message);
         }
+
+        public void greetCallback(String message) {
+            invokeCallback("greetToClient", message);
+        }
     }
 
     @Override
@@ -73,11 +87,19 @@ public class SimpleJavaScriptExtensionTest extends AbstractTestRoot {
         final SimpleJavascriptExtension simpleJavascriptExtension = new SimpleJavascriptExtension();
         simpleJavascriptExtension.setPrefix("Prefix: ");
         addExtension(simpleJavascriptExtension);
-        addComponent(new Button("Send greeting", new Button.ClickListener() {
-            public void buttonClick(ClickEvent event) {
-                simpleJavascriptExtension.greet("Greeted by button");
-            }
-        }));
+        addComponent(new Button("Send rpc greeting",
+                new Button.ClickListener() {
+                    public void buttonClick(ClickEvent event) {
+                        simpleJavascriptExtension.greetRpc("Rpc greeting");
+                    }
+                }));
+        addComponent(new Button("Send callback greeting",
+                new Button.ClickListener() {
+                    public void buttonClick(ClickEvent event) {
+                        simpleJavascriptExtension
+                                .greetCallback("Callback greeting");
+                    }
+                }));
     }
 
     @Override