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

[Philip Race]

PS .. there should be some tests on the JDK side that don't use FX
Or at least a reasoned explanation as to why this isn't possible.

-phil.

On 5/10/18, 5:49 PM, Philip Race wrote:
> I've looked over the Swing code. No time to actually try it out so I 
> can only comment on what I see.
>
> I am not sure what I think about class docs like "This class wraps 
> <names internal interface> .."
> I know no javadoc is generated but this module is exported and 
> probably should say something
> less specific.
>
> In general I really have to hold my nose when looking at this whole 
> module and I
> find it distasteful to endorse it.
>
> I really don't like all the things SwingInterOpUtils.java does.
>
> I worry we are creating something we'll come to regret here.
> The "unsupportedness" needs to be backed up by deprecated-for-removal 
> being included
> today so we can get rid of it the next release if we want to.
>
> No @since tags anywhere....
>
> -phil.
>
> On 5/10/18, 8:32 AM, Kevin Rushforth wrote:
>> I confirm that this fixes the issue. The interface change to make the 
>> two static methods e instance methods is a good change in any case.
>>
>> I am not a Swing "R"eviewer, but the .13 webrev gets a +1 from me.
>>
>> -- Kevin
>>
>>
>> On 5/10/2018 8:20 AM, Prasanta Sadhukhan wrote:
>>>
>>> Hi Kevin,All,
>>>
>>> Please find the modified webrev fixing this #1 issue
>>> http://cr.openjdk.java.net/~psadhukhan/fxswing.13/
>>> via change in 
>>> jdk/swing/interop/DropTargetContextWrapper.java#setDropTargetContext 
>>> and FXDnD.java#postDropTargetEvent
>>>
>>> For me, #2 works, #3 doesn't work even now due to JDK-8141391 
>>> <https://bugs.openjdk.java.net/browse/JDK-8141391> and #4 works for me.
>>>
>>> Regards
>>> Prasanta
>>> On 5/9/2018 11:29 PM, Kevin Rushforth wrote:
>>>> Hi Prasanta,
>>>>
>>>> The API looks good now.
>>>>
>>>> All of our automated tests work (except for the ones with a 
>>>> security manager due to JDK-8202451 
>>>> <https://bugs.openjdk.java.net/browse/JDK-8202451>).
>>>>
>>>> The only functional problem that I see is that Drag and Drop onto a 
>>>> SwingNode doesn't work. We need to make sure that we test the 
>>>> following four cases:
>>>>
>>>> 1. Drag / drop onto a Swing component in a SwingNode
>>>> 2. Drag / drop from a Swing component in a SwingNode
>>>> 3. Drag / drop onto a JavaFX control in a JFXPanel
>>>> 4. Drag / drop from a JavaFX control in a JFXPanel
>>>>
>>>> So far I only tried the first one; the others still need to be 
>>>> validated.
>>>>
>>>> -- Kevin
>>>>
>>>>
>>>>
>>>> On 5/9/2018 7:14 AM, Prasanta Sadhukhan wrote:
>>>>> Modified webrev to cater to this
>>>>>
>>>>> http://cr.openjdk.java.net/~psadhukhan/fxswing.12/
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>> On 5/9/2018 5:58 PM, Kevin Rushforth wrote:
>>>>>> The following can also be abstract:
>>>>>>
>>>>>> LightweightContentWrapper:
>>>>>>   getComponent, createDragGestureRecognizer, 
>>>>>> createDragSourceContextPeer
>>>>>>
>>>>>> DropTargetContextWrapper:
>>>>>>   getTargetActions, getDropTarget, getTransferDataFlavors, 
>>>>>> getTransferable, isTransferableJVMLocal
>>>>>>
>>>>>> DispatcherWrapper:
>>>>>>   isDispatchThread, createSecondaryLoop
>>>>>>
>>>>>> The rest looks good to me (although I still see two public 
>>>>>> methods with "Peer" in the name, so Phil may want those renamed).
>>>>>>
>>>>>> -- Kevin
>>>>>>
>>>>>>
>>>>>> On 5/9/2018 2:14 AM, Prasanta Sadhukhan wrote:
>>>>>>> Modified webrev to cater to these 3 observations
>>>>>>> http://cr.openjdk.java.net/~psadhukhan/fxswing.11/
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>>
>>>>>>> On 5/9/2018 5:03 AM, Kevin Rushforth wrote:
>>>>>>>> The module definition for jdk.unsupported.desktop and the 
>>>>>>>> changes to java.desktop look fine.
>>>>>>>>
>>>>>>>> In reviewing the jdk.swing.interop API, I have the following 
>>>>>>>> suggestions / observations:
>>>>>>>>
>>>>>>>> 1. DispatcherWrapper, DragSourceContextWrapper, 
>>>>>>>> DropTargetContextWrapper, and LightweightContentWrapper can all 
>>>>>>>> be abstract, along with most of the methods (rather than having 
>>>>>>>> an empty body return value that is never used).
>>>>>>>>
>>>>>>>> 2. The addNotify method in LightweightFrameWrapper is unused. 
>>>>>>>> Should be used somewhere? If not, then it can be removed.
>>>>>>>>
>>>>>>>> The implementation of the new wrapper classes looks OK to me 
>>>>>>>> with one observation that might or might not matter:
>>>>>>>>
>>>>>>>> 3. The behavior of getDefaultScaleX/Y (which is now in 
>>>>>>>> SwingInteropUtils) has changed in the case where the Graphics 
>>>>>>>> is not an instance of SunGraphics2D. The former behavior was to 
>>>>>>>> leave the instance variables X and Y unchanged. The new 
>>>>>>>> behavior will set them back to 1.0. Maybe this can't happen in 
>>>>>>>> practice, but it is something to consider.
>>>>>>>>
>>>>>>>> -- Kevin
>>>>>>>>
>>>>>>>>
>>>>>>>> On 5/8/2018 3:31 AM, Alan Bateman wrote:
>>>>>>>>> On 08/05/2018 06:51, Prasanta Sadhukhan wrote:
>>>>>>>>>> Modified webrev to rename to InteropProviderImpl
>>>>>>>>>>
>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/fxswing.10/
>>>>>>>>> This looks okay to me.
>>>>>>>>>
>>>>>>>>> -Alan
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>