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

[artem malinko]

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?

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/0f758513/attachment.html>