Hello everyone, After a long hiatus, I'm trying again to get XEmbed systray support into Wine. I've been working on it for the past few days and I've got these patches. These are based on the system tray code from the Wine version used in the Linux port of Picasa [1] (which in turn was based on the work of Mike Hearn and Rob Shearman.) These patches are basically the same thing, with some fixes:
1. Use proper wine_tsx11 locking. 2. Clean up inappropriate C++ comments in that code. 3. Properly populated the XEvent structure to send docking messages. (According to the XEmbed systray spec [2], any unused fields in the message must be set to 0.) 4. Broken into five atomic patches against today's GIT.
In my testing on Gnome 2.14, these patches dock much more reliabaly than the last ones, but they're still not quite perfect. In most cases, programs dock fine until about the seventh consecutive run (behavior observed with Steam and a small test program I wrote.) However, I noticed that it works perfectly if I enabled a +x11drv trace, so I suspect that this has something to do with stack problems, but I've been unable to find the source of the problem. I'd appreciate it if someone could give me some assistance on tracking down this issue.
If you'd like to test these, please apply them in the order that they're numbered. Any comments/suggestions appreciated.
Thnaks, James Liggett
[1]: http://code.google.com/wine.html [2]: http://standards.freedesktop.org/systemtray-spec/systemtray-spec-0.2.html#me...
On Sun, 06 Aug 2006 20:10:19 -0700, James Liggett wrote:
so I suspect that this has something to do with stack problem
More likely it's a speed issue - logging slows the code down a lot which could "fix" a race condition. X is sort of susceptible to this kind of thing it seems.
On Mon, 2006-08-07 at 18:02 +0100, Mike Hearn wrote:
On Sun, 06 Aug 2006 20:10:19 -0700, James Liggett wrote:
so I suspect that this has something to do with stack problem
More likely it's a speed issue - logging slows the code down a lot which could "fix" a race condition. X is sort of susceptible to this kind of thing it seems.
Interesting thought...I knew that logging could slow down code, but I didn't think it could slow it down so much that it actually allowed the icon to dock. When I last tried working with these, I don't recall the trace working to solve this problem (I could be wrong though) Nonetheless, these patches seem to work better than the last ones, but I don't really understand why that is.
On Mon, 2006-08-07 at 18:02 +0100, Mike Hearn wrote:
On Sun, 06 Aug 2006 20:10:19 -0700, James Liggett wrote:
so I suspect that this has something to do with stack problem
More likely it's a speed issue - logging slows the code down a lot which could "fix" a race condition. X is sort of susceptible to this kind of thing it seems.
Turns out you're right, Mike. If I add a small (2 ms) sleep after the dock event is sent, things work perfectly. :) But, this really strikes me as a hack that doesn't stand a chance of getting into Wine. Is there a better way to slow down the execution of this code.
Thanks.
On 8/7/06, James Liggett jrliggett@cox.net wrote:
Turns out you're right, Mike. If I add a small (2 ms) sleep after the dock event is sent, things work perfectly. :) But, this really strikes me as a hack that doesn't stand a chance of getting into Wine. Is there a better way to slow down the execution of this code.
It's not that we need to slow it down - slowing something down is never an acceptable solution to a race. We are missing some kind of synchronisation somewhere, or perhaps we are doing something funny to the window immediately after we try and dock it, which causes it to undock. You need to trace exactly what happens to that window after it's docked.
thanks -mike
On Tue, 2006-08-08 at 13:48 +0100, Mike Hearn wrote:
It's not that we need to slow it down - slowing something down is never an acceptable solution to a race. We are missing some kind of synchronisation somewhere,
Actually, I don't think it has to do with synchronization--I think it has to do with window mapping. Today I did some more research on the issue, and I came across the XEmbed spec, and there's a pretty interesting tidbit in there [1] (look under the XEMBED_MAPPED section.) The problem is that the systray window has to be unmapped so that when we dock, the embedder (the systray applet that holds the icon) has to map the client (the systray icon) itself--we can't do it in WINE or we risk having the race conditions that we do (where the window becomes visible as a child of the root window and not the tray, or sometimes both the root window and tray)
So here's the solution that I'm currently using to solve it: don't call XMapWindow on systray icon windows. That combined with the mapped flag in the XEMBED_INFO array we set just before we dock the icon allows it to work properly. I've run my small test app over 50 times straight (probably more by now--I've lost count,) and it works. I'll do some more tests to make sure everything is good, and hone my solution a little before submitting. The problem with this is that we probably want mapping if we don't have a tray to send our icon to, so I'll be playing with getting that to work cleanly.
James
Notes: [1]: http://standards.freedesktop.org/xembed-spec/latest/ar01s04.html
On 8/9/06, James Liggett jrliggett@cox.net wrote:
Actually, I don't think it has to do with synchronization--I think it has to do with window mapping.
Ah awesome, good work! I know when I looked at this originally it seemed you could set a flag that told the embedder whether it was already mapped or not, maybe that doesn't work properly. There was also a problem where a window would dock but only be allocated 1 pixel, I think that was a bug in GNOME that is now fixed. Hopefully we can nail this all finally.
thanks -mike
On Wed, 2006-08-09 at 12:16 +0100, Mike Hearn wrote:
Ah awesome, good work! I know when I looked at this originally it seemed you could set a flag that told the embedder whether it was already mapped or not, maybe that doesn't work properly.
It's not that it doesn't work properly. What happened is that the x11drv maps any visible window, so that and the mapping flag were conflicting with one another. In order for this to work, the systray window (our icon) can *never* be mapped by WINE at any point during its life, or the tray will have problems with it, the extent of which depends on how said tray implements XEmbed (which explains disparities between the behavior of GNOME, XFce, and IceWM with the patches.) IOW, the tray has to handle everything with mapping/unmapping, or you get races.
On that subject, I've found that with some more tests, we're not quite out of the woods yet. I think the problem now is that WINE wants to map the window every time the tray becomes visible. I've thought about handling this in X11DRV_set_window_pos, but I'm not sure of the correct way to handle it. What we need to do is get the extended style of the window, but this seems to involve a bunch of wineserver calls that I'm not familiar with. Can you help me out on that? Thanks!
There was also a problem where a window would dock but only be allocated 1 pixel, I think that was a bug in GNOME that is now fixed.
I'm not quite sure about that one--I've got an X test app that docks, but I can only get 1 pixel wide. :( On wine, this isn't a problem though. Still trying to figure out what I need to do.
James
On 8/9/06, James Liggett jrliggett@cox.net wrote:
way to handle it. What we need to do is get the extended style of the window, but this seems to involve a bunch of wineserver calls that I'm not familiar with. Can you help me out on that? Thanks!
You can always stick some needed field in the X11DRV private data (you can see how to access that in the code itself).
I'm not quite sure about that one--I've got an X test app that docks, but I can only get 1 pixel wide. :( On wine, this isn't a problem though. Still trying to figure out what I need to do.
Oh dear :( Maybe you should ping one of the GNOME guys?
thanks -mike
On Thu, 2006-08-10 at 00:16 +0100, Mike Hearn wrote:
You can always stick some needed field in the X11DRV private data (you can see how to access that in the code itself).
Are you referring to x11drv_win_data? I looked at that, but it seems cleaner if I just use GetWindowLongW. I don't know if that's considered correct in this case (Sorry if it's a really dumb question, but I'm not sure on that.)
Oh dear :( Maybe you should ping one of the GNOME guys?
Yes, oh dear indeed. Maybe I will talk to the GNOME people about that.
James
On Thu, 2006-08-10 at 00:16 +0100, Mike Hearn wrote:
You can always stick some needed field in the X11DRV private data (you can see how to access that in the code itself).
OK--I apologize for being so dumb. it's the server reply that needs the extended styles field, not X11DRV_win_data. Sorry to waste your time.
I think I've finally got this one figured out. The results from two days ago turned out to be false positives because:
1. I wasn't checking for the tray style right (Didn't get the extended styles when I needed to; I was trying to check for WS_EX_TRAYWINDOW in the regular styles flag--argh! I've fixed that now.)
2. I noticed that the patches (without the right mapping fixes) work properly (for some strange reason) after I killed metacity about 2 or 3 times to get rid of those pesky systray adaptor windows that don't go away.
Now that I've fixed the mapping and properly check extended styles for the tray icon flag, the icons dock perfectly. I ran my test app 100 times in a row, and it worked every time! :) I haven't had to kill metacity once (Honest!)
I'll get the patches together and send an update.
Thanks for all your help, James
On Wed, 2006-08-09 at 13:30 -0700, James Liggett wrote:
also a problem where a window would dock but only be allocated 1 pixel, I think that was a bug in GNOME that is now fixed.
I'm not quite sure about that one--I've got an X test app that docks, but I can only get 1 pixel wide. :( On wine, this isn't a problem though. Still trying to figure out what I need to do.
Just FYI--I figured out what was wrong with my X test app. I just needed to set some size hints on the window, and now the size is perfect when the window docks. Now I've got my test app, a GNOME program, and some wine apps all peacefully coexisting on my tray, and they all dock perfectly. Guess there wasn't a bug in GNOME after all... ;)
On 8/15/06, James Liggett jrliggett@cox.net wrote:
Just FYI--I figured out what was wrong with my X test app. I just needed to set some size hints on the window, and now the size is perfect when the window docks. Now I've got my test app, a GNOME program, and some wine apps all peacefully coexisting on my tray, and they all dock perfectly. Guess there wasn't a bug in GNOME after all... ;)
Great work! Now let's hope Alexandre accepts it.
On Tue, 2006-08-15 at 12:14 +0100, Mike Hearn wrote:
Great work! Now let's hope Alexandre accepts it.
We're working on it. I talked about it briefly with Alexandre this morning on IRC. ATM he thinks I should get rid of the low-level checks for the systray windows. He suggested that I do something like have the window created off-screen so it isn't mapped. He also didn't like the idea of adding more dependencies on the WS_EX_TRAYWINDOW style, so right now I'm trying to figure out a way to get rid of it entirely, since now is as good a time as any to finally ditch that particular hack. ATM I'm considering using the systray adaptor class name as an identifier and calling the dock function in X11DRV_CreateWindow. I'll put that together, and if it works, I'll send another RFC to the list.
James
-
James Liggett -
Mike Hearn