Allow generation of a chromium outdirs when enable_extensions=false

Allow generation of a chromium outdirs when enable_extensions=false

This allows us at Opera to patch chromium a little less before
building a downstream product that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other browser products that use
enable_extensions=false can now generate an outdir without
getting complaints from GN.

BUG=725906
TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest

Review-Url: https://codereview.chromium.org/2904443004
Cr-Commit-Position: refs/heads/master@{#479646}
Committed: https://chromium.googlesource.com/chromium/src/+/f7a620c87d8df98cbf01a6775c668768d0cd4840

[hugoh_UTC2, 2017-05-23 12:06:37]

hugoh@opera.com changed reviewers:
+ dpranke@chromium.org, michaelpg@chromium.org

[hugoh_UTC2, 2017-05-23 12:06:38]

michaelpg@ PTAL

dpranke@ since you already approved this idea (we upstreamed this in
https://codereview.chromium.org/1950003002/), I hope you will approve this
updated version, once michaelpg@ has had a look.

[hugoh_UTC2, 2017-05-23 12:11:57]

Description was changed from

==========
Allow generation of a chromium outdirs when enable_extensions=false

This is important to us because it allows us to patch the upstream
tree a little less before building our product.

We need an upstream buildbot that checks that these build configurations;
 $ gn gen --args='enable_extensions=false' out
 $ gn gen --args='target_os="android" enable_extensions=false' out
are always OK (even if they don't compile).

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of ours)
that needs enable_extensions=false can now generate an outdir
without getting complaints from GN.

BUG=641155

TEST=gn gen out/T
TEST=gn gen --args='enable_extensions=false' out
TEST=gn gen --args='is_component_build=true enable_extensions=false' out
TEST=gn gen --args='target_os="android"' out
TEST=gn gen --args='target_os="android" enable_extensions=false' out
TEST=gn gen --args='target_os="chromeos"' out
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out
==========

to

==========
Allow generation of a chromium outdirs when enable_extensions=false

This is important to us because it allows us to patch the upstream
tree a little less before building some of our products.

We need an upstream buildbot that checks that these build configurations;
 $ gn gen --args='enable_extensions=false' out
 $ gn gen --args='target_os="android" enable_extensions=false' out
are always OK (even if they don't compile).

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of ours)
that need enable_extensions=false can now generate an outdir
without getting complaints from GN.

BUG=641155

TEST=gn gen out/T
TEST=gn gen --args='enable_extensions=false' out
TEST=gn gen --args='is_component_build=true enable_extensions=false' out
TEST=gn gen --args='target_os="android"' out
TEST=gn gen --args='target_os="android" enable_extensions=false' out
TEST=gn gen --args='target_os="chromeos"' out
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out
==========

[Mostyn Bramley-Moore, 2017-05-23 12:16:21]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 12:16:32]

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, 2017-05-23 12:20:23]

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

[commit-bot: I haz the power, 2017-05-23 12:20:24]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Mostyn Bramley-Moore, 2017-05-23 12:43:55]

The CQ bit was checked by mostynb@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-05-23 12:44:09]

Dry run: CQ is trying da patch.

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

[hugoh_UTC2, 2017-05-23 12:46:22]

Description was changed from

==========
Allow generation of a chromium outdirs when enable_extensions=false

This is important to us because it allows us to patch the upstream
tree a little less before building some of our products.

We need an upstream buildbot that checks that these build configurations;
 $ gn gen --args='enable_extensions=false' out
 $ gn gen --args='target_os="android" enable_extensions=false' out
are always OK (even if they don't compile).

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of ours)
that need enable_extensions=false can now generate an outdir
without getting complaints from GN.

BUG=641155

TEST=gn gen out/T
TEST=gn gen --args='enable_extensions=false' out
TEST=gn gen --args='is_component_build=true enable_extensions=false' out
TEST=gn gen --args='target_os="android"' out
TEST=gn gen --args='target_os="android" enable_extensions=false' out
TEST=gn gen --args='target_os="chromeos"' out
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out
==========

to

==========
Allow generation of a chromium outdirs when enable_extensions=false

This is important to us because it allows us to patch the upstream
tree a little less before building some of our products.

We need an upstream buildbot that checks that these build configurations;
 $ gn gen --args='enable_extensions=false' out
 $ gn gen --args='target_os="android" enable_extensions=false' out
are always OK (even if they don't compile).

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of ours)
that need enable_extensions=false can now generate an outdir
without getting complaints from GN.

BUG=641155

TEST=gn gen out/
TEST=gn gen --args='enable_extensions=false' out/
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/
TEST=gn gen --args='target_os="android"' out/
TEST=gn gen --args='target_os="android" enable_extensions=false' out/
TEST=gn gen --args='target_os="chromeos"' out/
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/
==========

