Gtk3 ui: Add libgtk3ui as a separate build component

Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
TBR=brettw@chromium.org
NOPRESUBMIT=true

Committed: https://crrev.com/215681db8aa96912f9ce19f9e1fffe38681e1af9
Cr-Commit-Position: refs/heads/master@{#427927}

[Tom (Use chromium acct), 2016-10-25 21:31:34]

Patchset #2 (id:20001) has been deleted

[Tom (Use chromium acct), 2016-10-25 21:31:40]

Patchset #1 (id:1) has been deleted

[Tom (Use chromium acct), 2016-10-25 21:31:53]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-25 21:32:59]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Tom (Use chromium acct), 2016-10-25 21:33:42]

Description was changed from

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
==========

to

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org
==========

[Tom (Use chromium acct), 2016-10-25 21:34:20]

thomasanderson@google.com changed reviewers:
+ dpranke@chromium.org, erg@chromium.org

[Tom (Use chromium acct), 2016-10-25 21:34:20]

From git cl owners:

dpranke:
  build/config/linux/gtk2/BUILD.gn
  build/config/linux/gtk3/BUILD.gn
erg:
  chrome/browser/ui/libgtk2ui/BUILD.gn
  chrome/browser/ui/libgtk2ui/gtk2_ui.cc
  chrome/browser/ui/libgtk2ui/unity_service.cc

[Elliot Glaysher, 2016-10-25 21:48:57]

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24: #include
"chrome/browser/themes/theme_properties.h"  // nogncheck
What's the nogncheck here for?

[Tom (Use chromium acct), 2016-10-25 22:00:23]

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24: #include
"chrome/browser/themes/theme_properties.h"  // nogncheck
On 2016/10/25 21:48:57, Elliot Glaysher wrote:
> What's the nogncheck here for?

Without it, if I run gn gen --check, I get this error:

ERROR at //chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24:11: Include not allowed.
#include "chrome/browser/themes/theme_properties.h"
          ^---------------------------------------
It is not in any dependency of
  //chrome/browser/ui/libgtk2ui:libgtk2ui
The include file is in the target(s):
  //chrome/browser:browser
which should somehow be reachable.


Might be related to the removal of allow_circular_includes_from in
chrome/browser/BUILD.gn

[Tom (Use chromium acct), 2016-10-25 22:41:47]

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24: #include
"chrome/browser/themes/theme_properties.h"  // nogncheck
On 2016/10/25 22:00:23, Tom Anderson wrote:
> On 2016/10/25 21:48:57, Elliot Glaysher wrote:
> > What's the nogncheck here for?
> 
> Without it, if I run gn gen --check, I get this error:
> 
> ERROR at //chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24:11: Include not allowed.
> #include "chrome/browser/themes/theme_properties.h"
>           ^---------------------------------------
> It is not in any dependency of
>   //chrome/browser/ui/libgtk2ui:libgtk2ui
> The include file is in the target(s):
>   //chrome/browser:browser
> which should somehow be reachable.
> 
> 
> Might be related to the removal of allow_circular_includes_from in
> chrome/browser/BUILD.gn

dpranke@:
Me and erg@ discussed this offline and we're not confident that nogncheck here
is the right solution.  I was hitting other problems when preserving
'allow_circular_includes_from'.

There's nothing we actually need to link against from theme_properties.h, it's
just a bunch of enums.  Maybe we can split the constants out?

[commit-bot: I haz the power, 2016-10-25 22:49:00]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-25 22:49:01]

Dry run: This issue passed the CQ dry run.

[Dirk Pranke, 2016-10-25 23:56:10]

https://codereview.chromium.org/2449243002/diff/40001/build/config/linux/gtk3...
File build/config/linux/gtk3/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/40001/build/config/linux/gtk3...
build/config/linux/gtk3/BUILD.gn:28: "//chrome/browser/ui/libgtk2ui:libgtk3ui",
You're not really creating a target called libgtk3ui in a directory called
libgtk2ui, are you? :)

Can we rename the directory?

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1895: # world.
Nit: I'd rewrite this to "these are the only components that can interact with
gtk".

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1900: }
You eventually want to be building and depending on both, right? Because if you
don't want that, we should structure the targets a bit differently.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtk2ui/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:26: sources_common = [
Nit: `common_sources` reads better.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:67: configs_common = []
common_configs.

