Review request for 7129732 - [macosx] JCK failure: no focus transfer back to Window owner

[Anton V. Tarasov]

On 31.01.2012 13:53, Artem Ananiev wrote:
>
> The fix looks fine.
>
> Unrelated question: should LWWindowPeer.updateFocusableWindowState() be called before 
> platformWindow.setVisible()?

Due to a simple window is unfocusable in terms of cocoa, its showing doesn't make it focused. So, 
the order of the above calls is actually unimportant.
What is important is to call updateFocusableWindowState() before changeFocusedWindow(...).

Thanks,
Anton.

>
> Thanks,
>
> Artem
>
> On 1/31/2012 5:12 PM, Anton V. Tarasov wrote:
>> Hi Anthony,
>>
>> On 1/30/12 6:30 PM, Anthony Petrov wrote:
>>> Hi Anton,
>>>
>>> On 01/30/12 16:05, Anton V. Tarasov wrote:
>>>> A question to the reviewers. Do we still need to invoke on EDT?
>>>> 236 // it is important to call this method on EDT
>>>> 237 // to prevent the deadlocks during the painting of the lightweight
>>>> delegates
>>>> 238 //TODO: WHY? This is a native-system related call. Perhaps NOT
>>>> calling
>>>> 239 // the painting procedure right from the setVisible(), but rather
>>>> relying
>>>> 240 // on the native Expose event (or, scheduling the repainting
>>>> asynchronously)
>>>> 241 // is better?
>>>> 242 SwingUtilities.invokeLater(new Runnable() {
>>>> 243 @Override
>>>> 244 public void run() {
>>>> 245 platformWindow.setVisible(visible);
>>>
>>> This "WHY?..." comment was added by me long time ago. If all the
>>> invoked methods may be called on other threads I don't see a reason to
>>> dispatch this code to the EDT. However, if you wish to remove this
>>> invokeLater(), you'll have to thoroughly test that nothing is broken.
>>>
>>> We might want to ask Alex (CC'ed) since he added this invokeLater()
>>> originally IIRC. Is this still relevant? Doesn't the painting
>>> operation gets dispatched to the EDT automatically? May there still be
>>> any painting artifacts should the platformWindow.setVisible() be
>>> invoked on non-EDT thread?
>>
>> Oh, I see that I'm better to separate this investigation (due to the
>> urgency of the fix). Thanks for your comments!
>>
>>>
>>>
>>>
>>>>> Please review a fix for 7129732.
>>>>>
>>>>> webrev: http://cr.openjdk.java.net/~ant/7129732/webrev.0/
>>>
>>> At line 255 a check for !visible is unnecessary since the else branch
>>> already ensures that visible is false.
>>
>> Indeed. I added the first if-statement and forget to refine the second
>> one. Thanks for catching this.
>>
>> Please, see the improved version:
>>
>> http://cr.openjdk.java.net/~ant/7129732/webrev.1
>>
>> Thanks,
>> Anton.
>>
>>>
>>> Otherwise looks good.
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>>
>>>>>
>>>>> When a focused simple window is getting closed, focus should be
>>>>> transferred to the owner.
>>>>>
>>>>> Thanks,
>>>>> Anton.
>>>>>
>>>>>
>>>>
>>