[commit-bot: I haz the power, 2017-05-23 14:09:36]

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

[commit-bot: I haz the power, 2017-05-23 14:09:37]

Dry run: This issue passed the CQ dry run.

[Nico, 2017-05-23 14:35:03]

> We need an upstream buildbot that checks that these build configurations;

I'm pretty sure we wouldn't want to add that (it'd require its own try bot, and
we don't have the machines for that, also it'd make everyone pay for this config
even though most people don't use it). Landing this CL seems fine to me, but
it'd be on you to keep that configuration building.

[hugoh_UTC2, 2017-05-23 15:16:34]

On 2017/05/23 14:35:03, Nico wrote:
> > We need an upstream buildbot that checks that these build configurations;
> 
> I'm pretty sure we wouldn't want to add that (it'd require its own try bot,
and
> we don't have the machines for that, also it'd make everyone pay for this
config
> even though most people don't use it). Landing this CL seems fine to me, but
> it'd be on you to keep that configuration building.

Thanks. Oh I don't mean that this target should be built - all I would like a
trybot to do is to check that the commands;
 $ gn gen --args='enable_extensions=false' an_outdir
 $ gn gen --args='target_os="android" enable_extensions=false' an_outdir
exit without errors (Done ... )

Above "test" takes about ~ 2 seconds to run. I guess it could be done at an
existing bot, for example linux_chromium_rel_ng.

The test ensures that we don't add new unconditional deps to //extensions. Let
me explain why this is is important to us. Often when my team brings in a new
chromium version, a new unconditional dep to //extensions was added - this
breaks our enable_extensions=false build until we've added a |if
(extensions_enabled)| around that new dep. So above test would help us avoid a
frequent breakage and thus help us, and other embedders, being closer to
upstream. I think the test is valuable for upstream too to sustain the
enable_extensions *build* argument (if a build argument can't be passed, that is
a breakage IMO).

[michaelpg, 2017-05-23 16:54:40]

(dpranke: Correct me if I'm imbuing .gn files with meaning they aren't meant to
have.)

I only looked at the first few files, but my concern is that the extensions deps
don't *go away* when enable_extensions = false. We still create targets for
files that #include extensions directories, but don't add those deps, which is
why targets like chrome don't compile.

I think it would make more sense and better enforce accurate dependencies if we
simply don't generate targets dependent on extensions when !enable_extensions.

Perhaps there are some configurations that should be usable, like {target_os =
"chromeos", enable_extensions = false}, but the burden should be on the targets
that need to be built in that configuration to remove other targets, like
chrome, from that build.

You'll probably need to get support from those targets' OWNERS (eg for
chrome/BUILD.gn) but I think that's WAI.

https://codereview.chromium.org/2904443004/diff/20001/apps/BUILD.gn
File apps/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/20001/apps/BUILD.gn#newcode11
apps/BUILD.gn:11: static_library("apps") {
I would rather this assert(enable_extensions) because apps don't make sense
without extensions -- trying to reason about build behavior, target generation,
dependencies for that case is not useful.

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/devtools...
File chrome/browser/devtools/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/devtools...
chrome/browser/devtools/BUILD.gn:83: deps += [ "//chrome/common/extensions/api"
]
I don't see includes of this dir in c/b/devtools.

The only thing I see is, for the parent dir,
chrome/common/extensions/chrome_manifest_url_handlers.h, but that's part of the
//chrome/common target when enable_extensions = true.

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/media/ro...
File chrome/browser/media/router/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/BUILD.gn:70: "//extensions/browser",
What about this? (!enable_extensions should not apply is_android, right?)

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/BUILD.gn:112: deps += [ "//extensions/common" ]
media-router sources unconditionally include files from extensions/common.
Either the tests target shouldn't exist if !enable_extensions, or... this kind
of defeats the purpose of deps, I think.

[michaelpg, 2017-05-23 16:57:00]

Also, crbug.com/641155 is not directly relevant to this CL. Maybe open a new bug
for discussion?

[Mostyn Bramley-Moore, 2017-05-23 17:46:59]

IMO, to do this cleanly would require:
1) move targets like chromium that depend on extensions inside an if
(enable_extensions) block
2) perhaps as an alternative to (1), assert(enable_extensions) inside gn files
inside //extensions and in gn files which only contain targets that require
extensions support, to at least make it easy for us to diagnose incorrect
unconditional dependencies
3) add an extensions config that defines an ENABLE_EXTENSIONS=1 macro that
source/header files can use if they can be built with or without extensions
support, and have targets inside //extensions include that in their
public_configs or all_dependent_configs

We would still expect to fix breakage ourselves, but at least we would have a
clean design to point reviewers to when submitting such fixes.

[Dirk Pranke, 2017-05-23 20:00:10]

dpranke@chromium.org changed reviewers:
+ brettw@chromium.org

