aura: Have aura and aura_demo build and run without gtk on linux.

aura: Have aura and aura_demo build and run without gtk on linux.

BUG=97131
TEST=manually

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

[sadrul, 2011-09-21 19:39:06]

@derat: Please review the changes in ui/base/x and build/

@ben: Please review the changes in aura/ and build/

All: Please note that this CL turns off TOOLKIT_USES_GTK for use_aura. This
means some of the components, e.g. views, aura_shell, chrome will stop compiling
with use_aura for the moment. Is that acceptable? If not, I can revert the
relevant changes from this CL.

[Ben Goodger (Google), 2011-09-21 20:01:43]

I wonder how much work it is to get Chrome to build with USE_AURA and
TOOLKIT_USES_GTK. Do you have a sense?

-Ben

On Wed, Sep 21, 2011 at 12:39 PM, <sadrul@chromium.org> wrote:

> Reviewers: Ben Goodger (Google), Daniel Erat,
>
> Message:
> @derat: Please review the changes in ui/base/x and build/
>
> @ben: Please review the changes in aura/ and build/
>
> All: Please note that this CL turns off TOOLKIT_USES_GTK for use_aura. This
> means some of the components, e.g. views, aura_shell, chrome will stop
> compiling
> with use_aura for the moment. Is that acceptable? If not, I can revert the
> relevant changes from this CL.
>
> Description:
> aura: Have aura and aura_demo build and run without gtk on linux.
>
> BUG=97131
> TEST=manually
>
>
> Please review this at
http://codereview.chromium.**org/7981030/<http://codereview.chromium.org/7981...
>
> SVN Base:
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
>
> Affected files:
>  M build/build_config.h
>  M build/common.gypi
>  M ui/aura/demo/demo_main.cc
>  M ui/base/x/active_window_**watcher_x.h
>  M ui/base/x/active_window_**watcher_x.cc
>  M ui/base/x/x11_util.h
>  M ui/base/x/x11_util.cc
>
>
>

[DaveMoore, 2011-09-21 20:09:28]

I don't think we want that, do we? I'm working on fixing the USE_AURA /
TOOLKIT_USES_GTK=0 case.

On 2011/09/21 20:01:43, Ben Goodger (Google) wrote:
> I wonder how much work it is to get Chrome to build with USE_AURA and
> TOOLKIT_USES_GTK. Do you have a sense?
> 
> -Ben
> 
> On Wed, Sep 21, 2011 at 12:39 PM, <mailto:sadrul@chromium.org> wrote:
> 
> > Reviewers: Ben Goodger (Google), Daniel Erat,
> >
> > Message:
> > @derat: Please review the changes in ui/base/x and build/
> >
> > @ben: Please review the changes in aura/ and build/
> >
> > All: Please note that this CL turns off TOOLKIT_USES_GTK for use_aura. This
> > means some of the components, e.g. views, aura_shell, chrome will stop
> > compiling
> > with use_aura for the moment. Is that acceptable? If not, I can revert the
> > relevant changes from this CL.
> >
> > Description:
> > aura: Have aura and aura_demo build and run without gtk on linux.
> >
> > BUG=97131
> > TEST=manually
> >
> >
> > Please review this at
>
http://codereview.chromium.**org/7981030/%3Chttp://codereview.chromium.org/79...>
> >
> > SVN Base:
>
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
> >
> > Affected files:
> >  M build/build_config.h
> >  M build/common.gypi
> >  M ui/aura/demo/demo_main.cc
> >  M ui/base/x/active_window_**watcher_x.h
> >  M ui/base/x/active_window_**watcher_x.cc
> >  M ui/base/x/x11_util.h
> >  M ui/base/x/x11_util.cc
> >
> >
> >

[Ben Goodger (Google), 2011-09-21 20:12:06]

My suspicion was that it was a fair bit of work, and that it was easier to
tackle transgressions incrementally until we can throw this master switch.

It would be good to have a list/spreadsheet/buglist of such transgressions
so it can be burned down.

-Ben

On Wed, Sep 21, 2011 at 1:09 PM, <davemoore@chromium.org> wrote:

> I don't think we want that, do we? I'm working on fixing the USE_AURA /
> TOOLKIT_USES_GTK=0 case.
>
>
> On 2011/09/21 20:01:43, Ben Goodger (Google) wrote:
>
>> I wonder how much work it is to get Chrome to build with USE_AURA and
>> TOOLKIT_USES_GTK. Do you have a sense?
>>
>
>  -Ben
>>
>
>  On Wed, Sep 21, 2011 at 12:39 PM, <mailto:sadrul@chromium.org> wrote:
>>
>
>  > Reviewers: Ben Goodger (Google), Daniel Erat,
>> >
>> > Message:
>> > @derat: Please review the changes in ui/base/x and build/
>> >
>> > @ben: Please review the changes in aura/ and build/
>> >
>> > All: Please note that this CL turns off TOOLKIT_USES_GTK for use_aura.
>> This
>> > means some of the components, e.g. views, aura_shell, chrome will stop
>> > compiling
>> > with use_aura for the moment. Is that acceptable? If not, I can revert
>> the
>> > relevant changes from this CL.
>> >
>> > Description:
>> > aura: Have aura and aura_demo build and run without gtk on linux.
>> >
>> > BUG=97131
>> > TEST=manually
>> >
>> >
>> > Please review this at
>>
>
> http://codereview.chromium.****org/7981030/%3Chttp://coderevi**
> ew.chromium.org/7981030/ <http://codereview.chromium.org/7981030/>>
>
>> >
>> > SVN Base:
>>
>
>
svn://svn.chromium.org/chrome/****trunk/src<http://svn.chromium.org/chrome/**trunk/src>
>
<http://svn.**chromium.org/chrome/trunk/src<http://svn.chromium.org/chrome/tru...
> >
>
>  >
>> > Affected files:
>> >  M build/build_config.h
>> >  M build/common.gypi
>> >  M ui/aura/demo/demo_main.cc
>> >  M ui/base/x/active_window_****watcher_x.h
>> >  M ui/base/x/active_window_****watcher_x.cc
>> >  M ui/base/x/x11_util.h
>> >  M ui/base/x/x11_util.cc
>> >
>> >
>> >
>>
>
>
>
>
http://codereview.chromium.**org/7981030/<http://codereview.chromium.org/7981...
>

[sadrul, 2011-09-21 20:21:42]

On 2011/09/21 20:01:43, Ben Goodger (Google) wrote:
> I wonder how much work it is to get Chrome to build with USE_AURA and
> TOOLKIT_USES_GTK. Do you have a sense?

I believe saintlou@ has already made Chrome compile with both USE_AURA and
TOOLKIT_USES_GTK set. The effort here is to take the next step: i.e. remove GTK
and turn off TOOLKIT_USES_GTK when USE_AURA is set.

With this CL, aura and aura_demo is buildable without GTK. The next step is to
have views build without GTK, which should allow aura_shell to be also buildable
without GTK. The next step would be to have Chrome compile without GTK. I think
for views and aura_shell, the required effort is not a whole lot. For Chrome, I
honestly have no idea yet.

> 
> -Ben
> 
> On Wed, Sep 21, 2011 at 12:39 PM, <mailto:sadrul@chromium.org> wrote:
> 
> > Reviewers: Ben Goodger (Google), Daniel Erat,
> >
> > Message:
> > @derat: Please review the changes in ui/base/x and build/
> >
> > @ben: Please review the changes in aura/ and build/
> >
> > All: Please note that this CL turns off TOOLKIT_USES_GTK for use_aura. This
> > means some of the components, e.g. views, aura_shell, chrome will stop
> > compiling
> > with use_aura for the moment. Is that acceptable? If not, I can revert the
> > relevant changes from this CL.
> >
> > Description:
> > aura: Have aura and aura_demo build and run without gtk on linux.
> >
> > BUG=97131
> > TEST=manually
> >
> >
> > Please review this at
>
http://codereview.chromium.**org/7981030/%3Chttp://codereview.chromium.org/79...>
> >
> > SVN Base:
>
svn://svn.chromium.org/chrome/**trunk/src<http://svn.chromium.org/chrome/trunk/src>
> >
> > Affected files:
> >  M build/build_config.h
> >  M build/common.gypi
> >  M ui/aura/demo/demo_main.cc
> >  M ui/base/x/active_window_**watcher_x.h
> >  M ui/base/x/active_window_**watcher_x.cc
> >  M ui/base/x/x11_util.h
> >  M ui/base/x/x11_util.cc
> >
> >
> >

[Ben Goodger (Google), 2011-09-21 20:22:43]

Sorry, I meant USE_AURA and !TOOLKIT_USES_GTK.

-Ben

On Wed, Sep 21, 2011 at 1:21 PM, <sadrul@chromium.org> wrote:

