Two tab dragging fixes for various window managers:...

Two tab dragging fixes for various window managers:
* Reverse the order of enumeration when enumerating all top-level windows.  XQueryTree returns the list of windows in bottom-to-top order, while DockInfo expects the windows in top-to-bottom order (ratpoison WM, Awesome WM).
* Extend the enumeration one level deeper for window managers that parent top-level windows into an unnamed real top-level window (ion WM).

BUG=14004
TEST=Extensive tab dragging in the following WMs: Metacity, Compiz, kwm, ion3, xmonad, ratpoison, awesome.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=19303

[Evan Martin, 2009-06-25 21:54:11]

LGTM

http://codereview.chromium.org/149028/diff/1/3
File chrome/common/gtk_util.cc (right):

http://codereview.chromium.org/149028/diff/1/3#newcode238
Line 238: // windows in unnamed actual top-level windows (ion WM), so extend the
I believe this is actually kinda common -- the app provides its top level window
and then the WM wraps a window around it to attach buttons and such.

http://codereview.chromium.org/149028/diff/1/2
File chrome/common/x11_util.cc (right):

http://codereview.chromium.org/149028/diff/1/2#newcode166
Line 166: bool WindowHasName(XID window, int depth) {
Can you doc this function?  At first glance I thought it was testing whether a
window had a specific name.

http://codereview.chromium.org/149028/diff/1/2#newcode171
Line 171: XFree(prop.value);
Does prop.value leak if (prop.value && !prop.nitems) ?

http://codereview.chromium.org/149028/diff/1/2#newcode180
Line 180: if (depth > max_depth)
You could put this before you declare the locals.  *shrug*

[James Hawkins, 2009-06-25 22:07:58]

http://codereview.chromium.org/149028/diff/1/3
File chrome/common/gtk_util.cc (right):

http://codereview.chromium.org/149028/diff/1/3#newcode238
Line 238: // windows in unnamed actual top-level windows (ion WM), so extend the
On 2009/06/25 21:54:11, Evan Martin wrote:
> I believe this is actually kinda common -- the app provides its top level
window
> and then the WM wraps a window around it to attach buttons and such.

ion is the only WM (that I know of) that enumerates this wrapper window instead
of the application window, though.

http://codereview.chromium.org/149028/diff/1/2
File chrome/common/x11_util.cc (right):

http://codereview.chromium.org/149028/diff/1/2#newcode166
Line 166: bool WindowHasName(XID window, int depth) {
On 2009/06/25 21:54:11, Evan Martin wrote:
> Can you doc this function?  At first glance I thought it was testing whether a
> window had a specific name.

I'll rename it to IsWindowNamed.  Is that clearer?

http://codereview.chromium.org/149028/diff/1/2#newcode171
Line 171: XFree(prop.value);
On 2009/06/25 21:54:11, Evan Martin wrote:
> Does prop.value leak if (prop.value && !prop.nitems) ?

I don't think that condition is valid, so I guess I can remove the check for
!prop.nitems.

http://codereview.chromium.org/149028/diff/1/2#newcode180
Line 180: if (depth > max_depth)
On 2009/06/25 21:54:11, Evan Martin wrote:
> You could put this before you declare the locals.  *shrug*

Done.

[Evan Martin, 2009-06-25 22:22:22]

LGTM