Wayland support for views. views_desktop on Wayland.

Wayland support for views. views_desktop on Wayland.

This CL depends on:
 * http://codereview.chromium.org/7457023
 * http://codereview.chromium.org/7467007
 * http://codereview.chromium.org/7473010

Wayland requires newer libraries than Ubuntu currently provides. I've created a
list of required dependencies: 
https://sites.google.com/a/google.com/chrome_on_wayland/home/wayland-build-dependencies


BUG=
TEST=Built Chrome to verify that Wayland dependencies and changes don't interfere with
the usual build.

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

[sadrul, 2011-08-15 14:50:23]

Very nice! Left some comments.

http://codereview.chromium.org/7464027/diff/4001/views/controls/menu/native_m...
File views/controls/menu/native_menu_wayland.h (right):

http://codereview.chromium.org/7464027/diff/4001/views/controls/menu/native_m...
views/controls/menu/native_menu_wayland.h:5: #ifndef
VIEWS_CONTROLS_MENU_NATIVE_MENU_WAYLAND_H_
Is there anything wayland-y in here? Or can you continue to use native_menu_x
instead? And perhaps rename that to native_menu_linux?

http://codereview.chromium.org/7464027/diff/4001/views/controls/native_contro...
File views/controls/native_control_wayland.cc (right):

http://codereview.chromium.org/7464027/diff/4001/views/controls/native_contro...
views/controls/native_control_wayland.cc:16: #if defined(TOUCH_UI)
Let's not use any TOUCH_UI defines unless we absolutely need to.

http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/nat...
File views/controls/scrollbar/native_scroll_bar_wayland.cc (right):

http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/nat...
views/controls/scrollbar/native_scroll_bar_wayland.cc:9: #include
"ui/wayland/wayland_widget.h"
How much wayland-only code is in here? It looks like not much (if at all?).
Perhaps this can just be 'native_scroll_bar_views' instead (and help the
sans-gtk transition)?

http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/nat...
views/controls/scrollbar/native_scroll_bar_wayland.cc:110: int content_size,
Indentation (here and other places).

http://codereview.chromium.org/7464027/diff/4001/views/desktop/desktop_main.cc
File views/desktop/desktop_main.cc (right):

http://codereview.chromium.org/7464027/diff/4001/views/desktop/desktop_main.c...
views/desktop/desktop_main.cc:33: #if !defined(USE_WAYLAND)
Is TOOLKIT_USES_GTK defined for wayland? If not, this can be just

#if defined(TOOLKIT_USES_GTK)

http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc
File views/mouse_watcher.cc (right):

http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc#newco...
views/mouse_watcher.cc:11: #include "ui/wayland/events/wayland_event.h"
conditional include

