<AWT Dev> [14] Review Request: 8236953 [macos] JavaFX SwingNode is not rendered on macOS

[Phil Race]

OK. Approved.

I wonder how much of a hole we have in our testing where code that
matters only to FX is not tested anywhere since FX was decoupled.

And please file a bug for Linux if there is not one already as noted in 
my first email.

 >>and probably should do this on Linux(it is currently works because on 
Linux AWT posts too many events).

 > Is there a bug filed on this ?

-phil.

On 1/15/20 9:32 AM, Sergey Bylokhov wrote:
> On 1/15/20 9:11 am, Phil Race wrote:
>> You say this is for 14, but it is not a 14 regression.
>> Seems like it would be nice to fix but clearly not many people are 
>> running FX on top of the latest JDK.
>>
>>
>> Can you please join the dots for me on how a change of false to true 
>> in setBounds> is the way to fix it ? It is not obvious.
>
> That flag is responsible of updating the target(usually noop) and 
> posting events after the size of the peer
> is changed.
>
> setBounds() will call handleMove() if the component was moved
> https://github.com/openjdk/client/blob/master/src/java.desktop/macosx/classes/sun/lwawt/LWComponentPeer.java#L597 
>
>
> and handleMove() will post event if the flag is "true"
> https://github.com/openjdk/client/blob/master/src/java.desktop/macosx/classes/sun/lwawt/LWComponentPeer.java#L1124 
>
>
>>
>> What testing have you done to ensure there are no other regressions 
>> either on macos or other OSes ?
>
> The fix in question affects the macOS only, and the file which was 
> changed is used by the FX only.
> We have only one test which uses JLightweightFrame and it passes, I 
> have run client-tests on all
> platforms just to be sure that no unexpected issues were found.
>
>> On 1/14/20 8:10 PM, Sergey Bylokhov wrote:
>>> Hello.
>>> Please review the fix for JDK 14.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8236953
>>> Fix: http://cr.openjdk.java.net/~serb/8236953/webrev.00
>>>
>>> The Swing components depends on the COMPONENT_RESIZED and
>>> COMPONENT_MOVED posted by the AWT. These events are posted
>>> in three situations:
>>>  1. If the Component is invisible then events are always posted
>>>     by the Component class.
>>>  2. If the Component is visible and the size is changed by the
>>>     user via setBounds() method then events are posted by the
>>>     component only if the component is not Window.
>>>  3. If the Component is visible and the size is changed by the
>>>     OS then the peer will post these events.
>>>
>>> Note that in point 2. we skip any top-level windows because we
>>> know that sometime later we will get a native callback and will
>>> post events at point 3.
>>>
>>> In JDK-8144125 
>> I note that fix had only one reviewer. We should have two unless 
>> there's a good reason not to.
>>
>>> I have fixed a bug when we always posted these
>>> evens at point 2., and as a result, we got duplicated events after
>>> point 3. But the fix caused current regression, the JLightweightFrame
>>> which is used by the SwingNode is a kind of top-level window but w/o
>>> the native part, so it will never get a native callback and should post
>>> events ourself, we already do this on windows, and probably should do
>>> this on Linux(it is currently works because on Linux AWT posts too
>>> many events).
>>
>> Is there a bug filed on this ?
>>
>> -phil.
>>>
>>>
>>
>
>