GN(android): Use foreach() loops when dealing with locales

GN(android): Use foreach() loops when dealing with locales

Committed: https://crrev.com/fe56f1285feafa8fd32aa78ce1d04092e13169e0
Cr-Commit-Position: refs/heads/master@{#421546}

[agrieve, 2016-09-27 01:29:06]

agrieve@chromium.org changed reviewers:
+ michaelbai@chromium.org

[agrieve, 2016-09-27 01:29:07]

From codesearching, I'm fairly confident this is a no-op in terms of
functionality. The only target this actually seems to affect is
//chromecast:chromecast_locales_pak, but that isn't used outside the scope of a
source_set (aka, not on android)

[michaelbai, 2016-09-27 01:44:14]

On 2016/09/27 01:29:07, agrieve wrote:
> From codesearching, I'm fairly confident this is a no-op in terms of
> functionality. The only target this actually seems to affect is
> //chromecast:chromecast_locales_pak, but that isn't used outside the scope of
a
> source_set (aka, not on android)

What make you believe they are not functional? If they don't, we should make
them work.

[agrieve, 2016-09-27 02:48:05]

Description was changed from

==========
Adjust GN locales to reflect actual ones shipped on Android

and utilize foreach where appropriate.
==========

to

==========
GN(android): Use foreach() loops when dealing with locales
==========

[agrieve, 2016-09-27 02:49:48]

On 2016/09/27 01:44:14, michaelbai wrote:
> On 2016/09/27 01:29:07, agrieve wrote:
> > From codesearching, I'm fairly confident this is a no-op in terms of
> > functionality. The only target this actually seems to affect is
> > //chromecast:chromecast_locales_pak, but that isn't used outside the scope
of
> a
> > source_set (aka, not on android)
> 
> What make you believe they are not functional? If they don't, we should make
> them work.

Because they have no android_resources() nor android_assets() that would put
them into an apk.

But... the bigger issue (as you pointed out) is that webview does actually ship
all locales. I've re-worked it to just add a new "android_unsupported_locales"
variables and left "locales" itself alone.

[michaelbai, 2016-09-27 03:04:04]

On 2016/09/27 02:49:48, agrieve wrote:
> On 2016/09/27 01:44:14, michaelbai wrote:
> > On 2016/09/27 01:29:07, agrieve wrote:
> > > From codesearching, I'm fairly confident this is a no-op in terms of
> > > functionality. The only target this actually seems to affect is
> > > //chromecast:chromecast_locales_pak, but that isn't used outside the scope
> of
> > a
> > > source_set (aka, not on android)
> > 
> > What make you believe they are not functional? If they don't, we should make
> > them work.
> 
> Because they have no android_resources() nor android_assets() that would put
> them into an apk.
> 
> But... the bigger issue (as you pointed out) is that webview does actually
ship
> all locales. I've re-worked it to just add a new "android_unsupported_locales"
> variables and left "locales" itself alone.

they were in the APK, I verified a few months ago.

[agrieve, 2016-09-27 03:15:40]

On 2016/09/27 03:04:04, michaelbai wrote:
> On 2016/09/27 02:49:48, agrieve wrote:
> > On 2016/09/27 01:44:14, michaelbai wrote:
> > > On 2016/09/27 01:29:07, agrieve wrote:
> > > > From codesearching, I'm fairly confident this is a no-op in terms of
> > > > functionality. The only target this actually seems to affect is
> > > > //chromecast:chromecast_locales_pak, but that isn't used outside the
scope
> > of
> > > a
> > > > source_set (aka, not on android)
> > > 
> > > What make you believe they are not functional? If they don't, we should
make
> > > them work.
> > 
> > Because they have no android_resources() nor android_assets() that would put
> > them into an apk.
> > 
> > But... the bigger issue (as you pointed out) is that webview does actually
> ship
> > all locales. I've re-worked it to just add a new
"android_unsupported_locales"
> > variables and left "locales" itself alone.
> 
> they were in the APK, I verified a few months ago.

The only apk I see for Chromecast is CastShell.apk, which doesn't include
translations. 
Regardless, I've reverted the change to the locales variable.

[agrieve, 2016-09-27 19:10:20]

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

[commit-bot: I haz the power, 2016-09-27 19:10:44]

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-09-27 20:17:42]

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

[commit-bot: I haz the power, 2016-09-27 20:17:43]

Dry run: This issue passed the CQ dry run.

[michaelbai, 2016-09-28 00:13:03]

LGTM with one comment.

https://codereview.chromium.org/2371053002/diff/40001/build/config/locales.gni
File build/config/locales.gni (right):

https://codereview.chromium.org/2371053002/diff/40001/build/config/locales.gn...
build/config/locales.gni:8: android_unsupported_locales = [
android_unsupported_locales seems not accurate, those languages are supported by
Android platform, I understand it means chrome android, consider we  also build
webview and Monochrome, maybe just say chrome_android_unsupported_locales?

[agrieve, 2016-09-28 14:11:40]

https://codereview.chromium.org/2371053002/diff/40001/build/config/locales.gni
File build/config/locales.gni (right):

https://codereview.chromium.org/2371053002/diff/40001/build/config/locales.gn...
build/config/locales.gni:8: android_unsupported_locales = [
On 2016/09/28 00:13:03, michaelbai wrote:
> android_unsupported_locales seems not accurate, those languages are supported
by
> Android platform, I understand it means chrome android, consider we  also
build
> webview and Monochrome, maybe just say chrome_android_unsupported_locales?

I struggled with this as well. Changed it to "android_chrome_omitted_locales"

[agrieve, 2016-09-28 14:11:45]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2016-09-28 14:11:46]

The patchset sent to the CQ was uploaded after l-g-t-m from
michaelbai@chromium.org
Link to the patchset: https://codereview.chromium.org/2371053002/#ps60001
(title: "rename to android_chrome_omitted_locales")

[commit-bot: I haz the power, 2016-09-28 14:11:58]

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-09-28 14:18:24]

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

[commit-bot: I haz the power, 2016-09-28 14:18:25]

Try jobs failed on following builders:
  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_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_...)

[agrieve, 2016-09-28 14:24:05]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-09-28 14:24:21]

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-09-28 14:30:34]

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

[commit-bot: I haz the power, 2016-09-28 14:30:35]

Try jobs failed on following builders:
  linux_chromium_chromeos_compile_dbg_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  win_chromium_compile_dbg_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_comp...)

[agrieve, 2016-09-28 14:38:38]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-09-28 14:38:56]

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-09-28 14:46:52]

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

[commit-bot: I haz the power, 2016-09-28 14:46:53]

Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[agrieve, 2016-09-28 16:02:27]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2016-09-28 16:02:40]

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-09-28 16:47:54]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-28 16:53:15]

Description was changed from

==========
GN(android): Use foreach() loops when dealing with locales
==========

to

==========
GN(android): Use foreach() loops when dealing with locales

Committed: https://crrev.com/fe56f1285feafa8fd32aa78ce1d04092e13169e0
Cr-Commit-Position: refs/heads/master@{#421546}
==========

[commit-bot: I haz the power, 2016-09-28 16:53:16]

Patchset 4 (id:??) landed as
https://crrev.com/fe56f1285feafa8fd32aa78ce1d04092e13169e0
Cr-Commit-Position: refs/heads/master@{#421546}