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

[Leonid Romanov]

I share Anthony's concern that the fix might not be reliable. There is a 
new generation of window systems on the way, like Wayland or Mir, which 
provide xlib compatibility layer on top of its own protocol, so they 
might implement the optimization described by Anthony.

On 10/1/2013 17:21, Yuri Nesterenko wrote:
> Leonid, I think you would not deny that fix is elegant even if risky.
> Adding a sniffer will make it ugly; besides, modern lightweight
> desktop environments become wildly popular really fast --
> look at LXDE -- much faster than Java version juggernaut is
> changing version. What to sniff?
> Perhaps we would have to file bugs in too many bug tracking systems.
> "ICCCM establishes conventions, which are basically suggestions."
>
> Thanks,
> -yan
>
> 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
>>>>>>>
>>>>
>>>
>>
>