8238080: FXMLLoader: if script engines implement javax.script.Compila…

…ble compile scripts Reviewed-by: kcr, aghaisas
openjdk · Jun 30, 2020 · 45c9854ce9b5dc38638f6d2426db3f63d2361cd1 · 45c9854
1 parent 15f97ed
commit 45c9854ce9b5dc38638f6d2426db3f63d2361cd1
diff --git a/build.gradle b/build.gradle
@@ -3578,6 +3578,7 @@ project(":systemTests") {
         testapp5
         testapp6
         testscriptapp1
+        testscriptapp2
     }
 
     def nonModSrcSets = [
@@ -3591,7 +3592,8 @@ project(":systemTests") {
         sourceSets.testapp4,
         sourceSets.testapp5,
         sourceSets.testapp6,
-        sourceSets.testscriptapp1
+        sourceSets.testscriptapp1,
+        sourceSets.testscriptapp2
     ]
 
     project.ext.buildModule = false
@@ -3691,7 +3693,7 @@ project(":systemTests") {
     }
     test.dependsOn(createTestApps);
 
-    def modtestapps = [ "testapp2", "testapp3", "testapp4", "testapp5", "testapp6", "testscriptapp1" ]
+    def modtestapps = [ "testapp2", "testapp3", "testapp4", "testapp5", "testapp6", "testscriptapp1", "testscriptapp2" ]
     modtestapps.each { testapp ->
         def testappCapital = testapp.capitalize()
         def copyTestAppTask = task("copy${testappCapital}", type: Copy) {

diff --git a/modules/javafx.fxml/src/main/docs/javafx/fxml/doc-files/introduction_to_fxml.html b/modules/javafx.fxml/src/main/docs/javafx/fxml/doc-files/introduction_to_fxml.html
@@ -29,7 +29,7 @@
 
 <html lang="en">
 <head>
-<link href="fxml.css" type="text/css" rel="stylesheet"/>    
+<link href="fxml.css" type="text/css" rel="stylesheet"/>
 <meta http-equiv="content-type" content="text/html; charset=UTF-8"/>
 <title>Introduction to FXML | JavaFX @FXVERSION@</title>
 <meta name="description" content="The document introduces FXML, an XML-based declarative markup language for defining user interfaces in JavaFX @FXVERSION@ applications."/>
@@ -597,13 +597,13 @@ <h4><a id="expression_binding">Expression Binding</a></h4>
  <tr><th scope="row">- <br/>(unary operator)</th><td>Unary minus operator, applied on a number</td>
  <tr><th scope="row">! <br/>(unary operator)</th><td>Unary negation of a boolean</td></tr>
  <tr><th scope="row">+ - <br />
- 			* /
- 			%</th> <td>Numerical binary operators</td></tr>
+                        * /
+                        %</th> <td>Numerical binary operators</td></tr>
  <tr><th scope="row">&amp;&amp; ||</th><td>Boolean binary operators</td></tr>
  <tr><th scope="row">&gt; &gt;= <br />
- 		   &lt; &lt;= <br />
- 		   == !=</th>
- 		   <td>Binary operators of comparison.<br/> Both arguments must be of type Comparable</td></tr>
+                   &lt; &lt;= <br />
+                   == !=</th>
+                   <td>Binary operators of comparison.<br/> Both arguments must be of type Comparable</td></tr>
 </table>
 
 <h3><a id="static_property_attributes">Static Properties</a></h3>
@@ -641,6 +641,8 @@ <h4><a id="script_event_handlers">Script Event Handlers</a></h4>
 
 <p>Note the use of the language processing instruction at the beginning of the code snippet. This PI tells the FXML loader which scripting language should be used to execute the event handler. A page language must be specified whenever inline script is used in an FXML document, and can only be specified once per document. However, this does not apply to external scripts, which may be implemented using any number of supported scripting languages. Scripting is discussed in more detail in the next section.</p>
 
+<p>Note: to turn off automatic compilation of script code place the processing instruction <span class="code">&lt;?compile false?&gt;</span> before the element that contains the script. To turn on compilation of script code again use the processing instruction <span class="code">&lt;?compile true?&gt;</span> (or short: <span class="code">&lt;?compile?&gt;</span>). The compile processing instruction can be used repeatedly to turn compilation of script code off and on.</p>
+
 <h4><a id="controller_method_event_handlers">Controller Method Event Handlers</a></h4>
 <p>A controller method event handler is a method defined by a document's "controller". A controller is an object that is associated with the deserialized contents of an FXML document and is responsible for coordinating the behaviors of the objects (often user interface elements) defined by the document.</p>
 
@@ -700,15 +702,15 @@ <h4><a id="expression_handlers">Event handlers from expressions</a></h4>
 </pre>
 
 <p> With the controller that contains a field like this </p>
-    
+
 <pre class="code">
 public class MyController {
-    
+
     &#64;FXML
     public EventHandler&lt;ActionEvent&gt; onActionHandler = new EventHandler&lt;&gt;() { ... }
 
     ...
-}  
+}
 </pre>
 
 <p> Note that other kinds of expressions, like <a href="#expression_binding">binding expressions</a>
@@ -763,14 +765,18 @@ <h4><a id="collections_and_property_handlers">Special handlers for collections a
 &lt;VBox fx:controller="com.foo.MyController"
     xmlns:fx="http://javafx.com/fxml" onParentChange="#handleParentChange"/&gt;
 </pre>
-    
+
 <p>Note that collections and properties do not currently support scripting handlers.</p>
 
 <h2><a id="scripting">Scripting</a></h2>
 <p>
 The <span class="code">&lt;fx:script&gt;</span> tag allows a caller to import scripting code into or embed script within a FXML file. Any JVM scripting language can be used, including JavaScript, Groovy, and Clojure, among others. Script code is often used to define event handlers directly in markup or in an associated source file, since event handlers can often be written more concisely in more loosely-typed scripting languages than they can in a statically-typed language such as Java.</p>
 
-<p>For example, the following markup defines a function called <span class="code">handleButtonAction()</span> that is called by the action handler attached to the <span class="code">Button</span> element:</p>
+<p>Scripts are compiled by default, when they are first loaded, if the <span class="code">ScriptEngine</span> implements the <span class="code">javax.script.Compilable</span> interface. If compilation fails, the <span class="code">FXMLLoader</span> will fall back to interpreted mode.</p>
+
+<p>Note: to turn off automatic compilation of script code place the processing instruction <span class="code">&lt;?compile false?&gt;</span> before the script element. To turn on compilation of script code again use the processing instruction <span class="code">&lt;?compile true?&gt;</span> (or short: <span class="code">&lt;?compile?&gt;</span>). The compile processing instruction can be used repeatedly to turn compilation of script code off and on.</p>
+
+<p>The following example markup defines a function called <span class="code">handleButtonAction()</span> that is called by the action handler attached to the <span class="code">Button</span> element:</p>
 
 <pre class="code">
 &lt;?language javascript?&gt;
@@ -798,6 +804,8 @@ <h2><a id="scripting">Scripting</a></h2>
 
 <div class="caption">example.fxml</div>
 <pre class="code">
+&lt;?language javascript?&gt;
+
 &lt;?import javafx.scene.control.*?&gt;
 &lt;?import javafx.scene.layout.*?&gt;
 
@@ -833,8 +841,7 @@ <h2><a id="scripting">Scripting</a></h2>
 &lt;Label text="$myText"/&gt;
 </pre>
 
-
-<p><strong>Warning:</strong>As of JavaFX 8, <span class="code">importClass()</span> javascript function is no longer supported. You have to use fully qualified names as in the example above or load a nashorn compatibility script.</p>
+<p><strong>Warning:</strong> As of JavaFX 8, <span class="code">importClass()</span> javascript function is no longer supported. You have to use fully qualified names as in the example above or load a nashorn compatibility script.</p>
 
 <pre class="code">
 load("nashorn:mozilla_compat.js");
@@ -843,7 +850,7 @@ <h2><a id="scripting">Scripting</a></h2>
 function handleButtonAction(event) {
    System.out.println('You clicked me!');
 }
-</pre> 
+</pre>
 
 <h2><a id="controllers">Controllers</a></h2>
 <p>While it can be convenient to write simple event handlers in script, either inline or defined in external files, it is often preferable to define more complex application logic in a compiled, strongly-typed language such as Java. As discussed earlier, the <span class="code">fx:controller</span> attribute allows a caller to associate a "controller" class with an FXML document. A controller is a compiled class that implements the "code behind" the object hierarchy defined by the document.</p>

diff --git a/modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java b/modules/javafx.fxml/src/main/java/javafx/fxml/FXMLLoader.java
@@ -63,6 +63,8 @@
 import javafx.util.Callback;
 
 import javax.script.Bindings;
+import javax.script.Compilable;
+import javax.script.CompiledScript;
 import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineManager;
@@ -1558,21 +1560,55 @@ public void processStartElement() throws IOException {
                         location = new URL(FXMLLoader.this.location, source);
                     }
                     Bindings engineBindings = engine.getBindings(ScriptContext.ENGINE_SCOPE);
-                    engineBindings.put(engine.FILENAME, location.getPath());
+                    String filename = location.getPath();
+                    engineBindings.put(engine.FILENAME, filename);
 
                     InputStreamReader scriptReader = null;
+                    String script = null;
                     try {
                         scriptReader = new InputStreamReader(location.openStream(), charset);
-                        engine.eval(scriptReader);
-                    } catch(ScriptException exception) {
-                        exception.printStackTrace();
+                        StringBuilder sb = new StringBuilder();
+                        final int bufSize = 4096;
+                        char[] charBuffer = new char[bufSize];
+                        int n;
+                        do {
+                            n = scriptReader.read(charBuffer,0,bufSize);
+                            if (n > 0) {
+                                sb.append(new String(charBuffer,0,n));
+                          }
+                        } while (n > -1);
+                        script = sb.toString();
+                    } catch (IOException exception) {
+                        throw constructLoadException(exception);
                     } finally {
                         if (scriptReader != null) {
                             scriptReader.close();
                         }
                     }
-                } catch (IOException exception) {
-                    throw constructLoadException(exception);
+                    try {
+                        if (engine instanceof Compilable && compileScript) {
+                            CompiledScript compiledScript = null;
+                            try {
+                                compiledScript = ((Compilable) engine).compile(script);
+                            } catch (ScriptException compileExc) {
+                                Logging.getJavaFXLogger().warning(filename + ": compiling caused \"" + compileExc +
+                                    "\", falling back to evaluating script in uncompiled mode");
+                            }
+                            if (compiledScript != null) {
+                                compiledScript.eval();
+                            } else { // fallback to uncompiled mode
+                                engine.eval(script);
+                            }
+                        } else {
+                            engine.eval(script);
+                        }
+                    } catch (ScriptException exception) {
+                        System.err.println(filename + ": caused ScriptException");
+                        exception.printStackTrace();
+                    }
+                }
+                catch (IOException exception) {
+                  throw constructLoadException(exception);
                 }
             }
         }