[Dirk Pranke, 2017-05-23 20:00:11]

+brettw ...

I think there's two different questions here.

The first is, do we want to accept patches for a configuration that isn't used
for Chrome (e.g., a Linux build w/o extensions). The answer to this is usually
yes, for reasonable changes like the ones in this patch, as long as people are
willing to support these changes down the road.

And, the general rule of thumb we have is that if we don't have a configuration
being built on the (non-fyi) Chromium waterfalls, then people need to expect
that it'll be broken and they'll need to submit fixes accordingly.

This should all be relatively uncontroversial.

The second question is: do we want to attempt to "partially" support other
configurations, e.g., by running `gn gen --args="enable_extensions=false"` on a
bot, even if we don't actually compile that bot ?

I think this is harder, because it makes it less clear for an average developer
what this configuration is, or what their responsibilities are to keep that
configuration working (as opposed to having something that actually compiles,
which is pretty clear). 

You could look at this as if today we have three categories: "unsupported and we
won't accept patches for it", "unsupported but will accept patches for it",
"supported and checked via bots", and we're asking to add a fourth category of
"kinda supported, at least up through GN". 

And it's unclear if this new fourth category really adds enough benefit to
offset the confusion it might raise, but I think I'm inclined to think that it
doesn't. Perhaps you could say more as to how much it'll help you? Or perhaps
there's actually a better case for adding a new build configuration that is
compiled, and not just "gn-genned"?

