[9] review request: 8131888: Deliver javafx.swt as a modular jar in JDK 9

[Mandy Chung]

> On May 26, 2016, at 8:38 AM, Kevin Rushforth <kevin.rushforth at oracle.com> wrote:
> 
> Yes, I've tested it in both modes (with a simple HelloFXCanvas program) -- as an automatic jar file and as just an ordinary jar on the classpath.

an automatic module needs to be on modulepath.

For container-like environment, it can create a Layer to load javafx.swt with a parent class loader that loads SWT classes - would that be feasible?

Mandy

> 
> -- Kevin
> 
> 
> Tom Schindl wrote:
>> Rereading the jira it take that back if javafx.swt can still be loaded as a simple jar things will work
>> 
>> Tom
>> 
>> Von meinem iPhone gesendet
>> 
>>   
>> 
>>> Am 26.05.2016 um 16:51 schrieb Tom Schindl <tom.schindl at bestsolution.at>
>>> :
>>> 
>>> Hi,
>>> 
>>> I highly doubt this will work in an OSGi-Env like Eclipse (which the 99%) use case for SWT useage.
>>> 
>>> The SWT jar is not on the application classpath so how should a module (named or unnamed) find the SWT classes?
>>> 
>>> Tom
>>> 
>>> Von meinem iPhone gesendet
>>> 
>>>     
>>> 
>>>> Am 26.05.2016 um 02:43 schrieb Mandy Chung <mandy.chung at oracle.com>
>>>> :
>>>> 
>>>> 
>>>> 
>>>>       
>>>> 
>>>>> On May 25, 2016, at 3:38 PM, Kevin Rushforth <kevin.rushforth at oracle.com>
>>>>>  wrote:
>>>>> 
>>>>> Please review the following:
>>>>> 
>>>>> 
>>>>> https://bugs.openjdk.java.net/browse/JDK-8131888
>>>>> http://cr.openjdk.java.net/~kcr/8131888/webrev.00/
>>>>> 
>>>>> 
>>>>> This adds support for the javafx.embed.swt package back into the JDK, which will be delivered as an automatic module in $JAVA_HOME/lib/javafx-swt.jar (final location is TBD).
>>>>>         
>>>>> 
>>>> The approach to have javafx.swt be an automatic module that can access org.eclipse.swt.* (that may be from an unnamed module) sounds reasonable.  I wonder what the JAR file should be named -  javafx.swt.jar or javafx-swt.jar?  They both have the same module name “javafx.swt”.
>>>> 
>>>> I skimmed through the change.  There are several System.err.println calls that I assume are debugging code to be removed. e.g.
>>>> 
>>>> FXCanvas.java
>>>> 247         System.err.println("FXCanvas class successfully initialized”);
>>>> 294                 System.err.println("FXCanvas: FX platform is initlialized”);
>>>> 
>>>> PlatformImpl.java
>>>> 308                 System.err.println("FXCanvas: no permission to access JavaFX internals");
>>>> 309                 ex.printStackTrace();
>>>> 
>>>> I reviewed mainly addExportsToFXCanvas and addExportsToFXCanvas methods.  Happy to see StackWalker be useful in this case.  The check to compare the class name with “javafx.embed.swt.FXCanvas” to derermine whether qualified exports should be added.  You can consider checking the caller's module name as a starter.  I know you are planning to look into the integrity check as a follows up.
>>>> 
>>>> ModuleHelper.java
>>>> 57             // ignore
>>>> 
>>>> This deserves to be an InternalError.  This is temporary until FX is transitioned to be built with JDK 9.
>>>> 
>>>> Otherwise, look fine to me.
>>>> Mandy
>>>>       
>>>> 
>> 
>>   
>>