You could also use `shared_sources` if you wanted.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:86: deps_common = [
common_deps, etc.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:129: deps += [
These two lines can be collapsed to `deps = common_deps + [`

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24: #include
"chrome/browser/themes/theme_properties.h"  // nogncheck
On 2016/10/25 22:41:46, Tom Anderson wrote:
> On 2016/10/25 22:00:23, Tom Anderson wrote:
> > On 2016/10/25 21:48:57, Elliot Glaysher wrote:
> > > What's the nogncheck here for?
> > 
> > Without it, if I run gn gen --check, I get this error:
> > 
> > ERROR at //chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24:11: Include not
allowed.
> > #include "chrome/browser/themes/theme_properties.h"
> >           ^---------------------------------------
> > It is not in any dependency of
> >   //chrome/browser/ui/libgtk2ui:libgtk2ui
> > The include file is in the target(s):
> >   //chrome/browser:browser
> > which should somehow be reachable.
> > 
> > 
> > Might be related to the removal of allow_circular_includes_from in
> > chrome/browser/BUILD.gn
> 
> dpranke@:
> Me and erg@ discussed this offline and we're not confident that nogncheck here
> is the right solution.  I was hitting other problems when preserving
> 'allow_circular_includes_from'.
> 
> There's nothing we actually need to link against from theme_properties.h, it's
> just a bunch of enums.  Maybe we can split the constants out?

nogncheck is indeed the wrong solution :). 

It is subverting the intent and hiding a layering violation (i.e., you're not
actually fixing the circular
dependency). See the docs for `gn check` for more. The right thing to do is to
move theme_properties.h into a separate source_set() target that the libgtk2ui
targets depend on.

[Tom (Use chromium acct), 2016-10-26 18:17:30]

Description was changed from

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org
==========

to

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
NOPRESUBMIT=true
==========

[Tom (Use chromium acct), 2016-10-26 18:17:30]

thomasanderson@google.com changed reviewers:
+ sky@chromium.org

[Tom (Use chromium acct), 2016-10-26 18:20:02]

+sky for
chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux.cc
third_party/harfbuzz-ng/BUILD.gn

NOPRESUBMIT=true to fix:
Banned functions were used.
    chrome/browser/ui/libgtkui/select_file_dialog_impl.cc:89:
      New code should not use ScopedAllowIO. Post a task to the blocking
      pool or the FILE thread instead.
    chrome/browser/ui/libgtkui/select_file_dialog_impl_kde.cc:183:
      New code should not use ScopedAllowIO. Post a task to the blocking
      pool or the FILE thread instead.

https://codereview.chromium.org/2449243002/diff/40001/build/config/linux/gtk3...
File build/config/linux/gtk3/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/40001/build/config/linux/gtk3...
build/config/linux/gtk3/BUILD.gn:28: "//chrome/browser/ui/libgtk2ui:libgtk3ui",
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> You're not really creating a target called libgtk3ui in a directory called
> libgtk2ui, are you? :)
> 
> Can we rename the directory?

Ah, well I guess it had to be done eventually, yay sed :)

Note that there are still files within libgtkui with gtk2 in their name.  Maybe
this would be better for a separate refactoring CL.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/BUILD.gn
File chrome/browser/ui/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1895: # world.
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> Nit: I'd rewrite this to "these are the only components that can interact with
> gtk".

Done.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/BUILD...
chrome/browser/ui/BUILD.gn:1900: }
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> You eventually want to be building and depending on both, right? Because if
you
> don't want that, we should structure the targets a bit differently.

We'll want to build both during our gtk3 transition, but I think after that is
over, we would only need to build the gtk3 version.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtk2ui/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:26: sources_common = [
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> Nit: `common_sources` reads better.

Done.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:67: configs_common = []
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> common_configs.
> 
> You could also use `shared_sources` if you wanted.

Done.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:86: deps_common = [
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> common_deps, etc.

Done.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/BUILD.gn:129: deps += [
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> These two lines can be collapsed to `deps = common_deps + [`

Done.

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
File chrome/browser/ui/libgtk2ui/gtk2_ui.cc (right):

https://codereview.chromium.org/2449243002/diff/40001/chrome/browser/ui/libgt...
chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24: #include
"chrome/browser/themes/theme_properties.h"  // nogncheck
On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> On 2016/10/25 22:41:46, Tom Anderson wrote:
> > On 2016/10/25 22:00:23, Tom Anderson wrote:
> > > On 2016/10/25 21:48:57, Elliot Glaysher wrote:
> > > > What's the nogncheck here for?
> > > 
> > > Without it, if I run gn gen --check, I get this error:
> > > 
> > > ERROR at //chrome/browser/ui/libgtk2ui/gtk2_ui.cc:24:11: Include not
> allowed.
> > > #include "chrome/browser/themes/theme_properties.h"
> > >           ^---------------------------------------
> > > It is not in any dependency of
> > >   //chrome/browser/ui/libgtk2ui:libgtk2ui
> > > The include file is in the target(s):
> > >   //chrome/browser:browser
> > > which should somehow be reachable.
> > > 
> > > 
> > > Might be related to the removal of allow_circular_includes_from in
> > > chrome/browser/BUILD.gn
> > 
> > dpranke@:
> > Me and erg@ discussed this offline and we're not confident that nogncheck
here
> > is the right solution.  I was hitting other problems when preserving
> > 'allow_circular_includes_from'.
> > 
> > There's nothing we actually need to link against from theme_properties.h,
it's
> > just a bunch of enums.  Maybe we can split the constants out?
> 
> nogncheck is indeed the wrong solution :). 
> 
> It is subverting the intent and hiding a layering violation (i.e., you're not
> actually fixing the circular
> dependency). See the docs for `gn check` for more. The right thing to do is to
> move theme_properties.h into a separate source_set() target that the libgtk2ui
> targets depend on.

Done.

[Tom (Use chromium acct), 2016-10-26 18:20:13]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 18:20:50]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 18:27:08]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 18:27:10]