https://codereview.chromium.org/2904443004/diff/20001/apps/BUILD.gn
File apps/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/20001/apps/BUILD.gn#newcode11
apps/BUILD.gn:11: static_library("apps") {
On 2017/05/23 16:54:40, michaelpg wrote:
> I would rather this assert(enable_extensions) because apps don't make sense
> without extensions -- trying to reason about build behavior, target
generation,
> dependencies for that case is not useful.

This seems sensible to me.

https://codereview.chromium.org/2904443004/diff/20001/extensions/browser/BUIL...
File extensions/browser/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/20001/extensions/browser/BUIL...
extensions/browser/BUILD.gn:15: public_deps = [
How feasible is it to make it so that nothing under //extensions is getting
included if enable_extensions is false?

i.e., can we get to a point where all //extensions/* BUILD files have
assert(enable_extensions) at the top?

[hugoh_UTC2, 2017-05-24 13:58:06]

https://codereview.chromium.org/2904443004/diff/20001/apps/BUILD.gn
File apps/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/20001/apps/BUILD.gn#newcode11
apps/BUILD.gn:11: static_library("apps") {
On 2017/05/23 20:00:11, Dirk Pranke (slow) wrote:
> On 2017/05/23 16:54:40, michaelpg wrote:
> > I would rather this assert(enable_extensions) because apps don't make sense
> > without extensions -- trying to reason about build behavior, target
> generation,
> > dependencies for that case is not useful.
> 
> This seems sensible to me.

Done. See PS3.

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/media/ro...
File chrome/browser/media/router/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/BUILD.gn:70: "//extensions/browser",
On 2017/05/23 16:54:40, michaelpg wrote:
> What about this? (!enable_extensions should not apply is_android, right?)

True. In PS3 I moved the check for enable_extensions earlier up so we check both
conditions. (Just because we're not on Android, we can't assume that extensions
are enabled.)

https://codereview.chromium.org/2904443004/diff/20001/chrome/browser/media/ro...
chrome/browser/media/router/BUILD.gn:112: deps += [ "//extensions/common" ]
On 2017/05/23 16:54:40, michaelpg wrote:
> media-router sources unconditionally include files from extensions/common.
> Either the tests target shouldn't exist if !enable_extensions, or... this kind
> of defeats the purpose of deps, I think.

When !is_android, static_library("test_support") should only depend on
extensions if extensions_enabled? (But this test target should still exist?)

[hugoh_UTC2, 2017-05-24 14:00:36]

Description was changed from

==========
Allow generation of a chromium outdirs when enable_extensions=false

This is important to us because it allows us to patch the upstream
tree a little less before building some of our products.

We need an upstream buildbot that checks that these build configurations;
 $ gn gen --args='enable_extensions=false' out
 $ gn gen --args='target_os="android" enable_extensions=false' out
are always OK (even if they don't compile).

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of ours)
that need enable_extensions=false can now generate an outdir
without getting complaints from GN.

BUG=641155

TEST=gn gen out/
TEST=gn gen --args='enable_extensions=false' out/
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/
TEST=gn gen --args='target_os="android"' out/
TEST=gn gen --args='target_os="android" enable_extensions=false' out/
TEST=gn gen --args='target_os="chromeos"' out/
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/
==========

to

==========
Allow generation of a chromium outdirs when enable_extensions=false

This is important to us because it allows us to patch the upstream
tree a little less before building some of our products.

Ideally, an upstream buildbot would check that these build configurations;
 $ gn gen --args='enable_extensions=false' out/
 $ gn gen --args='target_os="android" enable_extensions=false' out/
are always OK (even if they don't compile).

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of ours)
that need enable_extensions=false can now generate an outdir
without getting complaints from GN.

BUG=725906

TEST=gn gen out/
TEST=gn gen --args='enable_extensions=false' out/
TEST=gn gen --args='is_component_build=true' out/
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/
TEST=gn gen --args='target_os="android"' out/
TEST=gn gen --args='target_os="android" enable_extensions=false' out/
TEST=gn gen --args='target_os="chromeos"' out/
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/
==========

[Dirk Pranke, 2017-05-24 16:44:00]

To repeat my earlier question, in case you missed it:

Can we get to a point where all //extensions/* BUILD files have
assert(enable_extensions) at the top, and nothing under //extensions is getting
included if enable_extensions is false

https://codereview.chromium.org/2904443004/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/40001/BUILD.gn#newcode156
BUILD.gn:156: }
Can we move this block out of the condition (on line 95) altogether?

[Mostyn Bramley-Moore, 2017-05-24 16:57:42]

On 2017/05/24 16:44:00, Dirk Pranke (slow) wrote:
> To repeat my earlier question, in case you missed it:
> 
> Can we get to a point where all //extensions/* BUILD files have
> assert(enable_extensions) at the top, and nothing under //extensions is
getting
> included if enable_extensions is false

We have been discussing this option amongst ourselves.  Tomorrow is a public
holiday here in Sweden, we will experiment a bit on friday and let you know how
this turns out.

[hugoh_UTC2, 2017-05-26 14:25:18]

On 2017/05/24 16:57:42, Mostyn Bramley-Moore wrote:
> On 2017/05/24 16:44:00, Dirk Pranke (slow) wrote:
> > To repeat my earlier question, in case you missed it:
> > 
> > Can we get to a point where all //extensions/* BUILD files have
> > assert(enable_extensions) at the top, and nothing under //extensions is
> getting
> > included if enable_extensions is false
> 
> We have been discussing this option amongst ourselves.  Tomorrow is a public
> holiday here in Sweden, we will experiment a bit on friday and let you know
how
> this turns out.

Today we tested to add this assert to all extensions/*/BUILD.gn files:
  assert(enable_extensions,
         "Dependency on extensions, but enable_extensions=false.")

Chrome obviously needs //extensions so the idea was to not generate the
chrome-target when enable_extensions=false. But we didn't manage so far...

The root BUILD.gn's group("gn_all") - that I understand is used when not
specifying a target as in [1] - has >40 dependencies to things in //chrome.
These things often depend on //chrome/browser or another target that eventually
depends on //extensions - so an assert will fail. Tricky.

We also spotted that some //components use //extensions unconditionally (see
crbug.com/726700).

[1] gn gen --args='enable_extensions=false' out/Disable

[Dirk Pranke, 2017-06-07 20:15:10]

On 2017/05/26 14:25:18, hugoh_UTC2 wrote:
> On 2017/05/24 16:57:42, Mostyn Bramley-Moore wrote:
> > On 2017/05/24 16:44:00, Dirk Pranke (slow) wrote:
> > > To repeat my earlier question, in case you missed it:
> > > 
> > > Can we get to a point where all //extensions/* BUILD files have
> > > assert(enable_extensions) at the top, and nothing under //extensions is
> > getting
> > > included if enable_extensions is false
> > 
> > We have been discussing this option amongst ourselves.  Tomorrow is a public
> > holiday here in Sweden, we will experiment a bit on friday and let you know
> how
> > this turns out.
> 
> Today we tested to add this assert to all extensions/*/BUILD.gn files:
>   assert(enable_extensions,
>          "Dependency on extensions, but enable_extensions=false.")
> 
> Chrome obviously needs //extensions so the idea was to not generate the
> chrome-target when enable_extensions=false. But we didn't manage so far...
> 
> The root BUILD.gn's group("gn_all") - that I understand is used when not
> specifying a target as in [1] - has >40 dependencies to things in //chrome.
> These things often depend on //chrome/browser or another target that
eventually
> depends on //extensions - so an assert will fail. Tricky.
> 
> We also spotted that some //components use //extensions unconditionally (see
> crbug.com/726700).
> 
> [1] gn gen --args='enable_extensions=false' out/Disable

Okay, thanks for the follow-up. We should try to get that cleaned up.

We do actually support configurations that disable extensions (android, ios); 
do we know why they don't run into this problem? Is it that extensions get
pulled in
on the non-{android,ios} platforms?

I'm not sure what the state of this CL is. It sounds like at least some of these
edits are better addressed as fixes in the dependent build files. But, you'll
probably 
still need something like this top-level change as well as a workaround until
the
layering violations are fixed.

So, should we review this CL as-is while we're working on crbug.com/726700 ? Or
do we need an updated version?

As to the other half of the request (about running GN on configurations that
otherwise
don't have matching trybots), I think we're reluctant to do that at this time;
it raises
complicated questions about what configurations are supported or not. I am happy
to raise those questions and have that discussion, but this CL is not the right
forum.

I suggest maybe you file a bug or start a separate thread on chromium-dev, and
we can have a more visible discussion there. How does that sound?

[hugoh_UTC2, 2017-06-08 10:57:11]

Patchset #4 (id:60001) has been deleted

[hugoh_UTC2, 2017-06-08 12:07:08]

Description was changed from

==========
Allow generation of a chromium outdirs when enable_extensions=false

This is important to us because it allows us to patch the upstream
tree a little less before building some of our products.

Ideally, an upstream buildbot would check that these build configurations;
 $ gn gen --args='enable_extensions=false' out/
 $ gn gen --args='target_os="android" enable_extensions=false' out/
are always OK (even if they don't compile).

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of ours)
that need enable_extensions=false can now generate an outdir
without getting complaints from GN.

BUG=725906

TEST=gn gen out/
TEST=gn gen --args='enable_extensions=false' out/
TEST=gn gen --args='is_component_build=true' out/
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/
TEST=gn gen --args='target_os="android"' out/
TEST=gn gen --args='target_os="android" enable_extensions=false' out/
TEST=gn gen --args='target_os="chromeos"' out/
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/
==========

to

==========
Allow generation of a chromium outdirs when enable_extensions=false

This allows Opera to patch the upstream tree a little less before
building one of our downstream products that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed needs
extensions. But other, similar browser products (some of ours) that
need enable_extensions=false can now generate an outdir without getting
complaints from GN.

BUG=725906

TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest
==========

[hugoh_UTC2, 2017-06-08 12:07:21]

https://codereview.chromium.org/2904443004/diff/40001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/40001/BUILD.gn#newcode156
BUILD.gn:156: }
On 2017/05/24 16:44:00, Dirk Pranke wrote:
> Can we move this block out of the condition (on line 95) altogether?

Done. See PS4.

[hugoh_UTC2, 2017-06-08 12:07:56]

On 2017/06/07 20:15:10, Dirk Pranke wrote:
> On 2017/05/26 14:25:18, hugoh_UTC2 wrote:
> > On 2017/05/24 16:57:42, Mostyn Bramley-Moore wrote:
> > > On 2017/05/24 16:44:00, Dirk Pranke (slow) wrote:
> > > > To repeat my earlier question, in case you missed it:
> > > > 
> > > > Can we get to a point where all //extensions/* BUILD files have
> > > > assert(enable_extensions) at the top, and nothing under //extensions is
> > > getting
> > > > included if enable_extensions is false
> > > 
> > > We have been discussing this option amongst ourselves.  Tomorrow is a
public
> > > holiday here in Sweden, we will experiment a bit on friday and let you
know
> > how
> > > this turns out.
> > 
> > Today we tested to add this assert to all extensions/*/BUILD.gn files:
> >   assert(enable_extensions,
> >          "Dependency on extensions, but enable_extensions=false.")
> > 
> > Chrome obviously needs //extensions so the idea was to not generate the
> > chrome-target when enable_extensions=false. But we didn't manage so far...
> > 
> > The root BUILD.gn's group("gn_all") - that I understand is used when not
> > specifying a target as in [1] - has >40 dependencies to things in //chrome.
> > These things often depend on //chrome/browser or another target that
> eventually
> > depends on //extensions - so an assert will fail. Tricky.
> > 
> > We also spotted that some //components use //extensions unconditionally (see
> > crbug.com/726700).
> > 
> > [1] gn gen --args='enable_extensions=false' out/Disable
> 
> Okay, thanks for the follow-up. We should try to get that cleaned up.
> 
> We do actually support configurations that disable extensions (android, ios); 
> do we know why they don't run into this problem? Is it that extensions get
> pulled in
> on the non-{android,ios} platforms?
> 
> I'm not sure what the state of this CL is. It sounds like at least some of
these
> edits are better addressed as fixes in the dependent build files. But, you'll
> probably 
> still need something like this top-level change as well as a workaround until
> the
> layering violations are fixed.
> 
> So, should we review this CL as-is while we're working on crbug.com/726700 ?
Or
> do we need an updated version?

I just rebased this CL. I think it can be reviewed as it is. PTAL.

> 
> As to the other half of the request (about running GN on configurations that
> otherwise
> don't have matching trybots), I think we're reluctant to do that at this time;
> it raises
> complicated questions about what configurations are supported or not. I am
happy
> to raise those questions and have that discussion, but this CL is not the
right
> forum.
> 
> I suggest maybe you file a bug or start a separate thread on chromium-dev, and
> we can have a more visible discussion there. How does that sound?

Sounds good. I updated this CL's description.

[Dirk Pranke, 2017-06-08 21:19:08]

lgtm if you file a follow-up bug to clean up the dependencies elsewhere in the
codebase so that we can just assert(enable_extensions) at the top of every
//extensions/... build file and add TODO(bug#) comments where you had to add if
(enable_extensions) in those files.

[michaelpg, 2017-06-08 22:54:40]

michaelpg@chromium.org changed reviewers:
+ rdevlin.cronin@chromium.org

[michaelpg, 2017-06-08 22:54:41]

Devlin, can you take a look too?

https://codereview.chromium.org/2904443004/diff/80001/apps/BUILD.gn
File apps/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/80001/apps/BUILD.gn#newcode15
apps/BUILD.gn:15: if (enable_extensions) {
remove if()

https://codereview.chromium.org/2904443004/diff/80001/apps/ui/views/BUILD.gn
File apps/ui/views/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/80001/apps/ui/views/BUILD.gn#...
apps/ui/views/BUILD.gn:8: source_set("views") {
this shouldn't be built if (!enable_extensions)
can the assert stay? only chrome/browser/ui should be depending on this.

[hugoh_UTC2, 2017-06-09 09:23:22]

Patchset #5 (id:100001) has been deleted

[hugoh_UTC2, 2017-06-09 09:25:42]

https://codereview.chromium.org/2904443004/diff/80001/apps/BUILD.gn
File apps/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/80001/apps/BUILD.gn#newcode15
apps/BUILD.gn:15: if (enable_extensions) {
On 2017/06/08 22:54:41, michaelpg wrote:
> remove if()

Done.

https://codereview.chromium.org/2904443004/diff/80001/apps/ui/views/BUILD.gn
File apps/ui/views/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/80001/apps/ui/views/BUILD.gn#...
apps/ui/views/BUILD.gn:8: source_set("views") {
On 2017/06/08 22:54:41, michaelpg wrote:
> this shouldn't be built if (!enable_extensions)
> can the assert stay? only chrome/browser/ui should be depending on this.

You're right. Problem was that chrome/browser/ui/BUILD.gn had an unconditional
dep to //apps/ui/views (=this file). In PS5 I've fixed
chrome/browser/ui/BUILD.gn instead of patching this file.

[hugoh_UTC2, 2017-06-09 12:49:46]

On 2017/06/08 21:19:08, Dirk Pranke wrote:
> lgtm if you file a follow-up bug to clean up the dependencies elsewhere in the
> codebase so that we can just assert(enable_extensions) at the top of every
> //extensions/... build file and add TODO(bug#) comments where you had to add
if
> (enable_extensions) in those files.

In PS6 I added the assert to all but two of the build files in extensions/. In
the two remaining build files I added a TODO comment pointing to a new bug,
crbug.com/731689 .

[michaelpg, 2017-06-09 16:45:40]

note: you'll want to rebase on https://codereview.chromium.org/2905153003

[Devlin, 2017-06-09 19:11:52]

https://codereview.chromium.org/2904443004/diff/140001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/BUILD.gn#newcode214
BUILD.gn:214: deps += [ "//extensions/shell:app_shell_unittests" ]
nit: combine these enable_extension blocks?

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

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/BUILD.g...
chrome/browser/BUILD.gn:3713: deps += [ "//apps" ]
Can we put this in the other enable_extensions block around line 1999 of this
patch?

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/devtool...
File chrome/browser/devtools/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/devtool...
chrome/browser/devtools/BUILD.gn:83: deps += [ "//chrome/common/extensions/api"
]
I think conceptually this probably doesn't belong in a !android block anymore -
maybe just a new block after line 150?

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/media/r...
File chrome/browser/media/router/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/media/r...
chrome/browser/media/router/BUILD.gn:67: if (!is_android && enable_extensions) {
ditto here and line 106 - probably just want an if (enable_extensions) block.

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/api...
File extensions/browser/api/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/api...
extensions/browser/api/BUILD.gn:47: if (enable_extensions) {
why do we need this if with the assert above?

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/app...
File extensions/browser/app_window/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/app...
extensions/browser/app_window/BUILD.gn:39: if (enable_extensions) {
here, too, why this block with the assert?

[hugoh_UTC2, 2017-06-12 09:19:08]

I've fixed Devlin's comments and rebased again. PTAL.

https://codereview.chromium.org/2904443004/diff/140001/BUILD.gn
File BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/BUILD.gn#newcode214
BUILD.gn:214: deps += [ "//extensions/shell:app_shell_unittests" ]
On 2017/06/09 19:11:51, Devlin (sheriff Jun 9 - 12) wrote:
> nit: combine these enable_extension blocks?

Done.

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

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/BUILD.g...
chrome/browser/BUILD.gn:3713: deps += [ "//apps" ]
On 2017/06/09 19:11:51, Devlin (sheriff Jun 9 - 12) wrote:
> Can we put this in the other enable_extensions block around line 1999 of this
> patch?

Done.

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/devtool...
File chrome/browser/devtools/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/devtool...
chrome/browser/devtools/BUILD.gn:83: deps += [ "//chrome/common/extensions/api"
]
On 2017/06/09 19:11:51, Devlin (sheriff Jun 9 - 12) wrote:
> I think conceptually this probably doesn't belong in a !android block anymore
-
> maybe just a new block after line 150?

Done.

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/media/r...
File chrome/browser/media/router/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/chrome/browser/media/r...
chrome/browser/media/router/BUILD.gn:67: if (!is_android && enable_extensions) {
On 2017/06/09 19:11:52, Devlin (sheriff Jun 9 - 12) wrote:
> ditto here and line 106 - probably just want an if (enable_extensions) block.

Done.

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/api...
File extensions/browser/api/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/api...
extensions/browser/api/BUILD.gn:47: if (enable_extensions) {
On 2017/06/09 19:11:52, Devlin (sheriff Jun 9 - 12) wrote:
> why do we need this if with the assert above?

Done. True. Assert is enough.

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/app...
File extensions/browser/app_window/BUILD.gn (right):

https://codereview.chromium.org/2904443004/diff/140001/extensions/browser/app...
extensions/browser/app_window/BUILD.gn:39: if (enable_extensions) {
On 2017/06/09 19:11:52, Devlin (sheriff Jun 9 - 12) wrote:
> here, too, why this block with the assert?

Done. You're right.

[Devlin, 2017-06-12 15:04:50]

lgtm; thank you for the cleanup!

[hugoh_UTC2, 2017-06-12 15:18:16]

Description was changed from

==========
Allow generation of a chromium outdirs when enable_extensions=false

This allows Opera to patch the upstream tree a little less before
building one of our downstream products that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed needs
extensions. But other, similar browser products (some of ours) that
need enable_extensions=false can now generate an outdir without getting
complaints from GN.

BUG=725906

TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest
==========

to

==========
Allow generation of a chromium outdirs when enable_extensions=false

This allows us at Opera to patch chromium a little less before
building a downstream product that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of
ours) that need enable_extensions=false can now generate an
outdir without getting complaints from GN.

BUG=725906

TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest
==========

[hugoh_UTC2, 2017-06-12 15:19:20]

Description was changed from

==========
Allow generation of a chromium outdirs when enable_extensions=false

This allows us at Opera to patch chromium a little less before
building a downstream product that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other, similar browser products (some of
ours) that need enable_extensions=false can now generate an
outdir without getting complaints from GN.

BUG=725906

TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest
==========

to

==========
Allow generation of a chromium outdirs when enable_extensions=false

This allows us at Opera to patch chromium a little less before
building a downstream product that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other browser products that use
enable_extensions=false can now generate an outdir without
getting complaints from GN.

BUG=725906

TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest
==========

[hugoh_UTC2, 2017-06-12 15:22:53]

The CQ bit was checked by hugoh@opera.com

[hugoh_UTC2, 2017-06-12 15:22:54]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/2904443004/#ps200001
(title: "Missed to fix one of Devlin's comments")

[commit-bot: I haz the power, 2017-06-12 15:23:13]

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, 2017-06-12 15:31:58]

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

[commit-bot: I haz the power, 2017-06-12 15:31:59]

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)

[hugoh_UTC2, 2017-06-12 15:56:44]

The CQ bit was checked by hugoh@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-06-12 15:57:04]

Dry run: CQ is trying da patch.

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

[hugoh_UTC2, 2017-06-12 16:24:05]

hugoh@opera.com changed reviewers:
+ benwells@chromium.org, imcheng@chromium.org, pfeldman@chromium.org

[hugoh_UTC2, 2017-06-12 16:24:07]

benwells@, could you approve apps/BUILD.gn and maybe also
extensions/browser/guest_view/extension_view/whitelist/BUILD.gn ?

Missing LGTM from an OWNER for these files:
  apps/BUILD.gn
  chrome/browser/devtools/BUILD.gn 
  chrome/browser/media/router/BUILD.gn
  extensions/browser/guest_view/extension_view/whitelist/BUILD.gn

pfeldman@ could you approve chrome/browser/devtools/BUILD.gn ?
imcheng@ could you approve chrome/browser/media/router/BUILD.gn ?

(Sorry for bothering you all.)

[pfeldman, 2017-06-12 16:50:19]

devtools lgtm

[commit-bot: I haz the power, 2017-06-12 17:29:52]

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

[commit-bot: I haz the power, 2017-06-12 17:29:54]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[imcheng, 2017-06-12 17:53:55]

chrome/browser/media/router/BUILD.gn lgtm

[benwells, 2017-06-13 02:42:28]

lgtm

[hugoh_UTC2, 2017-06-13 07:18:37]

The CQ bit was checked by hugoh@opera.com

[hugoh_UTC2, 2017-06-13 07:18:37]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, rdevlin.cronin@chromium.org, benwells@chromium.org,
imcheng@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2904443004/#ps240001
(title: "Rebase and add assert to extensions/shell/installer/linux/BUILD.gn")

[commit-bot: I haz the power, 2017-06-13 07:18:48]

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, 2017-06-13 07:29:11]

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

[commit-bot: I haz the power, 2017-06-13 07:29:12]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hugoh_UTC2, 2017-06-13 08:34:01]

Missing LGTM from an OWNER for these files:
  extensions/browser/guest_view/extension_view/whitelist/BUILD.gn

brettw@ could you approve
extensions/browser/guest_view/extension_view/whitelist/BUILD.gn ?

michaelpg@ FYI, I added an assert to extensions/shell/installer/linux/BUILD.gn
as well.

[brettw, 2017-06-14 19:17:06]

whitelist LGTM

[hugoh_UTC2, 2017-06-14 20:38:32]

The CQ bit was checked by hugoh@opera.com

[commit-bot: I haz the power, 2017-06-14 20:38:46]

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, 2017-06-14 20:41:43]

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

[commit-bot: I haz the power, 2017-06-14 20:41:45]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator-xco...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[hugoh_UTC2, 2017-06-15 07:28:16]

The CQ bit was checked by hugoh@opera.com

[hugoh_UTC2, 2017-06-15 07:28:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, rdevlin.cronin@chromium.org, benwells@chromium.org,
brettw@chromium.org, imcheng@chromium.org, pfeldman@chromium.org
Link to the patchset: https://codereview.chromium.org/2904443004/#ps260001
(title: "Rebase")

[commit-bot: I haz the power, 2017-06-15 07:28:30]

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, 2017-06-15 09:00:20]

CQ is committing da patch.

Bot data: {"patchset_id": 260001, "attempt_start_ts": 1497511696251180,
"parent_rev": "9c3619e9e404908d6f67e4e7195bd03e5f0316d7", "commit_rev":
"f7a620c87d8df98cbf01a6775c668768d0cd4840"}

[commit-bot: I haz the power, 2017-06-15 09:00:33]

Description was changed from

==========
Allow generation of a chromium outdirs when enable_extensions=false

This allows us at Opera to patch chromium a little less before
building a downstream product that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other browser products that use
enable_extensions=false can now generate an outdir without
getting complaints from GN.

BUG=725906

TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest
==========

to

==========
Allow generation of a chromium outdirs when enable_extensions=false

This allows us at Opera to patch chromium a little less before
building a downstream product that doesn't depend on extensions.

At the moment, the 'chrome' target does not compile when
enable_extensions=false - and this is fine since Chrome indeed
needs extensions. But other browser products that use
enable_extensions=false can now generate an outdir without
getting complaints from GN.

BUG=725906

TEST=gn gen out/GnTest
TEST=gn gen --args='enable_extensions=false' out/GnTest
TEST=gn gen --args='is_component_build=true' out/GnTest
TEST=gn gen --args='is_component_build=true enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="android"' out/GnTest
TEST=gn gen --args='target_os="android" enable_extensions=false' out/GnTest
TEST=gn gen --args='target_os="chromeos"' out/GnTest
TEST=gn gen --args='target_os="chromeos" is_component_build=true' out/GnTest

Review-Url: https://codereview.chromium.org/2904443004
Cr-Commit-Position: refs/heads/master@{#479646}
Committed:
https://chromium.googlesource.com/chromium/src/+/f7a620c87d8df98cbf01a6775c66...
==========

[commit-bot: I haz the power, 2017-06-15 09:00:35]

Committed patchset #12 (id:260001) as
https://chromium.googlesource.com/chromium/src/+/f7a620c87d8df98cbf01a6775c66...

[ashleymarie1, 2017-06-15 15:21:22]

On 2017/06/15 09:00:35, commit-bot: I haz the power wrote:
> Committed patchset #12 (id:260001) as
>
https://chromium.googlesource.com/chromium/src/+/f7a620c87d8df98cbf01a6775c66...

Looks like it may have broke something:
https://bugs.chromium.org/p/chromium/issues/detail?id=733663&can=2&start=0&nu...

[hugoh_UTC2, 2017-06-28 10:23:42]

Patchset #13 (id:280001) has been deleted