Thank you, Sergey. 

Please have a look at the updated version of the fix at:
http://cr.openjdk.java.net/~pchelko/8006634/webrev.01/

>  I suggest do not remove LWCToolkit.flushPendingEventsOnAppkit and reuse it 
> in CDropTargetContextPeer.flushEvents Is it possible?
Done.

> Can you please take a look to this 
> http://java.net/jira/browse/MACOSX_PORT-768 Looks like the workaround from 
> those issue is not necessary now.
There is another bug: http://bugs.sun.com/view_bug.do?bug_id=7199783 The 
changes in CCursorManager will be made in the fix for that bug.

> - It would be really helpful to add additional doce about runloops.
I have extended the documentation on the doAWTRunLoop method a bit, but I have 
now idea what else to write there.

> The test will be welcome, because this CR fix the possible deadlock, when we 
> use performOnMT inside DND .
The nested loop is run to synchronously process SunDropTargetEvents. There is 
no API to set a listener on this event without changing the JDK code.  When 
these events are processed, the new event (DropTargetEvent) is created which is 
that dispatched asynchronously. So I did not find any way to make a deadlock 
occur without hacking the JDK code. 

>  Synchronisation in CToolkitThreadBlockedHandler  is not necessary?
It extends a mutex, so the client is responsible for the synchronization. This 
is the same behavior as on other platforms.

> There are some changes related to VoiceOver  could you please check that it 
> works?
Yes, I've tried it for a bit, it seems to work.

>  I'm not sure but looks like stopAWTRunLoop should stop runloop synchronously 
> since after that we release CFRelease(mediatorObject). Is it possible that 
> when we call endRunLoop the object will be released?
The mediator object is retained two times, because it is used by 2 threads. 
Than it is released first time in endRunLoop and second time in the end of 
doAWTRunLoop, so it can't be already released when we end a runloop. There is 
no difference if we run stopRunLoop with waiting YES or NO, it is done on the 
main thread to avoid using @syncronized on a BOOL value. Making access to it 
@synchronized is about 10 times slower for some reason) 

With best regards. Petr.  

On Feb 14, 2013, at 1:18 PM, Sergey Bylokhov wrote:

> Hi, Petr.
> From the quick review:
> - I suggest do not remove LWCToolkit.flushPendingEventsOnAppkit and reuse it 
> in CDropTargetContextPeer.flushEvents Is it possible?
> - Can you please take a look to this 
> http://java.net/jira/browse/MACOSX_PORT-768 Looks like the workaround from 
> those issue is not necessary now.
> - It would be really helpful to add additional doce about runloops.
> - The test will be welcome, because this CR fix the possible deadlock, when 
> we use performOnMT inside DND .
> - Synchronisation in CToolkitThreadBlockedHandler  is not necessary?
> - There are some changes related to VoiceOver  could you please check that it 
> works?
> - I'm not sure but looks like stopAWTRunLoop should stop runloop 
> synchronously since after that we release CFRelease(mediatorObject). Is it 
> possible that when we call endRunLoop the object will be released?
> 
> 14.02.2013 10:58, Petr Pchelko wrote:
>> Hello, AWT Team.
>> 
>> Please review the fix for the issue:
>> http://bugs.sun.com/view_bug.do?bug_id=8006634
>> The fix is available at:
>> http://cr.openjdk.java.net/~pchelko/8006634/webrev.00/
>> 
>> This fix makes a lot of changes in nested event loops:
>> 
>> 1. CToolkitThreadBlockedHandler was made a subclass of Mutex as it is made 
>> on other platforms. Shared code relies on that with logic for 
>> entering/exiting nested event loops. Previously it did not matter as 
>> CToolkitThreadBlockedHandler did not make a actual nested loop. Now it makes 
>> a native nested loop via doAWTRunLoop.
>> 
>> 2. doAWTRunLoop method is changed
>>      a. Now we have a boolean flag processEvents. If it is set to false, we 
>> do not process native events while in the nested loop, same as how it was 
>> done before the fix. If it is set to true, events are processed while in the 
>> loop. Attention is needed when using this new feature, because we 
>> short-circuit native event processing and there could be some cases when it 
>> breaks Appkit. (resizing windows with a mouse could be an example) However 
>> we need this feature in the Drag and Drop.
>>      b. I have got rid of an feature to run the nested loop in 
>> NSDefaultRunLoopMode, it was never used.
>> 
>> 3. We decided that we are getting rid of the deadlock detection mechanism: 
>> it is rarely used, nobody have ever seen the error message from it. I have 
>> tested a lot without this feature and it looks like everything is fine, 
>> however I am absolutely not sure about this decision. I am including Peter 
>> and Ragini as Accessibility was using this functionality and I had to change 
>> Accessibility code a bit.
>> 
>> 4. Some native methods from the LWCToolkit were made package-private. The 
>> comment stated that deploy used them, but I've cloned there code and they 
>> apparently do not use it any more. Looks like the comment is from Apple 
>> times and we do not event have a WebkitPluginObject any more.
>> 
>> With best regards. Petr.
> 
> 
> -- 
> Best regards, Sergey.
> 

Reply via email to