Review Request: 8001533: Java launcher must launch JavaFX applications

[Mandy Chung]

Hi David,

On 11/14/12 9:43 AM, David DeHaven wrote:
> Bug:
> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8001533
>
> Webrev:
> http://cr.openjdk.java.net/~ddehaven/8001533/webrev.0/

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

     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.

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

    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.

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)?

Mandy