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

Kumar Srinivasan kumar.x.srinivasan at oracle.com
Fri Nov 16 16:56:23 UTC 2012

Previous message: Review Request: 8001533: Java launcher must launch JavaFX applications
Next message: Request for Review : 7175464 : entrySetView field is never updated in NavigableSubMap
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Mandy, Thanks Mandy!, that tip cleaned up the code quite a bit, it is generally looking a lot better.

David, One minor fix the while loop can be converted to a for loop making it slightly more compact, But I am fine either way.

       Class<?> sc = mainClass.getSuperclass();

```
       while (sc != null) {
```

       for (Class<?> sc = mainClass.getSuperclass(); sc != null;

               sc = sc.getSuperclass()) {
            if (sc.getName().equals(JAVAFX_APPLICATION_CLASS_NAME)) {
                return true;
            }

           sc = sc.getSuperclass();
        }

Steve, the case of jfxrt.jar not being on the class path, we currently pass vacuously. Instead, I think we can have a couple of negative tests, this way we can ensure the launcher behavior and error messages under these conditions. We can do this as a bug fix in M6.

Thanks Kumar

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. That's good. Mandy I cleaned it up quite a bit, I think it looks a lot better now: http://cr.openjdk.java.net/~ddehaven/8001533/webrev.1/ The comments still need some attention, I'll get that first thing on the morrow. -DrD-

Previous message: Review Request: 8001533: Java launcher must launch JavaFX applications
Next message: Request for Review : 7175464 : entrySetView field is never updated in NavigableSubMap
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

More information about the core-libs-dev mailing list