[11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

[Prasanta Sadhukhan]

Hi All,

I have tried to address all comments from Nir, Phil, Mandy and Kevin got 
so far in this webrev
http://cr.openjdk.java.net/~psadhukhan/fxswing.7

 >>In SwingNode, lines 870 -883, is there a reason that only the first 
method uncommented @Override? I don't understand the comment that wants 
to skip @Override.
If @override is not added, then it will cause recursion between 
LightweightContent.java:130 and LightweightContent.java:187 thereby 
causing StackOverflowError. I am also not sure about the comment of 
skipping @Override but I have removed it from this webrev.

Regards
Prasanta
On 5/5/2018 4:32 AM, Kevin Rushforth wrote:
> My quick comments:
>
> 1. The changes to java.desktop module-info.java won't compile when 
> applied to jdk-client, since there is already a qualified export of 
> the sun.swing package to another internal class. Can you update your 
> patch to be based off of jdk-client? I note that it requires a small 
> patch to be able to build FX with JDK 11 (it's a known gradle issue), 
> but it's not hard to do.
>
> 2. As I mentioned in an off-line email, the final version of the 
> javafx.swing patch will need to use 'requires static 
> jdk.unsupported.desktop' in its module-info.java, so that it will 
> still run on JDK 10 EA. This means that you will need to ensure that 
> jdk.unsupported.desktop is loaded via a service loader as discussed. 
> This will not affect the public API at all, so that can proceed in 
> parallel, but this needs to be fixed.
>
> 3. As I mentioned in my earlier reply to Mandy's comment, Swing 
> interop apps will fail if a security manager is installed, because the 
> jdk.unsupported.desktop classes are loaded by the app class loader. 
> This can be sorted out later as a follow-on bug, with permissions 
> added to the default policy file, etc.
>
> 4. As Phil mentioned, some docs would be good. They don't need to be 
> all that detailed, since it isn't intended for use by applications, 
> and will not be included in the published API docs bundle (i.e., it 
> should be excluded from "make docs").
>
> I will do a more detailed review early next week.
>
> -- Kevin
>
> On 5/4/2018 1:53 PM, Phil Race wrote:
>> Two quick comments.
>>
>> 1) I'd like "Peer" removed from all the exported API.
>> 2) It is probably stable enough now that you can start to add some 
>> javadoc.
>>
>> -phil.
>>
>> On 05/04/2018 05:00 AM, Prasanta Sadhukhan wrote:
>>> Hi All,
>>>
>>> Please review an enhancement to remove the tight coupling of JDK 
>>> internal class from FX so that
>>> when javafx.* modules are removed from Openjdk build in jdk11, FX in 
>>> general, and fx swing interop, in particular, can still build and 
>>> function.
>>>
>>> Right now, FX uses 6 jdk internal packages
>>> sun.swing, sun.awt, java.awt.dnd.peer, sun.awt.dnd, sun.awt.image 
>>> and sun.java2d.
>>>
>>> However, the goal is not to use qualified exports of these internal 
>>> packages once FX is removed to be built along with JDK,
>>>
>>> The solution will define a new "jdk.unsupported.desktop" module that 
>>> exports public API
>>> that is intended to be used by the javafx.swing module (but it does 
>>> so with public exports and not qualified exports).
>>> The idea is the same as jdk.unsupported, in that it is documented as 
>>> being unsupported.
>>> Further, since it is only intended to be used by javafx.swing, it 
>>> need not be in the default module graph.
>>>
>>> The module-info.java will look like this:
>>>
>>> |module jdk.unsupported.desktop { requires transitive java.desktop; 
>>> exports jdk.swing.interop; } |||
>>>
>>> Enhancement: https://bugs.openjdk.java.net/browse/JDK-8202199, 
>>> https://bugs.openjdk.java.net/browse/JDK-8195811
>>> webrev: cr.openjdk.java.net/~psadhukhan/fxswing.6/
>>> CSR: https://bugs.openjdk.java.net/browse/JDK-8202175
>>>
>>> Regards
>>> Prasanta
>>>
>>> ||
>>>
>>>
>>
>