<AWT Dev> Review request for 8047288: [macosx] Endless loop in EDT on Mac

[Anton V. Tarasov]

Artem,

I've just noticed the Window.isFocusableWindow method is _final_ . So, that was a false alert, 
sorry. And thanks for the check.
No more questions from me.

Regards,
Anton.

On 24.07.2014 15:37, artem malinko wrote:
> Call stack and sysout show that in case of applet Window.setVisible() is called from the main 
> thread or from EDT thread. So I think toolkit thread won't call this method.
>
> On 7/24/2014 2:02 PM, Anton V. Tarasov wrote:
>> On 24.07.2014 13:48, artem malinko wrote:
>>> Hi Anton.
>>>
>>> Sorry, didn't understand you well. Do you mean we should check(and test) that this 
>>> method(isFocusableWindow()) is also called on EDT if we run applet?
>>
>> IMHO, it's enough to just make sure (via inspecting the sources) that Window.setVisible() is 
>> called on EDT in case of an applet. That logic is triggered on the plugin side.
>>
>> Anton.
>>
>>>
>>> On 7/24/2014 11:42 AM, Anton V. Tarasov wrote:
>>>> Hi Artem,
>>>>
>>>> I'm Ok with the fix, provided that LWWindowPeer.setVisibleImpl() is called on EDT for applets 
>>>> (if I'm not mistaken). Otherwise, we still have a client method 
>>>> (getTarget().isFocusableWindow()) called on the toolkit thread, which is generally no good.
>>>>
>>>> Regards,
>>>> Anton.
>>>>
>>>> On 24.07.2014 10:25, artem malinko wrote:
>>>>> Thank you, Petr.
>>>>>
>>>>> Could anyone else review the fix, please?
>>>>>
>>>>> On 7/23/2014 10:30 PM, Petr Pchelko wrote:
>>>>>> Hello, Artem.
>>>>>>
>>>>>> The new version (it’s .00 for some reason) looks good.
>>>>>>
>>>>>> With best regards. Petr.
>>>>>>
>>>>>>> On Jul 23, 2014, at 6:55 PM, artem malinko <artem.malinko at oracle.com 
>>>>>>> <mailto:artem.malinko at oracle.com>> wrote:
>>>>>>>
>>>>>>> Hi, Petr. I ran focus regression tests and jck tests on awt. For fixed jdk results is the 
>>>>>>> same. Except my new test, of course which is not passed on not fixed jdk:) And also I fixed 
>>>>>>> issues in test. New webrev:
>>>>>>> http://cr.openjdk.java.net/~bae/8047288/9/webrev.00/
>>>>>>>
>>>>>>> On 7/22/2014 8:23 PM, Petr Pchelko wrote:
>>>>>>>> Hello, Artem.
>>>>>>>>
>>>>>>>> A couple of comments:
>>>>>>>> 1. LWWindowPeer: 268 - please remove an empty line.
>>>>>>>> 2. LWWIndowPeer. isTargetFocusable - the method is not needed at all.
>>>>>>>> 3. I’m concerned about the test. Do you really need the close button?
>>>>>>>> 4. frame and window variables are set from main thread and read from EDT. They should be 
>>>>>>>> declared volatile.
>>>>>>>>
>>>>>>>> Also please run all focus regression and JCK tests, because this area is very sensitive.
>>>>>>>>
>>>>>>>> With best regards. Petr.
>>>>>>>>
>>>>>>>>> On Jul 22, 2014, at 8:04 PM, artem malinko <artem.malinko at oracle.com 
>>>>>>>>> <mailto:artem.malinko at oracle.com>> wrote:
>>>>>>>>>
>>>>>>>>> Hello, AWT Team.
>>>>>>>>>
>>>>>>>>> Please review a fix for the issue:
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8047288
>>>>>>>>> The fix is available at:
>>>>>>>>> http://cr.openjdk.java.net/~mcherkas/artem/8047288/webrev.01/ 
>>>>>>>>> <http://cr.openjdk.java.net/%7Emcherkas/artem/8047288/webrev.01/>
>>>>>>>>>
>>>>>>>>> Window.isFocusableWindow() could lead to deadlock if it is invoked on AppKit thread. Fix 
>>>>>>>>> caches result of Window.isFocusableWindow() on a peer level and method is not invoked on 
>>>>>>>>> AppkitThread.
>>>>>>>>>
>>>>>>>>> Thank you.
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20140724/ce23e784/attachment-0001.html>