JavascriptManager -> JavaScript and changes based on review (#6730)

10 files changed, 83 insertions, 67 deletions

diff --git a/src/com/vaadin/terminal/JavascriptRpcHelper.java b/src/com/vaadin/terminal/JavascriptRpcHelper.java
index b566462833..937f7e3a54 100644
--- a/src/com/vaadin/terminal/JavascriptRpcHelper.java
+++ b/src/com/vaadin/terminal/JavascriptRpcHelper.java
@@ -12,8 +12,8 @@ import java.util.Map;
 import com.vaadin.external.json.JSONArray;
 import com.vaadin.external.json.JSONException;
 import com.vaadin.tools.ReflectTools;
+import com.vaadin.ui.JavaScript.JavascriptCallbackRpc;
 import com.vaadin.ui.JavascriptCallback;
-import com.vaadin.ui.JavascriptManager.JavascriptCallbackRpc;
 
 public class JavascriptRpcHelper {
 
diff --git a/src/com/vaadin/terminal/gwt/client/JavascriptConnectorHelper.java b/src/com/vaadin/terminal/gwt/client/JavascriptConnectorHelper.java
index ab0e62222c..36f7c47959 100644
--- a/src/com/vaadin/terminal/gwt/client/JavascriptConnectorHelper.java
+++ b/src/com/vaadin/terminal/gwt/client/JavascriptConnectorHelper.java
@@ -109,7 +109,7 @@ public class JavascriptConnectorHelper {
             'getCallback': function(name) {
                 return $entry(function() {
                     var args = [name, Array.prototype.slice.call(arguments, 0)];
-                    var iface = "com.vaadin.ui.JavascriptManager$JavascriptCallbackRpc";
+                    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);
                 });
@@ -173,8 +173,8 @@ public class JavascriptConnectorHelper {
 
     public void invokeJsRpc(MethodInvocation invocation,
             JSONArray parametersJson) {
-        if ("com.vaadin.ui.JavascriptManager$JavascriptCallbackRpc"
-                .equals(invocation.getInterfaceName())
+        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());
diff --git a/src/com/vaadin/terminal/gwt/client/JavascriptExtension.java b/src/com/vaadin/terminal/gwt/client/JavascriptExtension.java
index 6c098a52f6..74c3d7da45 100644
--- a/src/com/vaadin/terminal/gwt/client/JavascriptExtension.java
+++ b/src/com/vaadin/terminal/gwt/client/JavascriptExtension.java
@@ -7,11 +7,11 @@ 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.ui.AbstractConnector;
+import com.vaadin.terminal.gwt.client.extensions.AbstractExtensionConnector;
 import com.vaadin.terminal.gwt.client.ui.Connect;
 
 @Connect(AbstractJavascriptExtension.class)
-public class JavascriptExtension extends AbstractConnector implements
+public class JavascriptExtension extends AbstractExtensionConnector implements
         HasJavascriptConnectorHelper {
     private final JavascriptConnectorHelper helper = new JavascriptConnectorHelper(
             this);
diff --git a/src/com/vaadin/terminal/gwt/client/extensions/javascriptmanager/ExecuteJavaScriptRpc.java b/src/com/vaadin/terminal/gwt/client/extensions/javascriptmanager/ExecuteJavaScriptRpc.java
new file mode 100644
index 0000000000..f1185586d5
--- /dev/null
+++ b/src/com/vaadin/terminal/gwt/client/extensions/javascriptmanager/ExecuteJavaScriptRpc.java
@@ -0,0 +1,11 @@
+/* 
+@VaadinApache2LicenseForJavaFiles@
+ */
+
+package com.vaadin.terminal.gwt.client.extensions.javascriptmanager;
+
+import com.vaadin.terminal.gwt.client.communication.ClientRpc;
+
+public interface ExecuteJavaScriptRpc extends ClientRpc {
+    public void executeJavaScript(String script);
+}
diff --git a/src/com/vaadin/terminal/gwt/client/extensions/javascriptmanager/JavascriptManagerConnector.java b/src/com/vaadin/terminal/gwt/client/extensions/javascriptmanager/JavascriptManagerConnector.java
index a74cd762de..10c4847bac 100644
--- a/src/com/vaadin/terminal/gwt/client/extensions/javascriptmanager/JavascriptManagerConnector.java
+++ b/src/com/vaadin/terminal/gwt/client/extensions/javascriptmanager/JavascriptManagerConnector.java
@@ -14,13 +14,22 @@ import com.vaadin.terminal.gwt.client.communication.MethodInvocation;
 import com.vaadin.terminal.gwt.client.communication.StateChangeEvent;
 import com.vaadin.terminal.gwt.client.extensions.AbstractExtensionConnector;
 import com.vaadin.terminal.gwt.client.ui.Connect;
-import com.vaadin.ui.JavascriptManager;
+import com.vaadin.ui.JavaScript;
 
-@Connect(JavascriptManager.class)
+@Connect(JavaScript.class)
 public class JavascriptManagerConnector extends AbstractExtensionConnector {
     private Set<String> currentNames = new HashSet<String>();
 
     @Override
+    protected void init() {
+        registerRpc(ExecuteJavaScriptRpc.class, new ExecuteJavaScriptRpc() {
+            public void executeJavaScript(String script) {
+                eval(script);
+            }
+        });
+    }
+
+    @Override
     public void onStateChanged(StateChangeEvent stateChangeEvent) {
         super.onStateChanged(stateChangeEvent);
 
@@ -56,6 +65,13 @@ public class JavascriptManagerConnector extends AbstractExtensionConnector {
         delete $wnd[name];
     }-*/;
 
+    private static native void eval(String script)
+    /*-{
+      if(script) {
+         $wnd.eval(script);
+      }
+    }-*/;
+
     public void sendRpc(String name, JsArray<JavaScriptObject> arguments) {
         Object[] parameters = new Object[] { name, new JSONArray(arguments) };
 
@@ -63,12 +79,10 @@ public class JavascriptManagerConnector extends AbstractExtensionConnector {
          * Must invoke manually as the RPC interface can't be used in GWT
          * because of the JSONArray parameter
          */
-        getConnection()
-                .addMethodInvocationToQueue(
-                        new MethodInvocation(
-                                getConnectorId(),
-                                "com.vaadin.ui.JavascriptManager$JavascriptCallbackRpc",
-                                "call", parameters), true);
+        getConnection().addMethodInvocationToQueue(
+                new MethodInvocation(getConnectorId(),
+                        "com.vaadin.ui.JavaScript$JavascriptCallbackRpc",
+                        "call", parameters), true);
     }
 
     @Override
diff --git a/src/com/vaadin/terminal/gwt/client/ui/root/RootConnector.java b/src/com/vaadin/terminal/gwt/client/ui/root/RootConnector.java
index 9be41a9623..eebbe971a4 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/root/RootConnector.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/root/RootConnector.java
@@ -168,9 +168,6 @@ public class RootConnector extends AbstractComponentContainerConnector
                             getWidget().id, client);
                 }
                 getWidget().actionHandler.updateActionMap(childUidl);
-            } else if (tag == "execJS") {
-                String script = childUidl.getStringAttribute("script");
-                VRoot.eval(script);
             } else if (tag == "notifications") {
                 for (final Iterator<?> it = childUidl.getChildIterator(); it
                         .hasNext();) {
diff --git a/src/com/vaadin/terminal/gwt/client/ui/root/VRoot.java b/src/com/vaadin/terminal/gwt/client/ui/root/VRoot.java
index 12a69d5556..85e6f7f91a 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/root/VRoot.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/root/VRoot.java
@@ -188,21 +188,6 @@ public class VRoot extends SimplePanel implements ResizeHandler,
      }-*/;
 
     /**
-     * Evaluate the given script in the browser document.
-     * 
-     * @param script
-     *            Script to be executed.
-     */
-    static native void eval(String script)
-    /*-{
-      try {
-         if (script == null) return;
-         $wnd.eval(script);
-      } catch (e) {
-      }
-    }-*/;
-
-    /**
      * Returns true if the body is NOT generated, i.e if someone else has made
      * the page that we're running in. Otherwise we're in charge of the whole
      * page.
diff --git a/src/com/vaadin/ui/JavascriptManager.java b/src/com/vaadin/ui/JavaScript.java
index 72295dce2b..dcd8540947 100644
--- a/src/com/vaadin/ui/JavascriptManager.java
+++ b/src/com/vaadin/ui/JavaScript.java
@@ -10,10 +10,12 @@ import java.util.Map;
 import com.vaadin.external.json.JSONArray;
 import com.vaadin.external.json.JSONException;
 import com.vaadin.terminal.AbstractExtension;
+import com.vaadin.terminal.Extension;
 import com.vaadin.terminal.gwt.client.communication.ServerRpc;
+import com.vaadin.terminal.gwt.client.extensions.javascriptmanager.ExecuteJavaScriptRpc;
 import com.vaadin.terminal.gwt.client.extensions.javascriptmanager.JavascriptManagerState;
 
-public class JavascriptManager extends AbstractExtension {
+public class JavaScript extends AbstractExtension {
     private Map<String, JavascriptCallback> callbacks = new HashMap<String, JavascriptCallback>();
 
     // Can not be defined in client package as this JSONArray is not available
@@ -22,7 +24,7 @@ public class JavascriptManager extends AbstractExtension {
         public void call(String name, JSONArray arguments);
     }
 
-    public JavascriptManager() {
+    public JavaScript() {
         registerRpc(new JavascriptCallbackRpc() {
             public void call(String name, JSONArray arguments) {
                 JavascriptCallback callback = callbacks.get(name);
@@ -41,8 +43,8 @@ public class JavascriptManager extends AbstractExtension {
         return (JavascriptManagerState) super.getState();
     }
 
-    public void addCallback(String name, JavascriptCallback javascriptCallback) {
-        callbacks.put(name, javascriptCallback);
+    public void addCallback(String name, JavascriptCallback callback) {
+        callbacks.put(name, callback);
         if (getState().getNames().add(name)) {
             requestRepaint();
         }
@@ -55,4 +57,28 @@ public class JavascriptManager extends AbstractExtension {
         }
     }
 
+    public void execute(String script) {
+        getRpcProxy(ExecuteJavaScriptRpc.class).executeJavaScript(script);
+    }
+
+    public static JavaScript getCurrent() {
+        return Root.getCurrentRoot().getJavaScript();
+    }
+
+    private static JavaScript getJavascript(Root root) {
+        // TODO Add caching to avoid iterating collection every time
+        // Caching should use weak references to avoid memory leaks -> cache
+        // should be transient to avoid serialization problems
+        for (Extension extension : root.getExtensions()) {
+            if (extension instanceof JavaScript) {
+                return (JavaScript) extension;
+            }
+        }
+
+        // Extend root if it isn't yet done
+        JavaScript javascript = new JavaScript();
+        javascript.extend(root);
+        return javascript;
+    }
+
 }
diff --git a/src/com/vaadin/ui/Root.java b/src/com/vaadin/ui/Root.java
index 9814084cbc..60408fe1dc 100644
--- a/src/com/vaadin/ui/Root.java
+++ b/src/com/vaadin/ui/Root.java
@@ -357,12 +357,6 @@ public abstract class Root extends AbstractComponentContainer implements
     private List<Notification> notifications;
 
     /**
-     * A list of javascript commands that are waiting to be sent to the client.
-     * Cleared (set to null) when the commands have been sent.
-     */
-    private List<String> jsExecQueue = null;
-
-    /**
      * List of windows in this root.
      */
     private final LinkedHashSet<Window> windows = new LinkedHashSet<Window>();
@@ -408,7 +402,7 @@ public abstract class Root extends AbstractComponentContainer implements
     private DirtyConnectorTracker dirtyConnectorTracker = new DirtyConnectorTracker(
             this);
 
-    private JavascriptManager javascriptManager;
+    private JavaScript javaScript;
 
     private RootServerRpc rpc = new RootServerRpc() {
         public void click(MouseEventDetails mouseDetails) {
@@ -557,16 +551,6 @@ public abstract class Root extends AbstractComponentContainer implements
             notifications = null;
         }
 
-        // Add executable javascripts if needed
-        if (jsExecQueue != null) {
-            for (String script : jsExecQueue) {
-                target.startTag("execJS");
-                target.addAttribute("script", script);
-                target.endTag("execJS");
-            }
-            jsExecQueue = null;
-        }
-
         if (scrollIntoView != null) {
             target.addAttribute("scrollTo", scrollIntoView);
             scrollIntoView = null;
@@ -1000,15 +984,12 @@ public abstract class Root extends AbstractComponentContainer implements
      * 
      * @param script
      *            JavaScript snippet that will be executed.
+     * 
+     * @deprecated as of 7.0, use getJavaScript().execute(String) instead
      */
+    @Deprecated
     public void executeJavaScript(String script) {
-        if (jsExecQueue == null) {
-            jsExecQueue = new ArrayList<String>();
-        }
-
-        jsExecQueue.add(script);
-
-        requestRepaint();
+        getJavaScript().execute(script);
     }
 
     /**
@@ -1592,14 +1573,14 @@ public abstract class Root extends AbstractComponentContainer implements
         return dirtyConnectorTracker;
     }
 
-    public JavascriptManager getJavascriptManager() {
-        if (javascriptManager == null) {
+    public JavaScript getJavaScript() {
+        if (javaScript == null) {
             // Create and attach on first use
-            javascriptManager = new JavascriptManager();
-            addExtension(javascriptManager);
+            javaScript = new JavaScript();
+            addExtension(javaScript);
         }
 
-        return javascriptManager;
+        return javaScript;
     }
 
 }
diff --git a/tests/testbench/com/vaadin/tests/extensions/JavascriptManagerTest.java b/tests/testbench/com/vaadin/tests/extensions/JavascriptManagerTest.java
index c84d37cd0f..0270f7ed8f 100644
--- a/tests/testbench/com/vaadin/tests/extensions/JavascriptManagerTest.java
+++ b/tests/testbench/com/vaadin/tests/extensions/JavascriptManagerTest.java
@@ -10,6 +10,7 @@ import com.vaadin.external.json.JSONObject;
 import com.vaadin.terminal.WrappedRequest;
 import com.vaadin.tests.components.AbstractTestRoot;
 import com.vaadin.tests.util.Log;
+import com.vaadin.ui.JavaScript;
 import com.vaadin.ui.JavascriptCallback;
 
 public class JavascriptManagerTest extends AbstractTestRoot {
@@ -19,7 +20,8 @@ public class JavascriptManagerTest extends AbstractTestRoot {
     @Override
     protected void setup(WrappedRequest request) {
         addComponent(log);
-        getJavascriptManager().addCallback("testing", new JavascriptCallback() {
+        JavaScript js = JavaScript.getCurrent();
+        js.addCallback("testing", new JavascriptCallback() {
             public void call(JSONArray arguments) throws JSONException {
                 log.log("Got " + arguments.length() + " arguments");
                 log.log("Argument 1 as a number: " + arguments.getInt(0));
@@ -30,7 +32,7 @@ public class JavascriptManagerTest extends AbstractTestRoot {
                         + (arguments.get(3) == JSONObject.NULL));
             }
         });
-        executeJavaScript("window.testing(42, 'text', {p: true}, null)");
+        js.execute("window.testing(42, 'text', {p: true}, null)");
     }
 
     @Override