Make cld2_table_size configurable in GN builds

Make cld2_table_size configurable in GN builds

BUG=

Committed: https://crrev.com/457c1fdaf8287b8df965379962f16cdcc4da3477
Cr-Commit-Position: refs/heads/master@{#352566}

[Tomasz Moniuszko, 2015-10-01 08:16:01]

tmoniuszko@opera.com changed reviewers:
+ brettw@chromium.org

[brettw, 2015-10-01 17:24:22]

Can you move this while you're fixing it? Thanks!

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

https://codereview.chromium.org/1377333002/diff/1/build/config/features.gni#n...
build/config/features.gni:24: if (is_android || is_ios) {
I notice this is only used in third_party/cld_2/BUILD.gn now. We can just move
this block (enclosed by declare_args()) to that file so it's not global any
more, which will help clean this up.

[Tomasz Moniuszko, 2015-10-02 07:56:41]

tmoniuszko@opera.com changed reviewers:
+ andrewhayden@chromium.org

[Tomasz Moniuszko, 2015-10-02 07:56:41]

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

https://codereview.chromium.org/1377333002/diff/1/build/config/features.gni#n...
build/config/features.gni:24: if (is_android || is_ios) {
On 2015/10/01 17:24:22, brettw wrote:
> I notice this is only used in third_party/cld_2/BUILD.gn now. We can just move
> this block (enclosed by declare_args()) to that file so it's not global any
> more, which will help clean this up.

Done. It seems that there was another cld2_table_size=2 assignment in
third_party/cld_2/BUILD.gn already. I removed it because it would overwrite the
default values. Now cld2_table_size is defined like in GYP version: 0 on Android
and iOS, 2 on other platforms.

[Andrew Hayden (chromium.org), 2015-10-02 09:29:25]

We'd like embedders to still be able to choose the table size that they want to
ship. My GN-foo isn't super: isn't that the goal of features.gni? If we move
this down to third_party/cld2, what's the right way for embedders to override
it?

[Tomasz Moniuszko, 2015-10-02 09:53:04]

On 2015/10/02 09:29:25, Andrew Hayden (chromium.org) wrote:
> We'd like embedders to still be able to choose the table size that they want
to
> ship. My GN-foo isn't super: isn't that the goal of features.gni? If we move
> this down to third_party/cld2, what's the right way for embedders to override
> it?

gn variables declared in declare_args() can be overridden by arguments specified
by command-line --args flag and by arguments listed in args.gn file. It doesn't
matter if variable is global or declared in component's BUILD.gn file.

Comment from build/config/features.gni says:
# This file contains Chrome-feature-related build flags (see ui.gni for
# UI-related ones). These should theoretically be moved to the build files of
# the features themselves.
#
# However, today we have many "bad" dependencies on some of these flags from,
# e.g. base, so they need to be global to match the GYP configuration.

As brettw noticed cld2_table_size variable is used only in
third_party/cld_2/BUILD.gn file so there's no reason to declare it globally.

[Andrew Hayden (chromium.org), 2015-10-02 09:56:05]

Thanks for the clarification! In that case, LGTM and thank you for the cleanup.

[brettw, 2015-10-02 17:14:51]

lgtm

[Tomasz Moniuszko, 2015-10-06 07:32:04]

The CQ bit was checked by tmoniuszko@opera.com

[commit-bot: I haz the power, 2015-10-06 07:33:43]

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

[commit-bot: I haz the power, 2015-10-06 08:23:31]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-06 08:24:16]

Patchset 2 (id:??) landed as
https://crrev.com/457c1fdaf8287b8df965379962f16cdcc4da3477
Cr-Commit-Position: refs/heads/master@{#352566}