wayland: define base:NativeEvent for Wayland

wayland: define base:NativeEvent for Wayland

Fixes the use_wayland build which was broken by this commit:

http://codereview.chromium.org/8113028

Similar to win, we create a wayland namespace inside base
and define WaylandEvent there.

Historical note:

Wayland does not have a "native" event structure. Instead, the client
is made aware of events via a callback executed in the context
of display_run():

http://git.chromium.org/gitweb/?p=chromiumos/third_party/wayland-demos.git;a=blob;f=clients/window.c

So we create WaylandEvent structure which takes the parameters from
the callback and wraps them into a structure. For details, see:
src/ui/wayland/events/wayland_event.h

BUG=chromium:102903
TEST=Compiles but doesn't yet link with this change plus a series of
others I'm working on.

R=msw@chromium.org,oshima@chromium.org,mark@chromium.org


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

[oshima, 2011-11-04 01:18:31]

http://codereview.chromium.org/8378005/diff/1/base/event_types.h
File base/event_types.h (right):

http://codereview.chromium.org/8378005/diff/1/base/event_types.h#newcode17
base/event_types.h:17: #elif defined(USE_X11)
base cannot depend on ui, or has notion of ui. (even if you somehow convince me,
you probably won't get LGTM from OWNERS, although I'll be happy to be proven
wrong)
I think your idea about using third party may have more chance.

[Elliot Glaysher, 2011-11-04 01:20:23]

http://codereview.chromium.org/8378005/diff/1/base/event_types.h
File base/event_types.h (right):

http://codereview.chromium.org/8378005/diff/1/base/event_types.h#newcode17
base/event_types.h:17: #elif defined(USE_X11)
On 2011/11/04 01:18:31, oshima wrote:
> base cannot depend on ui, or has notion of ui. (even if you somehow convince
me,
> you probably won't get LGTM from OWNERS, although I'll be happy to be proven
> wrong)
> I think your idea about using third party may have more chance.

+1. base/ is included by everything.

[Mandeep Singh Baines, 2011-11-10 00:32:31]

Fixed. Create a wayland namespace inside base. Similar to base::win there is now
a base::wayland. Defined WaylandEvent there instead.

PTAL

[sky, 2011-11-10 00:52:42]

I only looked at ui and views.

http://codereview.chromium.org/8378005/diff/5001/views/widget/native_widget_w...
File views/widget/native_widget_wayland.h (right):

http://codereview.chromium.org/8378005/diff/5001/views/widget/native_widget_w...
views/widget/native_widget_wayland.h:25: using base::wayland::WaylandEvent;
no using in headers.

[dnicoara, 2011-11-10 01:13:32]

lgtm

Wayland changes look good.

[Mark Mentovai, 2011-11-10 04:40:07]

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc
File base/message_pump_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc...
base/message_pump_wayland.cc:6: #include "base/wayland/wayland_event.h"
This needs to follow system #includes, like <glib.h>.

[oshima, 2011-11-10 16:13:37]

I believe we want to support Wayland on aura, in which case, you can simply
remove accelerator_handler_wayland.cc and native_widget_wayland.{cc|h},
and add ui/aura/desktop_host_wayland (separate CL is fine)

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc
File base/message_pump_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc...
base/message_pump_wayland.cc:6: #include "base/wayland/wayland_event.h"
Do you need to include this? Looks like the event structure isn't used in this
file.

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h
File base/message_pump_wayland.h (right):

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h#...
base/message_pump_wayland.h:11: #include "base/wayland/wayland_event.h"
forward decl seems to be enough.

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h#...
base/message_pump_wayland.h:20: class BASE_EXPORT MessagePumpObserver {
Don't have to be in this CL, but please change to use one in
message_pump_observer. You should be able to eliminate a lot of ifdefs around
observer.

http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
File ui/base/wayland/events_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
ui/base/wayland/events_wayland.cc:11: #include "ui/base/events.h"
move this to top

[saintlou, 2011-11-10 18:12:44]

Yes supporting Wayland on Aura is in my mind more important than
Wayland on non-Aura :-)

-- Emmanuel