@@ -1583,13 +1619,31 @@ public void processEndElement() throws IOException {
 
             if (value != null && !staticLoad) {
                 // Evaluate the script
+                String filename = null;
                 try {
                     Bindings engineBindings = scriptEngine.getBindings(ScriptContext.ENGINE_SCOPE);
-                    engineBindings.put(scriptEngine.FILENAME, location.getPath() + "-script_starting_at_line_"
-                                       + (getLineNumber() - (int) ((String) value).codePoints().filter(c -> c == '\n').count()));
-                    scriptEngine.eval((String)value);
+                    String script = (String) value;
+                    filename = location.getPath() + "-script_starting_at_line_"
+                                       + (getLineNumber() - (int) script.codePoints().filter(c -> c == '\n').count());
+                    engineBindings.put(scriptEngine.FILENAME, filename);
+                    if (scriptEngine instanceof Compilable && compileScript) {
+                        CompiledScript compiledScript = null;
+                        try {
+                            compiledScript = ((Compilable) scriptEngine).compile(script);
+                        } catch (ScriptException compileExc) {
+                            Logging.getJavaFXLogger().warning(filename + ": compiling caused \"" + compileExc +
+                                "\", falling back to evaluating script in uncompiled mode");
+                        }
+                        if (compiledScript != null) {
+                            compiledScript.eval();
+                        } else { // fallback to uncompiled mode
+                            scriptEngine.eval(script);
+                        }
+                    } else {
+                        scriptEngine.eval(script);
+                    }
                 } catch (ScriptException exception) {
-                    System.err.println(exception.getMessage());
+                    System.err.println(filename + ": caused ScriptException\n" + exception.getMessage());
                 }
             }
         }
