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

[Anton V. Tarasov]

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/e65455aa/attachment-0001.html>