Review request for 7129732 - [macosx] JCK failure: no focus transfer back to Window owner
Anthony Petrov
anthony.petrov at oracle.com
Tue Jan 31 03:37:50 PST 2012
Thanks for the ling, Leonid. Since both disposing and showing/hiding
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.
>>>>>
>>>>>
>
More information about the macosx-port-dev
mailing list