@@ -1681,11 +1735,24 @@ public void handle(T event) {
         public final String script;
         public final ScriptEngine scriptEngine;
         public final String filename;
+        public CompiledScript compiledScript;
+        public boolean isCompiled = false;
 
         public ScriptEventHandler(String script, ScriptEngine scriptEngine, String filename) {
             this.script = script;
             this.scriptEngine = scriptEngine;
             this.filename = filename;
+            if (scriptEngine instanceof Compilable  && compileScript) {
+                try {
+                    // supply the filename to the scriptEngine engine scope Bindings in case it is needed for compilation
+                    scriptEngine.getBindings(ScriptContext.ENGINE_SCOPE).put(scriptEngine.FILENAME, filename);
+                    this.compiledScript = ((Compilable) scriptEngine).compile(script);
+                    this.isCompiled = true;
+                } catch (ScriptException compileExc) {
+                    Logging.getJavaFXLogger().warning(filename + ": compiling caused \"" + compileExc +
+                        "\", falling back to evaluating script in uncompiled mode");
+                }
+            }
         }
 
         @Override
@@ -1699,9 +1766,13 @@ public void handle(Event event) {
             localBindings.put(scriptEngine.FILENAME, filename);
             // Execute the script
             try {
-                scriptEngine.eval(script, localBindings);
-            } catch (ScriptException exception){
-                throw new RuntimeException(exception);
+                if (isCompiled) {
+                   compiledScript.eval(localBindings);
+                } else {
+                   scriptEngine.eval(script, localBindings);
+                }
+            } catch (ScriptException exception) {
+                throw new RuntimeException(filename + ": caused ScriptException", exception);
             }
         }
     }
