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

[Anthony Petrov]

I agree with Leonid on both points.

--
best regards,
Anthony

On 10/01/2013 04:38 PM, Leonid Romanov wrote:
> I agree with you, but it would change our approach to the fix: for
> instance, we might want to use the new realSync() version for Xfce only.
> Also, we should a bug in Xfce bug tracking system then.
>
> On 10/1/2013 16:27, Yuri Nesterenko wrote:
>> Sure we can say that Xfce is not complying -- it is not officially
>> supported by JDK -- neither is LXDE etc. -- but saying that gets
>> us nowhere.
>>
>> As to testing, could you suggest a platform selection? I'm afraid
>> we'll not be able to test Xsun properly but Xorg with Gnome on
>> Linux and Solaris, Unity and Xfce4 --
>> all that we can do by the weekend.
>>
>> Thanks,
>> -yan
>>
>> On 10/01/2013 04:01 PM, 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?
>>>
>>> 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
>>>>>>
>>>
>>
>