Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8238080: FXMLLoader: if script engines implement javax.script.Compilable compile scripts #192

Closed
Closed
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
5c33d59
fix for <https://bugs.openjdk.java.net/browse/JDK-8234959> JDK-823495…
ronyfla Feb 22, 2020
e251996
test for <https://bugs.openjdk.java.net/browse/JDK-8234959> JDK-82349…
ronyfla Feb 22, 2020
6e4df77
Removed/replaced white space.
ronyfla Feb 22, 2020
3053959
Removed/replaced white space.
ronyfla Feb 22, 2020
8cf0a6e
Incorporating Kevin Rushfort's review comments to this WIP.
ronyfla Feb 27, 2020
2db3171
Incorporating Kevin Rushfort's review comments to this WIP.
ronyfla Feb 27, 2020
799a814
Merge branch 'scripttest' into script as requested by reviewer Kevin
ronyfla Feb 27, 2020
8821d25
Removed executable bit from gradlew as this causes an error when
ronyfla Feb 27, 2020
a42fed5
corrected wrong test string
ronyfla Feb 27, 2020
c4d7cb1
JDK-8238080 : FXMLLoader: if script engines implement javax.script.Co…
ronyfla Feb 28, 2020
119e61b
Merge master
ronyfla Mar 31, 2020
0667a8d
Merge master, correct ovesights.
ronyfla Mar 31, 2020
ea0a8f2
appease the jcheck script, such that that important trailing Whitespa…
ronyfla Mar 31, 2020
a33284a
Correct error constant.
ronyfla Apr 2, 2020
316580b
add compile process instruction to control compilation of compilable …
ronyfla Apr 17, 2020
8dd9b00
Compile by default, have fallback if compilation fails, adapt/add tes…
ronyfla Apr 18, 2020
ef93a8f
Make message more pregnant.
ronyfla Apr 20, 2020
7c3c241
Always supply the script's filename in the error message first to fur…
ronyfla Apr 20, 2020
a97e758
Correct ModuleLauncherTest (remove non-existing test), correct format…
ronyfla Apr 20, 2020
c2f5c29
Revert temporary rename of test method.
ronyfla Apr 20, 2020
59b66d3
Make sure we test the default behaviour to compile script by leaving …
ronyfla Apr 22, 2020
494b888
Correct typo, replace tabs, remove trailing blanks.
ronyfla May 12, 2020
a037496
Add missing language processing instruction.
ronyfla May 12, 2020
44b0f9f
Document the compile processing instruction for scripts.
ronyfla May 12, 2020
59a16c9
Reword the compile processing instructions (replace 'Hint' with 'Note…
ronyfla May 13, 2020
ca4b818
Merge remote-tracking branch 'upstream/master' into scriptCompilableP…
ronyfla Jun 3, 2020
7ef1bd6
Updates to meet Kevin's comment in PR 192 as of May 27th.
ronyfla Jun 4, 2020
f2ea3d0
Incorporating Kevin's review comments (final int, fix for loop test, …
ronyfla Jun 19, 2020
f7f9cac
Incorporating Kevin's review comments.
ronyfla Jun 26, 2020
52da8a3
Incorporating Kevin's review comments (overlooked some places).
ronyfla Jun 27, 2020
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -3572,6 +3572,7 @@ project(":systemTests") {
testapp5
testapp6
testscriptapp1
testscriptapp2
}

def nonModSrcSets = [
@@ -3585,7 +3586,8 @@ project(":systemTests") {
sourceSets.testapp4,
sourceSets.testapp5,
sourceSets.testapp6,
sourceSets.testscriptapp1
sourceSets.testscriptapp1,
sourceSets.testscriptapp2
]

project.ext.buildModule = false
@@ -3685,7 +3687,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) {
@@ -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>
This conversation was marked as resolved by kevinrushforth

This comment has been minimized.

@kevinrushforth

kevinrushforth May 26, 2020 Member

I would start by saying that scripts are compiled by default if the script engine implements javax.script.Compilable. Maybe add a sentence something like this:

Scripts are compiled by default, when they are first loaded, if the ScriptEngine implements the javax.script.Compilable interface. If compilation fails, the FXMLLoader will fall back to interpreted mode.


<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>
@@ -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) {
This conversation was marked as resolved by kevinrushforth

This comment has been minimized.

@kevinrushforth

kevinrushforth Jun 19, 2020 Member

I think there may be other places you need to set isCompiled (it isn't set in the first couple of methods where you compile scripts). Can you check this?

This comment has been minimized.

@ronyfla

ronyfla Jun 19, 2020 Author Contributor

isCompiled gets set explicitly to false at object creation time. It only will be changed to true, if the script was successfully compiled.

This comment has been minimized.

@kevinrushforth

kevinrushforth Jun 19, 2020 Member

I was (mistakenly) thinking of the processStartElement and processEndElement methods of the ScriptElement class, but that's a different class, and they use a local compiledScript variable. So this is fine.

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";
This conversation was marked as resolved by kevinrushforth

This comment has been minimized.

@kevinrushforth

kevinrushforth May 26, 2020 Member

You need to add an @since 15 javadoc tag for this new public field.


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

@@ -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");
}
}
@@ -1,2 +1 @@
pseudoScriptEngine.RgfPseudoScriptEngineFactory

@@ -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;
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.