Review Request: 8001533: Java launcher must launch JavaFX applications (original) (raw)

David DeHaven david.dehaven at oracle.com
Fri Nov 16 01:01:28 UTC 2012

java.c: L427 it would be helpful to add a comment to explain the case where appClass is different than mainClass. Probably the comment above L425 should be updated to reflect the support for JavaFX

Done.

L428-430: is this fallback needed? Would it be better if LauncherHelper.getApplicationClass() always returns a non-null class if the mainClass has been loaded successfully. Looks like this is the case in your implementation.

Good point, the helper would have aborted by that point. How about I change to NULL_CHECK(appClass) just for safety's sake?

LauncherHelper.java L746 missing space between 'if' and '(' The javadoc for the checkAndLoadMain method would need to be updated

Done, along with a couple more that Kumar found.

The change looks okay to me and I can't spot anything wrong there.

L496-517 somewhat duplicates the logic added for FX in the getMainClassFromJar method. Have you considered some refactoring work you could do to simplify the fix since I think once you get the classname of the entry point (either from a JAR or command-line and with and without the static void main method), the logic is essentially the same. To elaborate, I see that FXHelper.launchName L707-725 is needed mainly to give a useful error message. When you find the classname of the entry point, perhaps you can load the class and catch any linkage error and determine if it's caused by the absence of FX runtime and output an appropriate error. If the main class is successfully loaded, then proceed with L496-517 (or something like that). Just an idea you can explore.

Yes, I do feel that especially in the -jar case there is some repetition. The trouble is the ambiguity of ClassNotFoundException.

I'll poke at it and see what I can come up with.

FXLauncherTest.java - very good test that covers many test cases. Do you plan to add the classpath case (i.e. not from a jar file)?

I hadn't, but if it's worthwhile then we could certainly add a test to do so. Thoughts on this Steve?

-DrD-

More information about the core-libs-dev mailing list