> On 2011/09/21 20:01:43, Ben Goodger (Google) wrote:
>
>> I wonder how much work it is to get Chrome to build with USE_AURA and
>> TOOLKIT_USES_GTK. Do you have a sense?
>>
>
> I believe saintlou@ has already made Chrome compile with both USE_AURA and
> TOOLKIT_USES_GTK set. The effort here is to take the next step: i.e. remove
> GTK
> and turn off TOOLKIT_USES_GTK when USE_AURA is set.
>
> With this CL, aura and aura_demo is buildable without GTK. The next step is
> to
> have views build without GTK, which should allow aura_shell to be also
> buildable
> without GTK. The next step would be to have Chrome compile without GTK. I
> think
> for views and aura_shell, the required effort is not a whole lot. For
> Chrome, I
> honestly have no idea yet.
>
>
>
>  -Ben
>>
>
>  On Wed, Sep 21, 2011 at 12:39 PM, <mailto:sadrul@chromium.org> wrote:
>>
>
>  > Reviewers: Ben Goodger (Google), Daniel Erat,
>> >
>> > Message:
>> > @derat: Please review the changes in ui/base/x and build/
>> >
>> > @ben: Please review the changes in aura/ and build/
>> >
>> > All: Please note that this CL turns off TOOLKIT_USES_GTK for use_aura.
>> This
>> > means some of the components, e.g. views, aura_shell, chrome will stop
>> > compiling
>> > with use_aura for the moment. Is that acceptable? If not, I can revert
>> the
>> > relevant changes from this CL.
>> >
>> > Description:
>> > aura: Have aura and aura_demo build and run without gtk on linux.
>> >
>> > BUG=97131
>> > TEST=manually
>> >
>> >
>> > Please review this at
>>
>
> http://codereview.chromium.****org/7981030/%3Chttp://coderevi**
> ew.chromium.org/7981030/ <http://codereview.chromium.org/7981030/>>
>
>> >
>> > SVN Base:
>>
>
>
svn://svn.chromium.org/chrome/****trunk/src<http://svn.chromium.org/chrome/**trunk/src>
>
<http://svn.**chromium.org/chrome/trunk/src<http://svn.chromium.org/chrome/tru...
> >
>
>> >
>> > Affected files:
>> >  M build/build_config.h
>> >  M build/common.gypi
>> >  M ui/aura/demo/demo_main.cc
>> >  M ui/base/x/active_window_****watcher_x.h
>> >  M ui/base/x/active_window_****watcher_x.cc
>> >  M ui/base/x/x11_util.h
>> >  M ui/base/x/x11_util.cc
>> >
>> >
>> >
>>
>
>
>
>
http://codereview.chromium.**org/7981030/<http://codereview.chromium.org/7981...
>

[Daniel Erat, 2011-09-21 20:27:08]

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/active_window_watc...
File ui/base/x/active_window_watcher_x.cc (right):

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/active_window_watc...
ui/base/x/active_window_watcher_x.cc:11: #if defined(TOOLKIT_USES_GTK)
nit: move this above the x11_util.h include

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/active_window_watc...
ui/base/x/active_window_watcher_x.cc:43: return false;
true may be a better default for us; WMs that don't support _NET_ACTIVE_WINDOW
are very rare

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/active_window_watc...
ui/base/x/active_window_watcher_x.cc:104: FOR_EACH_OBSERVER(
ifdef-ing this out essentially makes this class do nothing in non-GTK builds,
right?

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/x11_util.cc#newcode32
ui/base/x/x11_util.cc:32: #define gdk_error_trap_push()
stubbing these means that we're not trapping X errors, no?  if we're already
installing our own custom Xlib error handler, can the calls to these functions
just be removed?

[Daniel Erat, 2011-09-21 20:31:47]

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/x11_util.cc#newcode32
ui/base/x/x11_util.cc:32: #define gdk_error_trap_push()
On 2011/09/21 20:27:08, Daniel Erat wrote:
> stubbing these means that we're not trapping X errors, no?  if we're already
> installing our own custom Xlib error handler, can the calls to these functions
> just be removed?

http://codereview.chromium.org/7889040/ may be related

[sadrul, 2011-09-21 21:56:18]

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/active_window_watc...
File ui/base/x/active_window_watcher_x.cc (right):

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/active_window_watc...
ui/base/x/active_window_watcher_x.cc:11: #if defined(TOOLKIT_USES_GTK)
On 2011/09/21 20:27:08, Daniel Erat wrote:
> nit: move this above the x11_util.h include

I just realized that this file shouldn't be necessary at all for aura. So I
removed the changes in this file, and excluded this from ui.gyp for use_aura

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/x11_util.cc
File ui/base/x/x11_util.cc (right):

http://codereview.chromium.org/7981030/diff/2001/ui/base/x/x11_util.cc#newcode32
ui/base/x/x11_util.cc:32: #define gdk_error_trap_push()
On 2011/09/21 20:31:47, Daniel Erat wrote:
> On 2011/09/21 20:27:08, Daniel Erat wrote:
> > stubbing these means that we're not trapping X errors, no?  if we're already
> > installing our own custom Xlib error handler, can the calls to these
functions
> > just be removed?
> 
> http://codereview.chromium.org/7889040/ may be related


Thanks for the link. I think that change can be useful here. I have left a
comment in that CL.

[Daniel Erat, 2011-09-22 17:07:05]

lgtm

http://codereview.chromium.org/7981030/diff/17/build/build_config.h
File build/build_config.h (right):

http://codereview.chromium.org/7981030/diff/17/build/build_config.h#newcode48
build/build_config.h:48: #if defined(TOOLKIT_GTK) || (defined(TOOLKIT_VIEWS) &&
!defined(OS_WIN) && !defined(USE_AURA))
nit: wrap long line