Add is_chromecast to GN build.

Add is_chromecast to GN build.

There are several specific features needed for Chromecast builds. This
flag will be used to enable and disable these features. This flag is
the equivalent of 'embedded' and 'is_chromecast' in GYP, which mean the
same thing in practice.

BUG=516899
Bug:b/17615224

Committed: https://crrev.com/77bdc2e6b89cac63abb81da88d87bd897da67f71
Cr-Commit-Position: refs/heads/master@{#349961}

[slan, 2015-09-16 02:39:27]

slan@chromium.org changed reviewers:
+ alokp@chromium.org, brettw@chromium.org, dpranke@chromium.org,
gunsch@chromium.org

[slan, 2015-09-16 02:39:28]

All, 

This change duplicates the behavior of the 'chromecast' and 'embedded' GYP flags
by enabling or disabling special features needed by Chromecast. 

Please review the overall structure to see what this change looks like. After we
reach agreement that this is the best solution moving forward, I will pull
owners onto this CL to review their respective pieces. I will separate some of
the changes in //chromecast out into a different CL tomorrow to focus this CL
purely on the features.

This is verified on internal and external builds.

[gunsch, 2015-09-16 04:41:48]

Thanks for the work to push this through. Handful of comments but overall the
structure seems not too invasive and resolves a lot of outstanding issues.

https://codereview.chromium.org/1345013002/diff/60001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1345013002/diff/60001/build/config/features.g...
build/config/features.gni:102: # can be explicitly disabled in gn args.
note: this was raised to declare_args due to this comment, could potentially get
dropped outside declare_args now

https://codereview.chromium.org/1345013002/diff/60001/media/BUILD.gn
File media/BUILD.gn (right):

https://codereview.chromium.org/1345013002/diff/60001/media/BUILD.gn#newcode506
media/BUILD.gn:506: if (use_low_memory_buffer) {
note: I think this is going away shortly with some of the work Sergey is doing

https://codereview.chromium.org/1345013002/diff/60001/media/media_options.gni
File media/media_options.gni (right):

https://codereview.chromium.org/1345013002/diff/60001/media/media_options.gni...
media/media_options.gni:44: (is_chromecast && target_cpu != "arm" && is_linux))
{
why not ARM?

https://codereview.chromium.org/1345013002/diff/60001/ui/ozone/ozone.gni
File ui/ozone/ozone.gni (right):

https://codereview.chromium.org/1345013002/diff/60001/ui/ozone/ozone.gni#newc...
ui/ozone/ozone.gni:43: ozone_platform_ozonex = true
we also use ozone_platform_test (for our automated test running)

[slan, 2015-09-16 16:13:08]

https://codereview.chromium.org/1345013002/diff/60001/build/config/features.gni
File build/config/features.gni (right):

https://codereview.chromium.org/1345013002/diff/60001/build/config/features.g...
build/config/features.gni:102: # can be explicitly disabled in gn args.
On 2015/09/16 04:41:48, gunsch wrote:
> note: this was raised to declare_args due to this comment, could potentially
get
> dropped outside declare_args now

Thanks for the reminder. Dropping this into local scope, essentially reverting
https://codereview.chromium.org/1266693005/.

https://codereview.chromium.org/1345013002/diff/60001/media/BUILD.gn
File media/BUILD.gn (right):

https://codereview.chromium.org/1345013002/diff/60001/media/BUILD.gn#newcode506
media/BUILD.gn:506: if (use_low_memory_buffer) {
On 2015/09/16 04:41:48, gunsch wrote:
> note: I think this is going away shortly with some of the work Sergey is doing

Noted, I will make sure that he is aware of the addition here.

https://codereview.chromium.org/1345013002/diff/60001/media/media_options.gni
File media/media_options.gni (right):

https://codereview.chromium.org/1345013002/diff/60001/media/media_options.gni...
media/media_options.gni:44: (is_chromecast && target_cpu != "arm" && is_linux))
{
On 2015/09/16 04:41:48, gunsch wrote:
> why not ARM?

For now, (is_linux && target_cpu == "arm") => Device build. All of the rendering
on device builds is done through CMA.

https://codereview.chromium.org/1345013002/diff/60001/ui/ozone/ozone.gni
File ui/ozone/ozone.gni (right):

https://codereview.chromium.org/1345013002/diff/60001/ui/ozone/ozone.gni#newc...
ui/ozone/ozone.gni:43: ozone_platform_ozonex = true
On 2015/09/16 04:41:48, gunsch wrote:
> we also use ozone_platform_test (for our automated test running)

Added.

[Dirk Pranke, 2015-09-16 22:19:04]

mostly lgtm ; you'll probably need a review from brettw as well.

https://codereview.chromium.org/1345013002/diff/80001/content/shell/BUILD.gn
File content/shell/BUILD.gn (right):

https://codereview.chromium.org/1345013002/diff/80001/content/shell/BUILD.gn#...
content/shell/BUILD.gn:23: use_custom_freetype = is_linux && !is_chromecast
Does this really need to be in declare_args() ? If a user is never going to
change this variable, you can pull this out of this block.

[slan, 2015-09-17 15:04:07]

PS6 addresses dpranke@'s comment and rebases.

https://codereview.chromium.org/1345013002/diff/80001/content/shell/BUILD.gn
File content/shell/BUILD.gn (right):

https://codereview.chromium.org/1345013002/diff/80001/content/shell/BUILD.gn#...
content/shell/BUILD.gn:23: use_custom_freetype = is_linux && !is_chromecast
On 2015/09/16 22:19:04, Dirk Pranke wrote:
> Does this really need to be in declare_args() ? If a user is never going to
> change this variable, you can pull this out of this block.

Removed.

[slan, 2015-09-17 15:04:09]

PS6 addresses dpranke@'s comment and rebases.

[brettw, 2015-09-17 22:52:14]

LGTM

https://codereview.chromium.org/1345013002/diff/100001/build/config/chromecas...
File build/config/chromecast_build.gni (right):

https://codereview.chromium.org/1345013002/diff/100001/build/config/chromecas...
build/config/chromecast_build.gni:8: # Set this true for a Chromecast build.
It would be good to document here the platforms that are supported for setting
this flag (I assume Linux only?) Also, an assert at the bottom of the file
(outside declare_args) like:
  assert(linux || !is_chromecast, "Chromecast requires Linux")
(or whatever the restriction is) would be helpful.

https://codereview.chromium.org/1345013002/diff/100001/chromecast/build/args.gn
File chromecast/build/args.gn (right):

https://codereview.chromium.org/1345013002/diff/100001/chromecast/build/args....
chromecast/build/args.gn:12: chromecast_branding = "public"
This is the default value, so you can just remove it.

https://codereview.chromium.org/1345013002/diff/100001/chromecast/chromecast.gni
File chromecast/chromecast.gni (right):

https://codereview.chromium.org/1345013002/diff/100001/chromecast/chromecast....
chromecast/chromecast.gni:11: chromecast_branding = "public"
Can this be changed in a followup patch to be a boolean? It prevents typos in
the strings from subtly messing everything up. Chrome uses is_chrome_branded for
this, so a is_chromecast_branded would be nicely symmetrics.

https://codereview.chromium.org/1345013002/diff/100001/chromecast/chromecast....
chromecast/chromecast.gni:34: use_playready = false
Can you document this while you're changing everything in here?

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
File ui/ozone/platform/cast/BUILD.gn (right):

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
ui/ozone/platform/cast/BUILD.gn:28: defines = [ "OZONE_IMPLEMENTATION" ]
Remove this. This is not a component and will actually mess up component builds
linking this to ozone.

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
File ui/ozone/platform/cast/cast.gypi (right):

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
ui/ozone/platform/cast/cast.gypi:33: 'OZONE_IMPLEMENTATION',
Remove this too, it's wrong. It will only actively mess you up on Windows
though, but is misleading and unncessary on Posix.

[slan, 2015-09-18 02:31:26]

https://codereview.chromium.org/1345013002/diff/100001/build/config/chromecas...
File build/config/chromecast_build.gni (right):

https://codereview.chromium.org/1345013002/diff/100001/build/config/chromecas...
build/config/chromecast_build.gni:8: # Set this true for a Chromecast build.
On 2015/09/17 22:52:14, brettw wrote:
> It would be good to document here the platforms that are supported for setting
> this flag (I assume Linux only?) Also, an assert at the bottom of the file
> (outside declare_args) like:
>   assert(linux || !is_chromecast, "Chromecast requires Linux")
> (or whatever the restriction is) would be helpful.

Done. Internal builds support Linux and Android, public builds support Linux.

https://codereview.chromium.org/1345013002/diff/100001/chromecast/build/args.gn
File chromecast/build/args.gn (right):

https://codereview.chromium.org/1345013002/diff/100001/chromecast/build/args....
chromecast/build/args.gn:12: chromecast_branding = "public"
On 2015/09/17 22:52:14, brettw wrote:
> This is the default value, so you can just remove it.

This file is no longer needed. Removed.

https://codereview.chromium.org/1345013002/diff/100001/chromecast/chromecast.gni
File chromecast/chromecast.gni (right):

https://codereview.chromium.org/1345013002/diff/100001/chromecast/chromecast....
chromecast/chromecast.gni:11: chromecast_branding = "public"
On 2015/09/17 22:52:14, brettw wrote:
> Can this be changed in a followup patch to be a boolean? It prevents typos in
> the strings from subtly messing everything up. Chrome uses is_chrome_branded
for
> this, so a is_chromecast_branded would be nicely symmetrics.

This used to be our convention, but we changed it so that the concept of
"branding" was not constrained to a binary value. We use this variable heavily
in our internal code. I have opened up an internal bug where we can discuss this
further: b/24177554

https://codereview.chromium.org/1345013002/diff/100001/chromecast/chromecast....
chromecast/chromecast.gni:34: use_playready = false
On 2015/09/17 22:52:14, brettw wrote:
> Can you document this while you're changing everything in here?

Done.

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
File ui/ozone/platform/cast/BUILD.gn (right):

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
ui/ozone/platform/cast/BUILD.gn:28: defines = [ "OZONE_IMPLEMENTATION" ]
On 2015/09/17 22:52:14, brettw wrote:
> Remove this. This is not a component and will actually mess up component
builds
> linking this to ozone.

Done.

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
File ui/ozone/platform/cast/cast.gypi (right):

https://codereview.chromium.org/1345013002/diff/100001/ui/ozone/platform/cast...
ui/ozone/platform/cast/cast.gypi:33: 'OZONE_IMPLEMENTATION',
On 2015/09/17 22:52:14, brettw wrote:
> Remove this too, it's wrong. It will only actively mess you up on Windows
> though, but is misleading and unncessary on Posix.

Done.

[alokp, 2015-09-18 05:29:24]

lgtm

[slan, 2015-09-18 05:50:51]

slan@chromium.org changed reviewers:
+ dalecurtis@chromium.org, halliwell@chromium.org, pauljensen@chromium.org,
spang@chromium.org

[slan, 2015-09-18 05:50:51]

Adding OWNERs for visibility:
+spang@ for //ui/ozone
+dalecurtis@ for //media
+pauljensen@ for //net

[slan, 2015-09-18 05:52:43]

The CQ bit was checked by slan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-18 05:52:52]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1345013002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1345013002/140001

[commit-bot: I haz the power, 2015-09-18 05:56:34]

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

[commit-bot: I haz the power, 2015-09-18 05:56:35]

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

[DaleCurtis, 2015-09-18 16:50:25]

media/ lgtm

[slan, 2015-09-18 21:14:12]

The CQ bit was checked by slan@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2015-09-18 21:14:45]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1345013002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1345013002/160001

[commit-bot: I haz the power, 2015-09-18 22:09:41]

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

[commit-bot: I haz the power, 2015-09-18 22:09:43]

Dry run: This issue passed the CQ dry run.

[slan, 2015-09-19 00:17:00]

slan@chromium.org changed reviewers:
+ sky@chromium.org

[slan, 2015-09-19 00:17:01]

+sky@ 

PTAL at chrome_tests.gypi and //chrome/test/BUILD.gn. Note that I removed an
exclusion on media_router_ui_browsertest. This was introduced in this CL:
https://codereview.chromium.org/1149893020, and I would like confirmation that
my change is correct.

Thanks for the review. I will land this CL after your lgtm. All other changes
affect only Chromecast builds.

[sky, 2015-09-21 14:52:49]

On 2015/09/19 00:17:01, slan wrote:
> +sky@ 
> 
> PTAL at chrome_tests.gypi and //chrome/test/BUILD.gn. Note that I removed an
> exclusion on media_router_ui_browsertest. This was introduced in this CL:
> https://codereview.chromium.org/1149893020, and I would like confirmation that
> my change is correct.
> 
> Thanks for the review. I will land this CL after your lgtm. All other changes
> affect only Chromecast builds.

Brett is an owner of all these files. Do you really need me too?

[slan, 2015-09-21 15:27:37]

On 2015/09/21 14:52:49, sky wrote:
> On 2015/09/19 00:17:01, slan wrote:
> > +sky@ 
> > 
> > PTAL at chrome_tests.gypi and //chrome/test/BUILD.gn. Note that I removed an
> > exclusion on media_router_ui_browsertest. This was introduced in this CL:
> > https://codereview.chromium.org/1149893020, and I would like confirmation
that
> > my change is correct.
> > 
> > Thanks for the review. I will land this CL after your lgtm. All other
changes
> > affect only Chromecast builds.
> 
> Brett is an owner of all these files. Do you really need me too?

Contacting the original reviewers was unsuccessful last week, so this was an FYI
re: changes to //chrome tests. In practice, the set of tests run on any platform
should not change. 

Landing the patch. Thanks for the quick reply.

[slan, 2015-09-21 15:27:46]

The CQ bit was checked by slan@chromium.org

[slan, 2015-09-21 15:27:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, brettw@chromium.org, dalecurtis@chromium.org,
alokp@chromium.org
Link to the patchset: https://codereview.chromium.org/1345013002/#ps160001
(title: "Correct faulty source exclude in chrome browser tests.")

[commit-bot: I haz the power, 2015-09-21 15:28:03]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1345013002/160001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1345013002/160001

[commit-bot: I haz the power, 2015-09-21 15:30:12]

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

[commit-bot: I haz the power, 2015-09-21 15:30:13]

Try jobs failed on following builders:
  ios_dbg_simulator_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)

[spang, 2015-09-21 15:34:44]

A bit late, but since your bots are all red, I have one last comment.

https://codereview.chromium.org/1345013002/diff/160001/ui/ozone/ozone.gni
File ui/ozone/ozone.gni (right):

https://codereview.chromium.org/1345013002/diff/160001/ui/ozone/ozone.gni#new...
ui/ozone/ozone.gni:9: ozone_auto_platforms = !is_chromecast
Can you drop this hunk and just do

if (is_chromecast) {
  // cast defaults
} else {
  // non-cast defaults
} 

below? Otherwise |ozone_auto_platforms=false| doesn't work as intended.

[spang, 2015-09-21 15:37:37]

https://codereview.chromium.org/1345013002/diff/160001/ui/ozone/ozone.gni
File ui/ozone/ozone.gni (right):

https://codereview.chromium.org/1345013002/diff/160001/ui/ozone/ozone.gni#new...
ui/ozone/ozone.gni:9: ozone_auto_platforms = !is_chromecast
On 2015/09/21 15:34:44, spang wrote:
> Can you drop this hunk and just do
> 
> if (is_chromecast) {
>   // cast defaults
> } else {
>   // non-cast defaults
> } 
> 
> below? Otherwise |ozone_auto_platforms=false| doesn't work as intended.

Er, to be more specific what I meant is

if (ozone_auto_platforms) {
 if (is_chromecast) {
   // cast defaults
 } else {
   // non-cast defaults
 } 
}

[slan, 2015-09-21 16:16:23]

The CQ bit was checked by slan@chromium.org

[slan, 2015-09-21 16:16:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
dpranke@chromium.org, brettw@chromium.org, dalecurtis@chromium.org,
alokp@chromium.org
Link to the patchset: https://codereview.chromium.org/1345013002/#ps180001
(title: "Rebase + ui changes.")

[commit-bot: I haz the power, 2015-09-21 16:16:53]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1345013002/180001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1345013002/180001

[slan, 2015-09-21 16:42:48]

https://codereview.chromium.org/1345013002/diff/160001/ui/ozone/ozone.gni
File ui/ozone/ozone.gni (right):

https://codereview.chromium.org/1345013002/diff/160001/ui/ozone/ozone.gni#new...
ui/ozone/ozone.gni:9: ozone_auto_platforms = !is_chromecast
On 2015/09/21 15:37:37, spang wrote:
> On 2015/09/21 15:34:44, spang wrote:
> > Can you drop this hunk and just do
> > 
> > if (is_chromecast) {
> >   // cast defaults
> > } else {
> >   // non-cast defaults
> > } 
> > 
> > below? Otherwise |ozone_auto_platforms=false| doesn't work as intended.
> 
> Er, to be more specific what I meant is
> 
> if (ozone_auto_platforms) {
>  if (is_chromecast) {
>    // cast defaults
>  } else {
>    // non-cast defaults
>  } 
> }

Done.

[commit-bot: I haz the power, 2015-09-21 17:56:40]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-09-21 17:57:19]

Patchset 10 (id:??) landed as
https://crrev.com/77bdc2e6b89cac63abb81da88d87bd897da67f71
Cr-Commit-Position: refs/heads/master@{#349961}