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

[Anthony Petrov]

On 1/31/2012 3:37 PM, Anthony Petrov wrote:
> Thanks for the ling, Leonid. Since both disposing and showing/hiding 

s/ling/link/

--
best regards,
Anthony

> must happen on the AppKit thread, I think it makes sense to dispatch the 
> operations directly to that thread rather than using the EDT for this 
> purpose.
> 
> But again, this change requires very careful testing.
> 
> -- 
> best regards,
> Anthony
> 
> On 1/31/2012 1:58 PM, Leonid Romanov wrote:
>> Hi Anton,
>> Regarding your question, see
>> http://java.net/jira/browse/MACOSX_PORT-429
>>
>>
>> On 31.01.2012, at 17:12, 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.
>>>>>>
>>>>>>
>>