Neil,
The changes looks satisfactory, except for a few style nits:
1. JAVAFX_FXHELPER_CLASS_NAME_SUF -> JAVAFX_FXHELPER_CLASS_NAME_SUFFIX
// 3 more characters won't make much of a difference
2. FXHelper.setFXLaunchParameters(what,mode);
// missing space after comma.
A Launcher regression test is required, which ensure that the launcher
does not load
FXLauncher inadvertently in the future. ie. a regression test.
You can do this by adding another condition in the method
testExtraneousJars at:
http://hg.openjdk.java.net/jdk9/jdk9/jdk/file/3e8e199e23b2/test/tools/launcher/FXLauncherTest.java#l360
just like jfxrt.jar is being tested now.
Kumar
On 4/25/2014 6:55 PM, Neil Toda wrote:
Thanks Kevin. -neil
On 4/25/2014 8:22 AM, Kevin Rushforth wrote:
The code changes looks fine to me. Also, I ran all JavaFX unit tests
with no problems (at least none relating to launching).
-- Kevin
Neil Toda wrote:
Webrev
http://cr.openjdk.java.net/~ntoda/8035782/
for bug
https://bugs.openjdk.java.net/browse/JDK-8035782
The file : ./jdk/src/share/classes/sun/launcher/LauncherHelper.java
has been modified so that the inner class FXHelper is not loaded
unnecessarily.
FXHelper, which is needed to make initializations for any JavaFX
application, was
being loaded for all applications.
The fix was straight forward, with the lifting of one method and
several static
strings into FXHelper's superclass, LauncherHelper.
Kevin Rushforth supplied three tests of applications not in jar
files. These
needed to be explicitly tested. These tests require the JavaFX
bundle in the
build, and the return code 2 signifies success.
Launcher tests for jtreg: ./jdk/test/tools/launcher passed on
Windows 7 64 and Oracle-Linux6-64.
JPRT tests were run and passed on scv3.
Thanks
-neil