<AWT Dev> [8] Review request for 7033533: realSync() doesn't work with Xfce

[Anthony Petrov]

Hi Oleg,

I second to Leonid: you should add a comment and explain why you expect 
exactly 4 (or more) events to be processed. Preferably, you should list 
each event to clearly understand this.

A minor comment is, lines 2404 - 2407 should be moved to the nearest 
try{} block at line 2409.

A major concern is that I'm not sure the new solution is reliable in all 
cases. Previously, we expected to get a notification from another 
process (the WM). Now we process the notification ourselves and are the 
owners of the selection. I don't know for sure, but suppose some xlib 
implementations could optimize this scenario and process events locally 
w/o even sending them to the X server. In which case there wouldn't be 
any real synchronization of the native event queue. That's why 
communicating with another process was an important part of this procedure.

How about we try the original method first, and only if it fails, then 
try this workaround solution?

Also, this is a very sensitive method because a lot of code relies on 
it. I suggest running all automatic regression tests for AWT and Swing 
areas to make sure we don't introduce a regression with this fix.

--
best regards,
Anthony

On 09/26/2013 11:56 AM, Oleg Pekhovskiy wrote:
> Hi Leonid,
>
> I did it mostly logically, because my fix added one more service event
> (SelectionRequest), that should be excluded from the number of
> dispatched native events.
> IMHO, the previous number "2" should have been commented, but, that did
> not happen.
>
> Thanks,
> Oleg
>
> On 25.09.2013 18:11, Leonid Romanov wrote:
>> Hi,
>> I'm not an expert in X11 programming, so I can't comment about the fix
>> in general, but I think the line 2436, "return getEventNumber() -
>> event_number > 3", really asks for a comment.
>>
>> On 9/25/2013 16:38, Oleg Pekhovskiy wrote:
>>> Hi all,
>>>
>>> please review the fix
>>> http://cr.openjdk.java.net/~bagiras/7033533.1/
>>> for
>>> https://bugs.openjdk.java.net/browse/JDK-7033533
>>>
>>> Previous implementation of XToolkit.syncNativeQueue() relied upon
>>> WM_S0 atom existence and that it was owned by current window manager.
>>> But several WMs (like XFCE and LXDE) don't send SelectionNotify event
>>> to the client on XConvertSelection() for that atom. That led
>>> XToolkit.syncNativeQueue() to hang until TimeOutException.
>>>
>>> I decided to keep XConvertSelection() usage, but make root toolkit
>>> window as an owner for selection atom (with another name), and handle
>>> SelectionRequest event from X Server, sending SelectionNotify in
>>> response (as window manager is supposed to do).
>>>
>>> Tested on both XFCE and LXDE.
>>>
>>> Thanks,
>>> Oleg
>>