Review Request: 8001533: Java launcher must launch JavaFX applications

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


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





More information about the core-libs-dev mailing list