http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc#newco...
views/mouse_watcher.cc:62: WillProcessEvent(ui::WaylandEvent* event) {
Indent 4 space.

[oshima_google, 2011-08-15 16:32:37]

Menu2 will be removed soon (there are one or two usage remained i think),
so it'd be better to wait or help Roy to remove it completely rather than
porting menu2 to wayalnd.

- oshima

On Mon, Aug 15, 2011 at 7:50 AM, <sadrul@chromium.org> wrote:

> Very nice! Left some comments.
>
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
controls/menu/native_menu_**wayland.h<http://codereview.chromium.org/7464027/diff/4001/views/controls/menu/native_menu_wayland.h>
> File views/controls/menu/native_**menu_wayland.h (right):
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
controls/menu/native_menu_**wayland.h#newcode5<http://codereview.chromium.org/7464027/diff/4001/views/controls/menu/native_menu_wayland.h#newcode5>
> views/controls/menu/native_**menu_wayland.h:5: #ifndef
> VIEWS_CONTROLS_MENU_NATIVE_**MENU_WAYLAND_H_
> Is there anything wayland-y in here? Or can you continue to use
> native_menu_x instead? And perhaps rename that to native_menu_linux?
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
controls/native_control_**wayland.cc<http://codereview.chromium.org/7464027/diff/4001/views/controls/native_control_wayland.cc>
> File views/controls/native_control_**wayland.cc (right):
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
controls/native_control_**wayland.cc#newcode16<http://codereview.chromium.org/7464027/diff/4001/views/controls/native_control_wayland.cc#newcode16>
> views/controls/native_control_**wayland.cc:16: #if defined(TOUCH_UI)
> Let's not use any TOUCH_UI defines unless we absolutely need to.
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
controls/scrollbar/native_**scroll_bar_wayland.cc<http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/native_scroll_bar_wayland.cc>
> File views/controls/scrollbar/**native_scroll_bar_wayland.cc (right):
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
controls/scrollbar/native_**scroll_bar_wayland.cc#newcode9<http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/native_scroll_bar_wayland.cc#newcode9>
> views/controls/scrollbar/**native_scroll_bar_wayland.cc:**9: #include
> "ui/wayland/wayland_widget.h"
> How much wayland-only code is in here? It looks like not much (if at
> all?). Perhaps this can just be 'native_scroll_bar_views' instead (and
> help the sans-gtk transition)?
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
controls/scrollbar/native_**scroll_bar_wayland.cc#**newcode110<http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/native_scroll_bar_wayland.cc#newcode110>
> views/controls/scrollbar/**native_scroll_bar_wayland.cc:**110: int
> content_size,
> Indentation (here and other places).
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
desktop/desktop_main.cc<http://codereview.chromium.org/7464027/diff/4001/views/desktop/desktop_main.cc>
> File views/desktop/desktop_main.cc (right):
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
desktop/desktop_main.cc#**newcode33<http://codereview.chromium.org/7464027/diff/4001/views/desktop/desktop_main.cc#newcode33>
> views/desktop/desktop_main.cc:**33: #if !defined(USE_WAYLAND)
> Is TOOLKIT_USES_GTK defined for wayland? If not, this can be just
>
> #if defined(TOOLKIT_USES_GTK)
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
mouse_watcher.cc<http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc>
> File views/mouse_watcher.cc (right):
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
mouse_watcher.cc#newcode11<http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc#newcode11>
> views/mouse_watcher.cc:11: #include "ui/wayland/events/wayland_**event.h"
> conditional include
>
> http://codereview.chromium.**org/7464027/diff/4001/views/**
>
mouse_watcher.cc#newcode62<http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc#newcode62>
> views/mouse_watcher.cc:62: WillProcessEvent(ui::**WaylandEvent* event) {
> Indent 4 space.
>
>
http://codereview.chromium.**org/7464027/<http://codereview.chromium.org/7464...
>

[dnicoara, 2011-08-15 17:23:13]

With regards to Sadrul's comments, I've posted messages inlined bellow.

With regards to Oshima's comment, thanks, I'll contact Roy. Currently, I've just
followed Sadrul's advice since it was trivial and it doesn't require any changes
from a Wayland perspective.

On 2011/08/15 14:50:23, sadrul wrote:
> Very nice! Left some comments.
> 
>
http://codereview.chromium.org/7464027/diff/4001/views/controls/menu/native_m...
> File views/controls/menu/native_menu_wayland.h (right):
> 
>
http://codereview.chromium.org/7464027/diff/4001/views/controls/menu/native_m...
> views/controls/menu/native_menu_wayland.h:5: #ifndef
> VIEWS_CONTROLS_MENU_NATIVE_MENU_WAYLAND_H_
> Is there anything wayland-y in here? Or can you continue to use native_menu_x
> instead? And perhaps rename that to native_menu_linux?
> 

Right, I've merged native_menu_wayland and native_menu_x into native_menu_linux.

>
http://codereview.chromium.org/7464027/diff/4001/views/controls/native_contro...
> File views/controls/native_control_wayland.cc (right):
> 
>
http://codereview.chromium.org/7464027/diff/4001/views/controls/native_contro...
> views/controls/native_control_wayland.cc:16: #if defined(TOUCH_UI)
> Let's not use any TOUCH_UI defines unless we absolutely need to.
> 

I've left the defines since that code is only used in TOUCH_UI builds. I wanted
to keep the code as clean as possible in order to have it compile under
different build types.

>
http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/nat...
> File views/controls/scrollbar/native_scroll_bar_wayland.cc (right):
> 
>
http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/nat...
> views/controls/scrollbar/native_scroll_bar_wayland.cc:9: #include
> "ui/wayland/wayland_widget.h"
> How much wayland-only code is in here? It looks like not much (if at all?).
> Perhaps this can just be 'native_scroll_bar_views' instead (and help the
> sans-gtk transition)?
> 

Hmm, I see what you mean. This class can definitely be renamed as views. Though,
first, native_control_wayland.h needs to be converted to native_control_views.h.
I'll take a look at it and try converting that first. I'll update later on when
I've finished it.

>
http://codereview.chromium.org/7464027/diff/4001/views/controls/scrollbar/nat...
> views/controls/scrollbar/native_scroll_bar_wayland.cc:110: int content_size,
> Indentation (here and other places).
> 

Done.

> http://codereview.chromium.org/7464027/diff/4001/views/desktop/desktop_main.cc
> File views/desktop/desktop_main.cc (right):
> 
>
http://codereview.chromium.org/7464027/diff/4001/views/desktop/desktop_main.c...
> views/desktop/desktop_main.cc:33: #if !defined(USE_WAYLAND)
> Is TOOLKIT_USES_GTK defined for wayland? If not, this can be just
> 
> #if defined(TOOLKIT_USES_GTK)
> 

Yes, TOOLKIT_USES_GTK isn't defined for Wayland, but currently there are some
GTK dependencies that need to be pulled which are nontrivial to remove, so
currently TOOLKIT_USES_GTK still needs to be defined even when compiling with
USE_WAYLAND.

> http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc
> File views/mouse_watcher.cc (right):
> 
>
http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc#newco...
> views/mouse_watcher.cc:11: #include "ui/wayland/events/wayland_event.h"
> conditional include
> 

Done.

>
http://codereview.chromium.org/7464027/diff/4001/views/mouse_watcher.cc#newco...
> views/mouse_watcher.cc:62: WillProcessEvent(ui::WaylandEvent* event) {
> Indent 4 space.

Done.

[Ben Goodger (Google), 2011-08-16 17:58:45]

High level looks OK.

http://codereview.chromium.org/7464027/diff/9003/views/controls/scrollbar/nat...
File views/controls/scrollbar/native_scroll_bar_wayland.h (right):

http://codereview.chromium.org/7464027/diff/9003/views/controls/scrollbar/nat...
views/controls/scrollbar/native_scroll_bar_wayland.h:5: #ifndef
VIEWS_CONTROLS_SCROLLBAR_NATIVE_SCROLL_BAR_WAYLAND_H_
qq: won't Wayland just use the views-based scrollbars like the other ports will?

e.g. NativeScrollbarViews.

Eventually we'll get rid of NativeScrollbars altogether and just have ScrollBar,
a views-based class.

[dnicoara, 2011-08-16 18:57:26]

Right, but we don't have a views scrollbar at the moment. I'm working on getting
one done.

On 2011/08/16 17:58:45, Ben Goodger (Google) wrote:
> High level looks OK.
> 
>
http://codereview.chromium.org/7464027/diff/9003/views/controls/scrollbar/nat...
> File views/controls/scrollbar/native_scroll_bar_wayland.h (right):
> 
>
http://codereview.chromium.org/7464027/diff/9003/views/controls/scrollbar/nat...
> views/controls/scrollbar/native_scroll_bar_wayland.h:5: #ifndef
> VIEWS_CONTROLS_SCROLLBAR_NATIVE_SCROLL_BAR_WAYLAND_H_
> qq: won't Wayland just use the views-based scrollbars like the other ports
will?
> 
> e.g. NativeScrollbarViews.
> 
> Eventually we'll get rid of NativeScrollbars altogether and just have
ScrollBar,
> a views-based class.

[sadrul, 2011-08-22 18:44:11]

LGTM.

http://codereview.chromium.org/7464027/diff/9003/skia/ext/canvas_paint_wayland.h
File skia/ext/canvas_paint_wayland.h (right):

http://codereview.chromium.org/7464027/diff/9003/skia/ext/canvas_paint_waylan...
skia/ext/canvas_paint_wayland.h:21: explicit CanvasPaintT(cairo_surface_t*
cairo_window_surface,
Remove 'explicit'