On Thu, Nov 10, 2011 at 8:13 AM,  <oshima@chromium.org> wrote:
> I believe we want to support Wayland on aura, in which case, you can simply
> remove accelerator_handler_wayland.cc and native_widget_wayland.{cc|h},
> and add ui/aura/desktop_host_wayland (separate CL is fine)
>
>
> http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc
> File base/message_pump_wayland.cc (right):
>
>
http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc...
> base/message_pump_wayland.cc:6: #include "base/wayland/wayland_event.h"
> Do you need to include this? Looks like the event structure isn't used
> in this file.
>
> http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h
> File base/message_pump_wayland.h (right):
>
>
http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h#...
> base/message_pump_wayland.h:11: #include "base/wayland/wayland_event.h"
> forward decl seems to be enough.
>
>
http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h#...
> base/message_pump_wayland.h:20: class BASE_EXPORT MessagePumpObserver {
> Don't have to be in this CL, but please change to use one in
> message_pump_observer. You should be able to eliminate a lot of ifdefs
> around observer.
>
>
http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
> File ui/base/wayland/events_wayland.cc (right):
>
>
http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
> ui/base/wayland/events_wayland.cc:11: #include "ui/base/events.h"
> move this to top
>
> http://codereview.chromium.org/8378005/
>

[Elliot Glaysher, 2011-11-11 18:23:54]

lgtm from a linuxgtk POV. Most of this code looks like it's designed to run with
only a glib runloop and won't be used in the eventual GTK+/wayland port.

[Mandeep Singh Baines, 2011-11-11 19:26:51]

Fixed. PTAL.

Yes, eventual goal of this work is to target aura.

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc
File base/message_pump_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc...
base/message_pump_wayland.cc:6: #include "base/wayland/wayland_event.h"
On 2011/11/10 04:40:07, Mark Mentovai wrote:
> This needs to follow system #includes, like <glib.h>.

Done.

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.cc...
base/message_pump_wayland.cc:6: #include "base/wayland/wayland_event.h"
On 2011/11/10 16:13:38, oshima wrote:
> Do you need to include this? Looks like the event structure isn't used in this
> file.

Done.

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h
File base/message_pump_wayland.h (right):

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h#...
base/message_pump_wayland.h:11: #include "base/wayland/wayland_event.h"
On 2011/11/10 16:13:38, oshima wrote:
> forward decl seems to be enough.

Done.

http://codereview.chromium.org/8378005/diff/5001/base/message_pump_wayland.h#...
base/message_pump_wayland.h:20: class BASE_EXPORT MessagePumpObserver {
On 2011/11/10 16:13:38, oshima wrote:
> Don't have to be in this CL, but please change to use one in
> message_pump_observer. You should be able to eliminate a lot of ifdefs around
> observer.

Plan to do that in a following CL.

http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
File ui/base/wayland/events_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
ui/base/wayland/events_wayland.cc:11: #include "ui/base/events.h"
On 2011/11/10 16:13:38, oshima wrote:
> move this to top

Done.

So why does it need to be at the top?

http://codereview.chromium.org/8378005/diff/5001/views/widget/native_widget_w...
File views/widget/native_widget_wayland.h (right):

http://codereview.chromium.org/8378005/diff/5001/views/widget/native_widget_w...
views/widget/native_widget_wayland.h:25: using base::wayland::WaylandEvent;
On 2011/11/10 00:52:42, sky wrote:
> no using in headers.

Done.

[sky, 2011-11-11 20:04:44]

ui/views LGTM

[oshima, 2011-11-11 20:36:53]

>I believe we want to support Wayland on aura, in which case, >you can simply
remove accelerator_handler_wayland.cc and >native_widget_wayland.{cc|h},

You want to remove files in separate CL?

http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
File ui/base/wayland/events_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/5001/ui/base/wayland/events_wayla...
ui/base/wayland/events_wayland.cc:11: #include "ui/base/events.h"
On 2011/11/11 19:26:52, Mandeep Singh Baines wrote:
> On 2011/11/10 16:13:38, oshima wrote:
> > move this to top
> 
> Done.
> 
> So why does it need to be at the top?

Because the file defines the API this file implements.
See ui/base/x/events_x.cc and ui/base/win/events_win.h
(events_win.h includes windowsx.h due to some dependency issue )

http://codereview.chromium.org/8378005/diff/15001/base/message_pump_wayland.cc
File base/message_pump_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/15001/base/message_pump_wayland.c...
base/message_pump_wayland.cc:7: #include "base/message_pump_wayland.h"
this one has to stay on top. The order is

1 header for cc file
2 system
3 other common headers
4 platform specific that requires ifdef

http://codereview.chromium.org/8378005/diff/15001/ui/base/wayland/events_wayl...
File ui/base/wayland/events_wayland.cc (right):

http://codereview.chromium.org/8378005/diff/15001/ui/base/wayland/events_wayl...
ui/base/wayland/events_wayland.cc:17: 
remove extra line

[Mandeep Singh Baines, 2011-11-11 23:40:30]

Fixed. PTAL.

I want to get the aura port functional before I tear down the earlier Wayland
port (allows me to test).

[oshima, 2011-11-12 00:21:02]

LGTM

[Mandeep Singh Baines, 2011-11-14 17:25:09]

ping

Still need an LGTM for base.

[Mark Mentovai, 2011-11-14 17:29:29]

LGTM in base for OWNERS approval. I only reviewed the files in base.

[commit-bot: I haz the power, 2011-11-14 17:55:38]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/msb@chromium.org/8378005/20003

[commit-bot: I haz the power, 2011-11-14 19:23:48]

Change committed as 109932