Looks good with two minor comments:
1) It looks like the new "mainModule" parameter to
launchApplicationWithArgs is only ever called with "null". Did you
intend it to be called with the application module in the case of LM_MODULE?
2) In the case of LM_JAR with no preloader attribute (i.e.,
JavaFX-Preloader-Class is not present in the jar file), the former
behavior would then use the default preloader. The new behavior will
look for the javafx.preloader system property in that case. I think this
is a fine change, but I wanted to point it out.
-- Kevin
David DeHaven wrote:
Ping? The OpenJDK portion is reviewed and ready to push. I'd prefer to push
them at the same time so we don't have a bug that appears to be resolved when
it isn't.
-DrD-
Please review the JavaFX changes needed for this issue:
https://bugs.openjdk.java.net/browse/JDK-8169289
webrev:
http://cr.openjdk.java.net/~ddehaven/8169289/openjfx-rt.0
For reference, the OpenJDK changes needed are here:
http://cr.openjdk.java.net/~ddehaven/8169289/jdk.0/
These are being reviewed separately on [email protected]
I've tested the following cases so far:
LM_CLASS_1 : -cp some.jar main.class
LM_CLASS_2 : -cp path/to/loose/classes main.class
LM_JAR_1 : -jar some.jar (Main-Class)
LM_JAR_2 : -jar some.jar (Main-Class + JavaFX-Application-Class) <- JAC
should take precedence
LM_MODULE_1 : -m module/main.class
LM_MODULE_2 : -m module (main class declared in module-info)
and one more in progress:
LM_JAR_3 : -jar some.jar (Main-Class + JavaFX-Class-Path) <- creates new
ClassLoader
I've tested all combinations of having changes (or not) to openjfx and openjdk
and there are no unexpected results, the only cases that fail are the LM_MODULE
launch modes which fail currently. This means the changes can be pushed in any
order.
I will create unit tests for this, but don't want to push it since this change
is split between OpenJFX and OpenJDK (else LM_MODULE modes will just fail until
everything is in sync). I'll file a separate issue to push the unit tests at
the appropriate time.
A couple other notes:
- I brought over changes for handling diacritical marks on MacOS X [1], moved
class loading to a new method
- I assumed not having a default preloader for the -jar case was a bug, so I
fixed it
-DrD-
[1] https://bugs.openjdk.java.net/browse/JDK-8017248
