aura: Make sure the root-window retains the X events it was already listening for.

aura: Make sure the root-window retains the X events it was already listening for.

When aura starts listening for resize events on the X root-window, it
needs to make sure it continues to listen to other X events on the
root-window selected earlier.

Also, split out the handling of events targetted for the X root-window
into a separate function.

BUG=148784
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=156741

[sadrul, 2012-09-13 19:03:21]

derat@ Please review.

erg@ FYI (If you get a chance and review/test, that'd be great too)

[Daniel Erat, 2012-09-13 19:14:09]

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

http://codereview.chromium.org/10913253/diff/1/ui/aura/root_window_host_linux...
ui/aura/root_window_host_linux.cc:651: return false;
i think that we previously returned true in this case.  is the switch to false
here intentional?

[Elliot Glaysher, 2012-09-13 19:25:46]

On 2012/09/13 19:03:21, sadrul wrote:
> erg@ FYI (If you get a chance and review/test, that'd be great too)

Applied locally in a linux_aura build. chrome doesn't even start up:

[398:398:0913/122504:FATAL:root_window_host_linux.cc(645)] Check failed:
x_root_window_ == event->xconfigure.window (637 vs. 16778066)
Backtrace:
	base::debug::StackTrace::StackTrace() [0x126f27a]
	logging::LogMessage::~LogMessage() [0x1298929]
	aura::RootWindowHostLinux::DispatchEventForRootWindow() [0x13365c2]
	aura::RootWindowHostLinux::Dispatch() [0x13355d6]
	base::MessagePumpAuraX11::Dispatch() [0x131d720]
	base::MessagePumpAuraX11::ProcessXEvent() [0x131d3bf]
	base::MessagePumpAuraX11::DispatchXEvents() [0x131cd8f]
	(anonymous namespace)::XSourceDispatch() [0x131c48a]
	0x7faf141a5d53
	0x7faf141a60a0
	0x7faf141a6164
	base::MessagePumpGlib::RunWithDispatcher() [0x131a9d0]
	base::MessagePumpGlib::Run() [0x131adfe]
	MessageLoop::RunInternal() [0x129c4bf]
	MessageLoop::RunHandler() [0x129c376]
	base::RunLoop::Run() [0x12c2e30]
	ChromeBrowserMainParts::MainMessageLoopRun() [0xa82ef5]
	content::BrowserMainLoop::RunMainMessageLoopParts() [0x199e373]
	(anonymous namespace)::BrowserMainRunnerImpl::Run() [0x1b6a8ec]
	BrowserMain() [0x441a5bc]
	content::RunNamedProcessTypeMain() [0x2eef4ff]
	content::ContentMainRunnerImpl::Run() [0x2ef01e8]
	content::ContentMain() [0x2eeeb6b]
	ChromeMain [0x425909]
	main [0x4258d4]
	0x7faf0f67576d
	0x4257f9

[sadrul, 2012-09-13 23:41:24]

When testing on chromeos, I realized the GenericEvents on the root-window also
needed to be processed. So I have moved the GenericEvent handling out of
'Dispatch()' into 'DispatchGenericEvent()' and call it from both 'Dispatch()'
and 'DispatchEventForRootWindow()'.

(I also came across an issue with multi-mon support on chromeos. I have put up a
fix for that on http://codereview.chromium.org/10933080/)

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

http://codereview.chromium.org/10913253/diff/1/ui/aura/root_window_host_linux...
ui/aura/root_window_host_linux.cc:651: return false;
On 2012/09/13 19:14:09, Daniel Erat wrote:
> i think that we previously returned true in this case.  is the switch to false
> here intentional?

It wasn't. Incidentally, however, the return value from the root-window
message-dispatcher is ignored. So it doesn't make a difference. For consistency,
I have switched it back to returning true (and changed the only other such
root-window dispatcher that currently returns false to return true).

[sadrul, 2012-09-13 23:52:26]

On 2012/09/13 19:25:46, Elliot Glaysher wrote:
> On 2012/09/13 19:03:21, sadrul wrote:
> > erg@ FYI (If you get a chance and review/test, that'd be great too)
> 
> Applied locally in a linux_aura build. chrome doesn't even start up:
> 
> [398:398:0913/122504:FATAL:root_window_host_linux.cc(645)] Check failed:

That's weird. I can't repro this on my workstation. What WM are you using? What
revision are you on? Do you have multiple monitors, or a lot of other windows
open when this happens?

[Daniel Erat, 2012-09-14 00:14:24]

lgtm

http://codereview.chromium.org/10913253/diff/6002/ui/aura/root_window_host_li...
File ui/aura/root_window_host_linux.cc (right):

http://codereview.chromium.org/10913253/diff/6002/ui/aura/root_window_host_li...
ui/aura/root_window_host_linux.cc:605: void
RootWindowHostLinux::DispatchGenericEvent(const base::NativeEvent& event) {
nit: i sorta feel like DispatchXI2Event would be a better name for this...

[sadrul, 2012-09-14 02:09:20]

+sky for OWNERS

http://codereview.chromium.org/10913253/diff/6002/ui/aura/root_window_host_li...
File ui/aura/root_window_host_linux.cc (right):

http://codereview.chromium.org/10913253/diff/6002/ui/aura/root_window_host_li...
ui/aura/root_window_host_linux.cc:605: void
RootWindowHostLinux::DispatchGenericEvent(const base::NativeEvent& event) {
On 2012/09/14 00:14:24, Daniel Erat wrote:
> nit: i sorta feel like DispatchXI2Event would be a better name for this...

Agreed. Done.

[sky, 2012-09-14 02:18:56]

LGTM