<AWT Dev> [8] Request for review: JDK-8020927 JLightweightFrame API should export layout properties change notifications

[Anton V. Tarasov]

On 19.07.2013 19:05, Anthony Petrov wrote:
> The updated fix looks fine. Thanks.
>
> I'd only suggest to test whether the initial content.*SizeChanged() calls in the setContent() are 
> enough, or they should be moved/duplicated somewhere else in case creating and showing of a 
> SwingNode instance is stretched in time, so as to make sure the FX code always operates with valid 
> values when the SwingNode gets added to a live scene or gets modified while still being inserted 
> into the scene.

What you're talking about relates to the javafx part of the fix (which I'll submit for review 
separately via the javafx review tool, and I'll add you as a reviewer =).
Just to answer your question. The layout data is stored in SwingNode. When it's added to scene after 
the content has been set on it, it will return valid values via its pref/max/minWidth/Heigh() 
methods which its containing layout pane will ask for while laying it out as a newly added node. If 
a content is set/replaced on already visible SwingNode, it will ask for relayout via calling 
impl_notifyLayoutBoundsChanged(). If you're suggesting to test it, let me leave that to SQE as they 
have "layout" in their SwingNode test plan. Because layout means quite a lot testcases...

Thanks,
Anton.

>
> -- 
> best regards,
> Anthony
>
> On 08/25/2013 06:53 PM, Anton V. Tarasov wrote:
>> Hi Anthony,
>>
>> Thanks for the review.
>>
>> On 19.07.2013 17:43, Anthony Petrov wrote:
>>> Hi Anton,
>>>
>>> The fix looks good overall.
>>>
>>> I'm just not sure about the exact symmetry between the setContent()
>>> method and the componentRemoved() listener wrt. the layoutSizeListener
>>> adding/removing. In theory, the setContent() may be called several
>>> times with different (or same) content objects. Similarly, the
>>> contentPane may in theory be added or removed manually several times
>>> (or even transferred between different JLFs).
>>
>> setContent() is the API method which we don't control (and which indeed,
>> in theory, may be called several times with any content).
>> But the content pane is what we manage internally. It's tight to the JLF
>> instance and follows its life cycle. (Yes, a developer may hack it, but
>> this is strongly discouraged =)
>>
>>>
>>> Perhaps both adding and removing the layoutSizeListener should happen
>>> in componentAdded() and componentRemoved() correspondingly?
>>
>> I like the idea. Here's the new version:
>>
>> http://cr.openjdk.java.net/~ant/JDK-8020927/webrev.1
>>
>> Thanks,
>> Anton.
>>
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>> On 07/19/2013 04:27 PM, Anton V. Tarasov wrote:
>>>> Please, review a fix.
>>>>
>>>> jira: https://jbs.oracle.com/bugs/browse/JDK-8020927
>>>> webrev: http://cr.openjdk.java.net/~ant/JDK-8020927/webrev.0
>>>>
>>>> Layout bounds notifications are added to internal JLightweightFrame API.
>>>>
>>>> (Just FYI, related fx changes are here:
>>>> http://cr.openjdk.java.net/~ant/RT-30650/webrev.0)
>>>>
>>>> Thanks,
>>>> Anton.
>>