[11] RFR: Enhancement: JDK-8195811:Support FX Swing interop using public API

[Prasanta Sadhukhan]

Hi Kevin,

Please find modified webrev with some more refactoring to move more 
common code to SwingNode
and also this solves SwingNodeMemoryLeakTest failure
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.6/

Regards
Prasanta
On 7/10/2018 7:13 AM, Kevin Rushforth wrote:
> Hi Prasanta,
>
> The public API looks fine now. I sent you an offline note about one of 
> the test failures (SwingNodeMemoryLeakTest).
>
> There are still whitespace problems that will cause a failure in 
> 'gradle checkrepo' (and 'hg jcheck').
>
> I'll do a more thorough review in the next day or so.
>
> -- Kevin
>
>
> On 7/9/2018 4:12 AM, Prasanta Sadhukhan wrote:
>>
>> Hi Kevin,
>>
>> Modified webrev to address the "public" leakage
>> http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.5/
>>
>> I am looking into the test failures.
>>
>> Regards
>> Prasanta
>> On 7/7/2018 4:30 AM, Kevin Rushforth wrote:
>>> Most things are working with either mode (JDK 10 with qualified 
>>> exports or JDK 11 without), although I do get a few test failures.
>>>
>>> There is a serious issue with leakage into the public API that needs 
>>> to be addressed before worrying about the failures, but I'll list 
>>> the test failures at the end.
>>>
>>> SwingNode.java:
>>>
>>> This public class is part of the API. You cannot make any of the 
>>> following fields public as this would leak implementation into 
>>> public API:
>>>
>>> +    public int swingPrefWidth;
>>> +    public int swingPrefHeight;
>>> +    public int swingMaxWidth;
>>> +    public int swingMaxHeight;
>>> +    public int swingMinWidth;
>>> +    public int swingMinHeight;
>>>
>>> +    public final Object getLightweightFrame() { return lwFrame; }
>>>
>>> +    public final ReentrantLock paintLock = new ReentrantLock();
>>>
>>> +    public boolean grabbed; // lwframe initiated grab
>>>
>>> +    public void setImageBuffer(...)
>>>
>>> +   public void setImageBounds(...);
>>>
>>> +    public void repaintDirtyRegion(...)
>>>
>>> +    public void ungrabFocus(boolean postUngrabEvent)
>>>
>>> If you need to access them from other packages, you can either use 
>>> the accessor pattern (this might be easiest) or else refactor it 
>>> further to move more of this down to the implementation. I note that 
>>> even though SwingNodeInterop is abstract, it can still have 
>>> non-abstract methods if that helps in your refactoring.
>>>
>>>
>>> SwingFXUtils.java
>>>
>>> Same problem as SwingNode, although to a lesser extent. The 
>>> following must not be public:
>>>
>>> +    public static void runOnFxThread(Runnable runnable)
>>> +    public static void runOnEDT(final Runnable r)
>>> +    public static void runOnEDTAndWait(Object nestedLoopKey, 
>>> Runnable r)
>>> +    public static void leaveFXNestedLoop(Object nestedLoopKey)
>>>
>>>
>>> JFXPanel is fine.
>>>
>>> -----------------
>>>
>>> * System tests failures on Linux:
>>>
>>> test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
>>> testJDialogAbove FAILED
>>>     java.lang.AssertionError: JDialog is not above JavaFX stage 
>>> expected:<java.awt.Color[r=0,g=0,b=255]> but 
>>> was:<java.awt.Color[r=0,g=128,b=0]>
>>>
>>> test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
>>> testNodeRemovalAfterShow FAILED
>>>     java.lang.AssertionError: JDialog is not above JavaFX stage 
>>> expected:<java.awt.Color[r=0,g=0,b=255]> but 
>>> was:<java.awt.Color[r=0,g=128,b=0]>
>>>
>>> test.robot.javafx.embed.swing.SwingNodeJDialogTest > 
>>> testStageCloseAfterShow FAILED
>>>     java.lang.AssertionError: JDialog is not above JavaFX stage 
>>> expected:<java.awt.Color[r=0,g=0,b=255]> but 
>>> was:<java.awt.Color[r=0,g=128,b=0]>
>>>
>>> test.javafx.embed.swing.SwingNodeMemoryLeakTest > 
>>> testSwingNodeMemoryLeak FAILED
>>>     java.lang.AssertionError: expected:<10> but was:<9>
>>>         at org.junit.Assert.fail(Assert.java:91)
>>>         at org.junit.Assert.failNotEquals(Assert.java:645)
>>>         at org.junit.Assert.assertEquals(Assert.java:126)
>>>         at org.junit.Assert.assertEquals(Assert.java:470)
>>>         at org.junit.Assert.assertEquals(Assert.java:454)
>>>         at 
>>> test.javafx.embed.swing.SwingNodeMemoryLeakTest.testSwingNodeMemoryLeak(SwingNodeMemoryLeakTest.java:97)
>>>
>>> Two of these, SwingNodeJDialogTest.testNodeRemovalAfterShow and 
>>> SwingNodeJDialogTest.testStageCloseAfterShow, also fail on Mac
>>>
>>> -- Kevin
>>>
>>>
>>> On 7/5/2018 11:29 PM, Prasanta Sadhukhan wrote:
>>>>
>>>> My Bad. Please find modified webrev restoring the filter
>>>>
>>>> http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.4/
>>>>
>>>> Regards
>>>> Prasanta
>>>> On 7/6/2018 6:47 AM, Kevin Rushforth wrote:
>>>>> One quick comment:
>>>>>
>>>>> This no longer compiles with OpenJDK10. It looks like the logic to 
>>>>> optionally filter out jdk.unsupported.desktop from javafx.swing's 
>>>>> module-info.java got lost between the .2 and .3 versions.
>>>>>
>>>>> -- Kevin
>>>>>
>>>>>
>>>>> On 7/4/2018 4:35 AM, Prasanta Sadhukhan wrote:
>>>>>> Hi All,
>>>>>>
>>>>>> Please review an enhancement to support openjfx swing 
>>>>>> interoperability once the dependancy of internal jdk classes are 
>>>>>> removed.
>>>>>> JDK-8202199 <https://bugs.openjdk.java.net/browse/JDK-8202199> 
>>>>>> provided a new "jdk.unsupported.desktop" module in JDK 11 that 
>>>>>> exports public API that is intended to be used by the 
>>>>>> javafx.swing module
>>>>>> and unbundled OpenJFX is now made to depend on these APIs to 
>>>>>> support interoperation
>>>>>> between Swing and JavaFX components to replace previous use of 
>>>>>> internal APIs when it was part of Oracle JDK.
>>>>>>
>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8195811
>>>>>> webrev: http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.3/
>>>>>>
>>>>>> Regards
>>>>>> Prasanta
>>>>>
>>>>
>>>
>>
>