Dry run: Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  linux_chromium_asan_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clobber_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_compile_dbg_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Tom (Use chromium acct), 2016-10-26 19:04:29]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-26 19:05:28]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Dirk Pranke, 2016-10-26 19:07:24]

lgtm.

On 2016/10/26 18:20:02, Tom Anderson wrote:
> +sky for
> chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux.cc
> third_party/harfbuzz-ng/BUILD.gn
> 
> NOPRESUBMIT=true to fix:
> Banned functions were used.
>     chrome/browser/ui/libgtkui/select_file_dialog_impl.cc:89:
>       New code should not use ScopedAllowIO. Post a task to the blocking
>       pool or the FILE thread instead.
>     chrome/browser/ui/libgtkui/select_file_dialog_impl_kde.cc:183:
>       New code should not use ScopedAllowIO. Post a task to the blocking
>       pool or the FILE thread instead.
> 
>
https://codereview.chromium.org/2449243002/diff/40001/build/config/linux/gtk3...
> File build/config/linux/gtk3/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2449243002/diff/40001/build/config/linux/gtk3...
> build/config/linux/gtk3/BUILD.gn:28:
"//chrome/browser/ui/libgtk2ui:libgtk3ui",
> On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> > You're not really creating a target called libgtk3ui in a directory called
> > libgtk2ui, are you? :)
> > 
> > Can we rename the directory?
> 
> Ah, well I guess it had to be done eventually, yay sed :)
> 
> Note that there are still files within libgtkui with gtk2 in their name. 
Maybe
> this would be better for a separate refactoring CL.

Yeah, probably. Ideally you'd rename things first and then add the gtk3 support,
but
you can do things in the other order as well.

> On 2016/10/25 23:56:10, Dirk Pranke (slow) wrote:
> > You eventually want to be building and depending on both, right? Because if
> you
> > don't want that, we should structure the targets a bit differently.
> 
> We'll want to build both during our gtk3 transition, but I think after that is
> over, we would only need to build the gtk3 version.

That's fine, things can be cleaned up then.

[Dirk Pranke, 2016-10-26 19:09:43]

https://codereview.chromium.org/2449243002/diff/80001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/80001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:3969: source_set("theme_properties") {
Is libgtkui the only thing that depends on this? Is it possible to move the
header into that directory then, or is that a bad idea?

[sky, 2016-10-26 19:58:50]

chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux.cc LGTM 

I am not a good owner for the harbuzz change. Please use a more local owner for
that.

[commit-bot: I haz the power, 2016-10-26 20:27:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 20:27:43]

Dry run: This issue passed the CQ dry run.

[Elliot Glaysher, 2016-10-26 20:38:09]

directory rename (and minor cleanup in libgtkui) lgtm

[Tom (Use chromium acct), 2016-10-26 22:10:57]

On 2016/10/26 19:58:50, sky wrote:
> chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux.cc LGTM 
> 
> I am not a good owner for the harbuzz change. Please use a more local owner
for
> that.

I only changed a comment from libgtk2ui to libgtkui.  That should be fine,
right?

https://codereview.chromium.org/2449243002/diff/80001/chrome/browser/BUILD.gn
File chrome/browser/BUILD.gn (right):

