review request: 8035782 : sun/launcher/LauncherHelper$FXHelper loaded unnecessarily

[Neil Toda]

Thanks Kumar.  Will check these.

I have FXLauncherTest.java open right now as I type.  I started looking 
at it yesterday.  Good
that I am looking at the right test case.  Good organization.  I might 
also have to add a
non FX jar file to test.. unless I've missed that.

-neil




On 4/29/2014 7:17 AM, Kumar Srinivasan wrote:
> 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
>>>>
>>>>
>>>>
>>>
>>>
>>
>