@@ -1819,6 +1890,7 @@ public void invoke(Object... params) {
     private Element current = null;
 
     private ScriptEngine scriptEngine = null;
+    private static boolean compileScript = true;
 
     private List<String> packages = new LinkedList<String>();
     private Map<String, Class<?>> classes = new HashMap<String, Class<?>>();
@@ -1845,6 +1917,12 @@ public void invoke(Object... params) {
      */
     public static final String IMPORT_PROCESSING_INSTRUCTION = "import";
 
+    /**
+     * The tag name of the compile processing instruction.
+     * @since 15
+     */
+    public static final String COMPILE_PROCESSING_INSTRUCTION = "compile";
+
     /**
      * Prefix of 'fx' namespace.
      */
@@ -2678,6 +2756,10 @@ private void processProcessingInstruction() throws LoadException {
             processLanguage();
         } else if (piTarget.equals(IMPORT_PROCESSING_INSTRUCTION)) {
             processImport();
+        } else if (piTarget.equals(COMPILE_PROCESSING_INSTRUCTION)) {
+            String strCompile = xmlStreamReader.getPIData().trim();
+            // if PIData() is empty string then default to true, otherwise use Boolean.parseBoolean(string) to determine the boolean value
+            compileScript = (strCompile.length() == 0 ? true : Boolean.parseBoolean(strCompile));
         }
     }
 

diff --git a/tests/system/src/test/java/test/launchertest/ModuleLauncherTest.java b/tests/system/src/test/java/test/launchertest/ModuleLauncherTest.java
@@ -44,6 +44,7 @@
     private static final String modulePath5 = System.getProperty("launchertest.testapp5.module.path");
     private static final String modulePath6 = System.getProperty("launchertest.testapp6.module.path");
     private static final String modulePathScript1 = System.getProperty("launchertest.testscriptapp1.module.path");
+    private static final String modulePathScript2 = System.getProperty("launchertest.testscriptapp2.module.path");
 
     private static final String moduleName = "mymod";
 
@@ -281,4 +282,27 @@ public void testFXMLScriptDeployment() throws Exception {
         doTestLaunchModule(modulePathScript1, "myapp1.FXMLScriptDeployment");
     }
 
+    @Test (timeout = 15000)
+    public void testFXMLScriptDeployment2Compile_On() throws Exception {
+        doTestLaunchModule(modulePathScript2, "myapp2.FXMLScriptDeployment2Compile_On");
+    }
+
+    @Test (timeout = 15000)
+    public void testFXMLScriptDeployment2Compile_Off() throws Exception {
+        doTestLaunchModule(modulePathScript2, "myapp2.FXMLScriptDeployment2Compile_Off");
+    }
+
+    @Test (timeout = 15000)
+    public void testFXMLScriptDeployment2Compile_On_Off() throws Exception {
+        doTestLaunchModule(modulePathScript2, "myapp2.FXMLScriptDeployment2Compile_On_Off");
+    }
+
+    @Test (timeout = 15000)
+    public void testFXMLScriptDeployment2Compile_Off_On() throws Exception {
+        doTestLaunchModule(modulePathScript2, "myapp2.FXMLScriptDeployment2Compile_Off_On");
+    }
+    @Test (timeout = 15000)
+    public void testFXMLScriptDeployment2Compile_Fail_Compilation() throws Exception {
+        doTestLaunchModule(modulePathScript2, "myapp2.FXMLScriptDeployment2Compile_Fail_Compilation");
+    }
 }
diff --git a/...tem/src/testscriptapp1/resources/mymod/META-INF/services/javax.script.ScriptEngineFactory b/...tem/src/testscriptapp1/resources/mymod/META-INF/services/javax.script.ScriptEngineFactory
@@ -1,2 +1 @@
 pseudoScriptEngine.RgfPseudoScriptEngineFactory
-
diff --git a/tests/system/src/testscriptapp2/java/mymod/module-info.java b/tests/system/src/testscriptapp2/java/mymod/module-info.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.  Oracle designates this
+ * particular file as subject to the "Classpath" exception as provided
+ * by Oracle in the LICENSE file that accompanied this code.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+module mymod {
+    requires javafx.controls;
+    requires javafx.fxml;
+
+    requires java.scripting;
+    provides javax.script.ScriptEngineFactory with pseudoScriptEngineCompilable.RgfPseudoScriptEngineCompilableFactory;
+    exports pseudoScriptEngineCompilable;
+    exports myapp2;
+}
		@@ -1,2 +1 @@
		pseudoScriptEngine.RgfPseudoScriptEngineFactory