https://codereview.chromium.org/2449243002/diff/80001/chrome/browser/BUILD.gn...
chrome/browser/BUILD.gn:3969: source_set("theme_properties") {
On 2016/10/26 19:09:43, Dirk Pranke wrote:
> Is libgtkui the only thing that depends on this? Is it possible to move the
> header into that directory then, or is that a bad idea?

I changed some things around related to theme_properties in
chrome/browser/BUILD.gn and chrome/browser/ui/libgtkui/BUILD.gn. dpranke@ PTAL

[Dirk Pranke, 2016-10-26 22:15:19]

lgtm. I'd R= or TBR= brettw@ for the trivial harfbuzz change.

[Tom (Use chromium acct), 2016-10-26 22:33:56]

Description was changed from

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
NOPRESUBMIT=true
==========

to

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
TBR=brettw@chromium.org
NOPRESUBMIT=true
==========

[Tom (Use chromium acct), 2016-10-26 22:34:11]

The CQ bit was checked by thomasanderson@google.com

[Tom (Use chromium acct), 2016-10-26 22:34:12]

The patchset sent to the CQ was uploaded after l-g-t-m from erg@chromium.org,
sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2449243002/#ps100001
(title: "gn format")

[commit-bot: I haz the power, 2016-10-26 22:35:09]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-26 22:40:49]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-26 22:40:51]

Try jobs failed on following builders:
  cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
  chromeos_amd64-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_amd64-...)
  chromeos_daisy_chromium_compile_only_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_daisy_...)
  chromeos_x86-generic_chromium_compile_only_ng on
master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromeos_x86-ge...)
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[sky, 2016-10-26 23:38:16]

On 2016/10/26 22:10:57, Tom Anderson wrote:
> On 2016/10/26 19:58:50, sky wrote:
> > chrome/browser/ui/views/chrome_browser_main_extra_parts_views_linux.cc LGTM 
> > 
> > I am not a good owner for the harbuzz change. Please use a more local owner
> for
> > that.
> 
> I only changed a comment from libgtk2ui to libgtkui.  That should be fine,
> right?

I honestly didn't even look, but given your comment here I looked. Yes, LGTM

> https://codereview.chromium.org/2449243002/diff/80001/chrome/browser/BUILD.gn
> File chrome/browser/BUILD.gn (right):
> 
>
https://codereview.chromium.org/2449243002/diff/80001/chrome/browser/BUILD.gn...
> chrome/browser/BUILD.gn:3969: source_set("theme_properties") {
> On 2016/10/26 19:09:43, Dirk Pranke wrote:
> > Is libgtkui the only thing that depends on this? Is it possible to move the
> > header into that directory then, or is that a bad idea?
> 
> I changed some things around related to theme_properties in
> chrome/browser/BUILD.gn and chrome/browser/ui/libgtkui/BUILD.gn. dpranke@ PTAL

[Tom (Use chromium acct), 2016-10-27 00:41:26]

The CQ bit was checked by thomasanderson@google.com to run a CQ dry run

[commit-bot: I haz the power, 2016-10-27 00:42:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 02:29:43]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-27 02:29:44]

Dry run: This issue passed the CQ dry run.

[Tom (Use chromium acct), 2016-10-27 03:28:23]

The CQ bit was checked by thomasanderson@google.com

[Tom (Use chromium acct), 2016-10-27 03:28:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, erg@chromium.org, sky@chromium.org
Link to the patchset: https://codereview.chromium.org/2449243002/#ps120001
(title: "Add theme_properties dep to //chrome/browser/ui")

[commit-bot: I haz the power, 2016-10-27 03:28:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-27 03:30:48]

Description was changed from

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
TBR=brettw@chromium.org
NOPRESUBMIT=true
==========

to

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
TBR=brettw@chromium.org
NOPRESUBMIT=true
==========

[commit-bot: I haz the power, 2016-10-27 03:30:50]

Committed patchset #5 (id:120001)

[commit-bot: I haz the power, 2016-10-27 03:35:24]

Description was changed from

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
TBR=brettw@chromium.org
NOPRESUBMIT=true
==========

to

==========
Gtk3 ui: Add libgtk3ui as a separate build component

This CL adds libgtk3ui.so to supplement libgtk2ui.so so that the
builders can build using gtk3 without having to link the chrome binary
against it (or link against any component that links against it).

The next step is to add the target libgtk3ui to gn_all.

BUG=132847
R=erg@chromium.org,dpranke@chromium.org, sky@chromium.org
TBR=brettw@chromium.org
NOPRESUBMIT=true

Committed: https://crrev.com/215681db8aa96912f9ce19f9e1fffe38681e1af9
Cr-Commit-Position: refs/heads/master@{#427927}
==========

[commit-bot: I haz the power, 2016-10-27 03:35:26]

Patchset 5 (id:??) landed as
https://crrev.com/215681db8aa96912f9ce19f9e1fffe38681e1af9
Cr-Commit-Position: refs/heads/master@{#427927}