David,
It is looking good, a couple of comments and requests:
1. LauncherHelper.java: Can you please document the table below in
LauncherHelper.java, and a note to refer to LauncherHelper.java in
FXLauncherTest.java this will make it easier to understand everything
in the future.
2. FXLauncherTest.java
a. testExtraneousJars, you have changed the cmd from javac to java
using a java class, thus this error message is not accurate:
throw new Exception("jfxrt.jar is being loaded by javac!!!");
b. The case of -jar with no JAC is being commented out ? I take it this
is redundant now ? Does this need to present ? or a todo for later ?
If so please comment it appropriately.
// Todo: blah
// testBasic......
or
/*
* Redundant but can be used iff .....
* .......
*/
Kumar
[adding core-libs-dev back in.. not sure how that got lost]
[Back from vacation, let's get the ball rolling again… :]
In order to understand and explain here is a truth table:
Cmd line FAC LAUNCH_MODE JAVAFX_LAUNCH_MODE
java -jar fxapp.jar Present LM_JAR LM_JAR
java -jar fxapp.jar Not present LM_JAR LM_CLASS ??
java -cp fxapp.jar ... Not present LM_CLASS LM_CLASS
java -cp somedir ... Not present LM_CLASS LM_CLASS
If I am understanding this correctly, the confusion stems from the second case
where the
value is interpreted in different ways.
Correct. I have actually changed it to fix this case, and IMHO it cleans up the
launcher code a bit.. I'll post the webrev when I'm done syncing with TL.
The current patch I have:
http://cr.openjdk.java.net/~ddehaven/8004547/webrev.2/
Minor change to accommodate this in FX LauncherImpl (needs a new JIRA issue?):
http://cr.openjdk.java.net/~ddehaven/javafx/RT-26751/webrev.2/
This was all working fine when I left for vacation, I'm still in the process of
building/testing everything and will submit a JPRT run as soon as possible.
I had commented out two tests, but FX is in sync with JDK *and* they'll pass
vacuously until FX is on the ext classpath anyways so I don't think that's
necessary.
-DrD-
