[Chromecast] Replace architecure-based conditionals with flag.

[Chromecast] Replace architecture-based conditionals with flag.

Chromecast has historically used the target architecture as a proxy for
the type of device targeted (embedded vs. desktop). Looking to the
future, the decisions should be made on a per-feature basis and should
be independent of the architecture. Replace each of the architecture
conditionals with a flag that indicates Cast is being built for desktop.

BUG=

Committed: https://crrev.com/5fcffc49276168c1e80ca5f0ba6d1c13fdd8dd02
Cr-Commit-Position: refs/heads/master@{#365452}

[slan, 2015-12-15 01:40:06]

slan@chromium.org changed reviewers:
+ halliwell@chromium.org, smcgruer@google.com

[slan, 2015-12-15 01:40:06]

PS2 cleans up all the GN instances. GYP coming in next PS.

[halliwell, 2015-12-15 02:18:39]

On 2015/12/15 01:40:06, slan wrote:
> PS2 cleans up all the GN instances. GYP coming in next PS.

fix "architecure" typo in CL description.

[halliwell, 2015-12-15 02:18:46]

https://codereview.chromium.org/1528533002/diff/20001/media/media_options.gni
File media/media_options.gni (right):

https://codereview.chromium.org/1528533002/diff/20001/media/media_options.gni...
media/media_options.gni:41: is_cast_dekstop_build) {
fix "dekstop"

https://codereview.chromium.org/1528533002/diff/20001/ui/ozone/ozone.gni
File ui/ozone/ozone.gni (right):

https://codereview.chromium.org/1528533002/diff/20001/ui/ozone/ozone.gni#newc...
ui/ozone/ozone.gni:46: # platform on every A/V build.
I don't think this has anything to do with CMA.  The TODO here is to create a
cast vendor graphics shared lib implementation for desktop that uses x11.  And
can leave my name against it.

[slan, 2015-12-15 02:50:00]

Description was changed from

==========
[Chromecast] Replace architecure-based conditionals with flag.

Chromecast has historically used the target architcture as a proxy for
the type of device targetted (embedded vs. desktop). Looking to the
future, the decisions should be made on a per-feature basis and should
be independent of the architecture. Replace each of the architecture
conditionals with a flag that indicates Cast is being built for desktop.

BUG=
==========

to

==========
[Chromecast] Replace architecture-based conditionals with flag.

Chromecast has historically used the target architecture as a proxy for
the type of device targeted (embedded vs. desktop). Looking to the
future, the decisions should be made on a per-feature basis and should
be independent of the architecture. Replace each of the architecture
conditionals with a flag that indicates Cast is being built for desktop.

BUG=
==========

[slan, 2015-12-15 02:50:54]

On 2015/12/15 02:18:39, halliwell wrote:
> On 2015/12/15 01:40:06, slan wrote:
> > PS2 cleans up all the GN instances. GYP coming in next PS.
> 
> fix "architecure" typo in CL description.

Impressively, I spelled architecture 3 unique ways in the original CL. Done.

[slan, 2015-12-15 03:11:12]

GYP fixes still on the way, sometime later tonight.

https://codereview.chromium.org/1528533002/diff/20001/media/media_options.gni
File media/media_options.gni (right):

https://codereview.chromium.org/1528533002/diff/20001/media/media_options.gni...
media/media_options.gni:41: is_cast_dekstop_build) {
On 2015/12/15 02:18:46, halliwell wrote:
> fix "dekstop"

Good catch... turns out the logic wasn't correct either. Fixed.

https://codereview.chromium.org/1528533002/diff/20001/ui/ozone/ozone.gni
File ui/ozone/ozone.gni (right):

https://codereview.chromium.org/1528533002/diff/20001/ui/ozone/ozone.gni#newc...
ui/ozone/ozone.gni:46: # platform on every A/V build.
On 2015/12/15 02:18:46, halliwell wrote:
> I don't think this has anything to do with CMA.  The TODO here is to create a
> cast vendor graphics shared lib implementation for desktop that uses x11.  And
> can leave my name against it.

Right you are, TODO updated and reassigned.

[slan, 2015-12-15 03:52:30]

GYP updated, please take a look.

Stephen, please give the GYP build a go for your platform when you get the
chance. I will kick off some builds over here to make sure things look good.

[slan, 2015-12-15 04:15:24]

Excluding the declaration of the flag itself, there are only three callsites for
this flag:

use_alsa: This can be removed as soon as we upstream the ALSA CMA backend. I am
doing that this week as part of the mojo effort.

selecting ozone platforms: We have a clear path forward here. As soon as we
implement graphics and media CMA implementations, we can remove this instance.

test filters: These should not apply to all embedded devices universally. We
need to think of a better way to filter tests by platform

This means that this flag is very short-lived upstream. Downstream, there are a
handful of other cases, but these can probably be resolved in a similar fashion.

https://codereview.chromium.org/1528533002/diff/60001/chromecast/BUILD.gn
File chromecast/BUILD.gn (right):

https://codereview.chromium.org/1528533002/diff/60001/chromecast/BUILD.gn#new...
chromecast/BUILD.gn:54: if (target_os == "linux" && !is_cast_desktop_build) {
Filters need to be evaluated. These are most likely Chromecast-specific.

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp#newcode16
media/media.gyp:16: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
(chromecast==0 or is_cast_desktop_build==1)', {
We can get rid of this as soon as we have a CMA backend for desktop. We want all
AudioOutputStreams going into CMA. Same for midi.gyp and
//media/media_options.gni

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

https://codereview.chromium.org/1528533002/diff/60001/media/media_options.gni...
media/media_options.gni:41: (!is_chromecast || is_cast_desktop_build)) {
Can remove soon, see media.gyp.

https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp
File media/midi/midi.gyp (right):

https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp#new...
media/midi/midi.gyp:9: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
(chromecast==0 or is_cast_desktop_build==1)', {
Can be removed, see media.gyp

[halliwell, 2015-12-15 15:41:55]

https://codereview.chromium.org/1528533002/diff/60001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1528533002/diff/60001/build/common.gypi#newco...
build/common.gypi:2404: ['OS=="linux" and is_cast_desktop_build==1', {
no need for OS=="linux" here.

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp#newcode16
media/media.gyp:16: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
(chromecast==0 or is_cast_desktop_build==1)', {
What was the old "embedded!=1" condition for and why has that disappeared?

https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp
File media/midi/midi.gyp (right):

https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp#new...
media/midi/midi.gyp:9: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
(chromecast==0 or is_cast_desktop_build==1)', {
ditto on 'embedded'

[slan, 2015-12-15 15:59:35]

https://codereview.chromium.org/1528533002/diff/60001/build/common.gypi
File build/common.gypi (right):

https://codereview.chromium.org/1528533002/diff/60001/build/common.gypi#newco...
build/common.gypi:2404: ['OS=="linux" and is_cast_desktop_build==1', {
On 2015/12/15 15:41:55, halliwell wrote:
> no need for OS=="linux" here.

Done.

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp#newcode16
media/media.gyp:16: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
(chromecast==0 or is_cast_desktop_build==1)', {
On 2015/12/15 15:41:55, halliwell wrote:
> What was the old "embedded!=1" condition for and why has that disappeared?

'embedded' is synonymous with 'chromecast' (even on desktop....); we have
removed this flag in GN.

https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp
File media/midi/midi.gyp (right):

https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp#new...
media/midi/midi.gyp:9: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
(chromecast==0 or is_cast_desktop_build==1)', {
On 2015/12/15 15:41:55, halliwell wrote:
> ditto on 'embedded'

See above.

[halliwell, 2015-12-15 16:09:12]

On 2015/12/15 15:59:35, slan wrote:
> https://codereview.chromium.org/1528533002/diff/60001/build/common.gypi
> File build/common.gypi (right):
> 
>
https://codereview.chromium.org/1528533002/diff/60001/build/common.gypi#newco...
> build/common.gypi:2404: ['OS=="linux" and is_cast_desktop_build==1', {
> On 2015/12/15 15:41:55, halliwell wrote:
> > no need for OS=="linux" here.
> 
> Done.
> 
> https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp
> File media/media.gyp (right):
> 
>
https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp#newcode16
> media/media.gyp:16: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
> (chromecast==0 or is_cast_desktop_build==1)', {
> On 2015/12/15 15:41:55, halliwell wrote:
> > What was the old "embedded!=1" condition for and why has that disappeared?
> 
> 'embedded' is synonymous with 'chromecast' (even on desktop....); we have
> removed this flag in GN.
> 
> https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp
> File media/midi/midi.gyp (right):
> 
>
https://codereview.chromium.org/1528533002/diff/60001/media/midi/midi.gyp#new...
> media/midi/midi.gyp:9: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
> (chromecast==0 or is_cast_desktop_build==1)', {
> On 2015/12/15 15:41:55, halliwell wrote:
> > ditto on 'embedded'
> 
> See above.

chromecast/ lgtm

[halliwell, 2015-12-15 16:09:18]

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp
File media/media.gyp (right):

https://codereview.chromium.org/1528533002/diff/60001/media/media.gyp#newcode16
media/media.gyp:16: ['(OS=="linux" or OS=="freebsd" or OS=="solaris") and
(chromecast==0 or is_cast_desktop_build==1)', {
On 2015/12/15 15:59:35, slan wrote:
> On 2015/12/15 15:41:55, halliwell wrote:
> > What was the old "embedded!=1" condition for and why has that disappeared?
> 
> 'embedded' is synonymous with 'chromecast' (even on desktop....); we have
> removed this flag in GN.

Aha ... didn't know that :)  Thanks!

[spang, 2015-12-15 16:49:49]

spang@chromium.org changed reviewers:
+ spang@chromium.org

[spang, 2015-12-15 16:49:50]

lgtm

[slan, 2015-12-15 17:16:00]

+dalecurtis@ for //media/OWNERS
+dpranke@ for //build/common.gypi OWNERS

Dale and Dirk, 

We are breaking our habit of using target_cpu=="arm" to mean an embedded
Chromecast build. Separately, each use of this new flag should disappear from
public code over the next couple of weeks as we add feature support to x86
desktop.

Please take a look.

[slan, 2015-12-15 17:16:19]

slan@chromium.org changed reviewers:
+ dalecurtis@chromium.org, dpranke@chromium.org

[slan, 2015-12-15 17:16:42]

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

[commit-bot: I haz the power, 2015-12-15 17:16:56]

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

[DaleCurtis, 2015-12-15 17:20:01]

Assuming spang@ has already reviewed the embedded changes media/ lgtm

[slan, 2015-12-15 20:51:04]

The latest PS corrects the declare_args block in
//build/config/chromecast_build.gni to handle a known issue in GN:
crbug.com/542846. 

Dirk, is this the current best-practice to handle this issue?

Thanks.

[Dirk Pranke, 2015-12-15 22:40:19]

On 2015/12/15 20:51:04, slan wrote:
> The latest PS corrects the declare_args block in
> //build/config/chromecast_build.gni to handle a known issue in GN:
> crbug.com/542846. 
> 
> Dirk, is this the current best-practice to handle this issue?

Yup, that is a good way to handle the issue.

[Dirk Pranke, 2015-12-15 22:40:36]

lgtm

[byungchul, 2015-12-15 23:14:16]

byungchul@chromium.org changed reviewers:
+ byungchul@chromium.org

[byungchul, 2015-12-15 23:14:17]

https://codereview.chromium.org/1528533002/diff/100001/chromecast/BUILD.gn
File chromecast/BUILD.gn (right):

https://codereview.chromium.org/1528533002/diff/100001/chromecast/BUILD.gn#ne...
chromecast/BUILD.gn:98: } else if (is_cast_desktop_build || target_os ==
"android") {
It can be just "else".

[slan, 2015-12-16 02:48:01]

https://codereview.chromium.org/1528533002/diff/100001/chromecast/BUILD.gn
File chromecast/BUILD.gn (right):

https://codereview.chromium.org/1528533002/diff/100001/chromecast/BUILD.gn#ne...
chromecast/BUILD.gn:98: } else if (is_cast_desktop_build || target_os ==
"android") {
On 2015/12/15 23:14:17, byungchul wrote:
> It can be just "else".

This was done to be explicit about the condition:

https://chromiumcodereview.appspot.com/1382713003/diff/1/chromecast/BUILD.gn?...

[slan, 2015-12-16 02:48:06]

The CQ bit was checked by slan@chromium.org

[slan, 2015-12-16 02:48:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
halliwell@chromium.org, dalecurtis@chromium.org, spang@chromium.org
Link to the patchset: https://codereview.chromium.org/1528533002/#ps100001
(title: "Correct declare_args bug.")

[commit-bot: I haz the power, 2015-12-16 02:49:07]

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

[commit-bot: I haz the power, 2015-12-16 05:08:12]

Description was changed from

==========
[Chromecast] Replace architecture-based conditionals with flag.

Chromecast has historically used the target architecture as a proxy for
the type of device targeted (embedded vs. desktop). Looking to the
future, the decisions should be made on a per-feature basis and should
be independent of the architecture. Replace each of the architecture
conditionals with a flag that indicates Cast is being built for desktop.

BUG=
==========

to

==========
[Chromecast] Replace architecture-based conditionals with flag.

Chromecast has historically used the target architecture as a proxy for
the type of device targeted (embedded vs. desktop). Looking to the
future, the decisions should be made on a per-feature basis and should
be independent of the architecture. Replace each of the architecture
conditionals with a flag that indicates Cast is being built for desktop.

BUG=
==========

[commit-bot: I haz the power, 2015-12-16 05:08:13]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2015-12-16 05:09:58]

Description was changed from

==========
[Chromecast] Replace architecture-based conditionals with flag.

Chromecast has historically used the target architecture as a proxy for
the type of device targeted (embedded vs. desktop). Looking to the
future, the decisions should be made on a per-feature basis and should
be independent of the architecture. Replace each of the architecture
conditionals with a flag that indicates Cast is being built for desktop.

BUG=
==========

to

==========
[Chromecast] Replace architecture-based conditionals with flag.

Chromecast has historically used the target architecture as a proxy for
the type of device targeted (embedded vs. desktop). Looking to the
future, the decisions should be made on a per-feature basis and should
be independent of the architecture. Replace each of the architecture
conditionals with a flag that indicates Cast is being built for desktop.

BUG=

Committed: https://crrev.com/5fcffc49276168c1e80ca5f0ba6d1c13fdd8dd02
Cr-Commit-Position: refs/heads/master@{#365452}
==========

[commit-bot: I haz the power, 2015-12-16 05:10:03]

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