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







Reply via email to