Adding Wayland message loop and gyp build deps

Adding Wayland message loop and gyp build deps

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

BUG=
TEST=


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

[Evan Martin, 2011-07-21 16:44:21]

http://codereview.chromium.org/7473010/diff/1/base/message_pump_wayland.h
File base/message_pump_wayland.h (right):

http://codereview.chromium.org/7473010/diff/1/base/message_pump_wayland.h#new...
base/message_pump_wayland.h:14: typedef struct _GSource GSource;
ChromeOS seems to be trying to get rid of glib.  Is it necessary for a wayland
mainloop or did you just do it for expediency?

http://codereview.chromium.org/7473010/diff/1/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/7473010/diff/1/build/common.gypi#newcode53
build/common.gypi:53: 'use_wayland%': '<(use_wayland)',
I believe all of these nested use_foobars are not necessary, and were perhaps
cargo-culted into the current state.  Perhaps try deleting this change, L43-45,
and then just put the default value of 0 down at L97 along with the comment.

If that doesn't work, it wouldn't surprise me (for complicated reasons), but I
think it ought to.

http://codereview.chromium.org/7473010/diff/1/build/common.gypi#newcode221
build/common.gypi:221: # every dependency.
Can you elaborate on this comment?  I don't know what "too spread" means here.

[dnicoara, 2011-07-21 18:29:25]

On 2011/07/21 16:44:21, Evan Martin wrote:
> http://codereview.chromium.org/7473010/diff/1/base/message_pump_wayland.h
> File base/message_pump_wayland.h (right):
> 
>
http://codereview.chromium.org/7473010/diff/1/base/message_pump_wayland.h#new...
> base/message_pump_wayland.h:14: typedef struct _GSource GSource;
> ChromeOS seems to be trying to get rid of glib.  Is it necessary for a wayland
> mainloop or did you just do it for expediency?
> 

I used it because all the Linux message pumps use it. Also, the Wayland samples
used glib so it was easier to just use it. There is no reason Wayland wouldn't
work with a different type of main loop.

> http://codereview.chromium.org/7473010/diff/1/build/common.gypi
> File build/common.gypi (right):
> 
> http://codereview.chromium.org/7473010/diff/1/build/common.gypi#newcode53
> build/common.gypi:53: 'use_wayland%': '<(use_wayland)',
> I believe all of these nested use_foobars are not necessary, and were perhaps
> cargo-culted into the current state.  Perhaps try deleting this change,
L43-45,
> and then just put the default value of 0 down at L97 along with the comment.
> 
> If that doesn't work, it wouldn't surprise me (for complicated reasons), but I
> think it ought to.
> 

Updated common.gypi as per your suggestion.

> http://codereview.chromium.org/7473010/diff/1/build/common.gypi#newcode221
> build/common.gypi:221: # every dependency.
> Can you elaborate on this comment?  I don't know what "too spread" means here.

I meant that the use of toolkit_uses_gtk and use_x11 is throughout the code
which makes it hard to completely remove them right now. They are also used
interchangeably so removing just one of them is hard.

[Evan Martin, 2011-07-21 18:40:07]

http://codereview.chromium.org/7473010/diff/4001/base/base.gypi
File base/base.gypi (right):

http://codereview.chromium.org/7473010/diff/4001/base/base.gypi#newcode470
base/base.gypi:470: '../build/linux/system.gyp:dbus-glib',
What is this needed for?  I don't see glib in any of your code.

http://codereview.chromium.org/7473010/diff/4001/base/message_pump_wayland.cc
File base/message_pump_wayland.cc (right):

http://codereview.chromium.org/7473010/diff/4001/base/message_pump_wayland.cc...
base/message_pump_wayland.cc:9: #include <glib.h>
This should go above L7:

#include "header-matching this file"
\n
#include <systemheader1>
#include <systemheader2>
\n
#include "other headers1"
#include "other headers2"

http://codereview.chromium.org/7473010/diff/4001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/7473010/diff/4001/build/common.gypi#newcode216
build/common.gypi:216: # TODO Wayland build should have these disabled, but
currently
Can you write it "TODO(dnicoara): ..."

[dnicoara, 2011-07-21 19:06:29]

On 2011/07/21 18:40:07, Evan Martin wrote:
> http://codereview.chromium.org/7473010/diff/4001/base/base.gypi
> File base/base.gypi (right):
> 
> http://codereview.chromium.org/7473010/diff/4001/base/base.gypi#newcode470
> base/base.gypi:470: '../build/linux/system.gyp:dbus-glib',
> What is this needed for?  I don't see glib in any of your code.
> 

Right, removed. Dependency that pulls glib. I used it when I tried to compile
with toolkit_uses_gtk = 0. Right now that flag will be set to 1 when compiling
for Wayland so it isn't required.

> http://codereview.chromium.org/7473010/diff/4001/base/message_pump_wayland.cc
> File base/message_pump_wayland.cc (right):
> 
>
http://codereview.chromium.org/7473010/diff/4001/base/message_pump_wayland.cc...
> base/message_pump_wayland.cc:9: #include <glib.h>
> This should go above L7:
> 
> #include "header-matching this file"
> \n
> #include <systemheader1>
> #include <systemheader2>
> \n
> #include "other headers1"
> #include "other headers2"
> 

Right. I actually removed one include since it isn't required.

> http://codereview.chromium.org/7473010/diff/4001/build/common.gypi
> File build/common.gypi (right):
> 
> http://codereview.chromium.org/7473010/diff/4001/build/common.gypi#newcode216
> build/common.gypi:216: # TODO Wayland build should have these disabled, but
> currently
> Can you write it "TODO(dnicoara): ..."

Done.

[Evan Martin, 2011-07-21 19:23:19]

LGTM, be sure to update the review description before you land

http://codereview.chromium.org/7473010/diff/10001/build/common.gypi
File build/common.gypi (right):

http://codereview.chromium.org/7473010/diff/10001/build/common.gypi#newcode94
build/common.gypi:94: 'use_wayland%': 0,
I think you can remove this one as well, but you'd probably have to try it to
verify.

[commit-bot: I haz the power, 2011-08-10 13:11:28]

Try job failure for 7473010-13001 (retry) on win for step "compile" (clobber
build).
It's a second try, previously, step "compile" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win&number...

[commit-bot: I haz the power, 2011-08-11 18:20:23]

Change committed as 96428