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

[Prasanta Sadhukhan]

Hi Kevin,

Please find modified webrev fixing the other test failures by modifying 
the overrideNativeWindowHandle JNI method
http://cr.openjdk.java.net/~psadhukhan/fxswinterop/webrev.7/

Regards
Prasanta
On 7/10/2018 3:58 PM, Prasanta Sadhukhan wrote:
>
> 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
>>>>>>
>>>>>
>>>>
>>>
>>
>