Aura on Linux: Make a DesktopScreenX11 to match the windows one.

Aura on Linux: Make a DesktopScreenX11 to match the windows one.

BUG=125106
TEST=non-ash linux builds compile again.


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

[Elliot Glaysher, 2012-05-01 22:28:39]

derat: did I get the X11 parts here right?
ben: owner

[Ben Goodger (Google), 2012-05-01 22:38:40]

I'll leave it to derat to comment on the particulars of your X11 usage... but
your approach LGTM.

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
File ui/aura/desktop/desktop_screen_x11.cc (right):

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:34: public:
nit: lead with 1 space

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:48: private:
ditto

[Daniel Erat, 2012-05-01 22:51:48]

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
File ui/aura/desktop/desktop_screen_x11.cc (right):

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:9: // It clashes with out RootWindow.
nit: s/out/our/

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:23: // location data out of Xinerama.
s/Xinerama/XRandR, i think

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:64: gfx::Point
DesktopScreenX11::GetCursorScreenPoint() {
i'm a bit worried that this method gets called with reckless abandon (in the
aura implementation, the pointer position is cached from motion events).  not
much that can be done about that, i guess. :-/

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:67: // Unsure if I can leave these as
NULL.
pretty sure you can't

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:88: // Iterate across the return values of
XQuerryTree() here, which are sorted
nit: s/XQuerryTree/XQueryTree/

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:89: // in z-order.
nit: mention in the comment that they're in bottom-to-top order (at least,
that's not clear to me from "z-order")

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:95: if (XQueryTree(display,
DefaultRootWindow(display), &root, &parent,
note that if you intend to use this to find windows that are managed by a
conventional X window manager, most WMs reparent windows that they're managing
into frame windows, so you won't see chrome's windows as direct children of the
root.  _NET_CLIENT_LIST_STACKING is more appropriate in that case (assuming that
you're using a WM that implements it correctly):
http://standards.freedesktop.org/wm-spec/wm-spec-latest.html#id2533840

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:96: &children, &num_children)) {
nit: return early on failure instead of indenting all of this?

http://codereview.chromium.org/10279005/diff/1/ui/aura/desktop/desktop_screen...
ui/aura/desktop/desktop_screen_x11.cc:99:
aura::RootWindowHost::GetForAcceleratedWidget(children[i]);
actually, is this backwards?  you want the topmost window, right?

http://codereview.chromium.org/10279005/diff/1/ui/aura/root_window_host_linux.cc
File ui/aura/root_window_host_linux.cc (right):

http://codereview.chromium.org/10279005/diff/1/ui/aura/root_window_host_linux...
ui/aura/root_window_host_linux.cc:40: const char* kRootWindowHostLinuxKey =
"__AURA_ROOT_WINDOW_HOST_LINUX__";
nit: i think that "const char kRootWindowHostLinuxKey[]" is slightly preferred,
as it prevents reassignment

[Elliot Glaysher, 2012-05-02 19:41:37]

derat, ptal.

I removed the implementation of GetWindowAtCursorAtScreenPoint(); it's optional
and can safely return null. This patch fixes a compile error on trunk and I'd
like to get it fixed asap so I'm pushing an implementation of this mehtod to the
future.

[Daniel Erat, 2012-05-02 21:32:31]

lgtm

http://codereview.chromium.org/10279005/diff/11001/ui/aura/desktop/desktop_sc...
File ui/aura/desktop/desktop_screen_x11.cc (right):

http://codereview.chromium.org/10279005/diff/11001/ui/aura/desktop/desktop_sc...
ui/aura/desktop/desktop_screen_x11.cc:83: gfx::NativeWindow
DesktopScreenX11::GetWindowAtCursorScreenPoint() {
nit: add a comment mentioning that this is optional

http://codereview.chromium.org/10279005/diff/11001/ui/aura/desktop/desktop_sc...
ui/aura/desktop/desktop_screen_x11.cc:89: // about the Xinerama implications
here.
nit: s/Xinerama/XRandR/

[commit-bot: I haz the power, 2012-05-02 21:43:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/erg@chromium.org/10279005/18001

[commit-bot: I haz the power, 2012-05-03 00:00:31]

Change committed as 135036