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

[Oleg Pekhovskiy]

Hi Leonid,

On 01.10.2013 16:01, Leonid Romanov wrote:
> By the way, I was reading Inter-Client Communication Conventions Manual
> so I could better understand the fix, and found the following:
>
> "4.3. Communication with the Window Manager by Means of Selections
>
> For each screen they manage, window managers will acquire ownership of a
> selection named WM_Sn, where n is the screen number, as described in
> section 1.2.6. Window managers should comply with the conventions for
> "Manager Selections" described in section 2.8. The intent is for clients
> to be able to request a variety of information or services by issuing
> conversion requests on this selection."
>
> Considering this, can we say that Xfce is not complying to the spec?

not really, because Xfce HAS "VESRION" target for "WM_S0" selection, but 
in spite of the fast we request this atom as a target, we ask for "OOPS" 
property which is not mentioned anywhere.

Thanks,
Oleg

>
> On 10/1/2013 15:29, Anthony Petrov wrote:
>> 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
>>>>
>