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


Reply via email to