Introduce flag to disable static popular sites.

Introduce flag to disable static popular sites.

In order to disable the global static defaults while having popular
sites enabled, a separate flag is necessary.

BUG=700872
Review-Url: https://codereview.chromium.org/2741413008
Cr-Commit-Position: refs/heads/master@{#457403}
Committed: https://chromium.googlesource.com/chromium/src/+/f84f9f576720345c30d23204e6329ed2204d1903

[fhorschig, 2017-03-15 09:11:44]

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

[commit-bot: I haz the power, 2017-03-15 09:11:53]

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-03-15 10:26:34]

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

[commit-bot: I haz the power, 2017-03-15 10:26:35]

Dry run: Try jobs failed on following builders:
  android_n5x_swarming_rel on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_n5x_...)

[fhorschig, 2017-03-15 13:21:04]

fhorschig@chromium.org changed reviewers:
+ sfiera@chromium.org

[fhorschig, 2017-03-15 13:21:04]

Hi Chris,
would you please take a look at the new flag that disables the static popular
sites list (even if popular sites are enabled)?

[fhorschig, 2017-03-15 15:09:16]

On 2017/03/15 13:21:04, fhorschig wrote:
> Hi Chris,
> would you please take a look at the new flag that disables the static popular
> sites list (even if popular sites are enabled)?

You don't need to review yet. I am going to use the existing experiment and
just a different parameter.

[fhorschig, 2017-03-16 10:01:49]

Hi Chris,
it's simplified and ready to review!

[sfiera, 2017-03-16 10:15:41]

https://codereview.chromium.org/2741413008/diff/40001/components/ntp_tiles/fi...
File components/ntp_tiles/field_trial.cc (right):

https://codereview.chromium.org/2741413008/diff/40001/components/ntp_tiles/fi...
components/ntp_tiles/field_trial.cc:67: const std::string& trial_value =
base::GetFieldTrialParamValue(
Three things:
* I think the "modern" thing to do is to associate params with a feature.
* If you do this, GetFieldTrialParamByFeatureAsBool() is an appropriate helper.
* Params can be set from the command line. Do you really need to read the
command line directly above?

[fhorschig, 2017-03-16 10:57:18]

Patchset #4 (id:60001) has been deleted

[fhorschig, 2017-03-16 10:57:50]

https://codereview.chromium.org/2741413008/diff/40001/components/ntp_tiles/fi...
File components/ntp_tiles/field_trial.cc (right):

https://codereview.chromium.org/2741413008/diff/40001/components/ntp_tiles/fi...
components/ntp_tiles/field_trial.cc:67: const std::string& trial_value =
base::GetFieldTrialParamValue(
On 2017/03/16 10:15:40, sfiera wrote:
> Three things:
> * I think the "modern" thing to do is to associate params with a feature.
Looks like an alternative approach. I don't need any params with a feature.

> * If you do this, GetFieldTrialParamByFeatureAsBool() is an appropriate
helper.
Changed it to use base::FeatureList::IsEnabled() as I don't need a
parameter anymore.

> * Params can be set from the command line. Do you really need to read the
> command line directly above?
Dropped. Apparently, the same applies for features.

[sfiera, 2017-03-16 11:10:24]

LGTM

[fhorschig, 2017-03-16 11:14:25]

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

[commit-bot: I haz the power, 2017-03-16 11:14:36]

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-03-16 11:56:22]

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

[commit-bot: I haz the power, 2017-03-16 11:56:23]

Dry run: This issue passed the CQ dry run.

[fhorschig, 2017-03-16 12:10:17]

The CQ bit was checked by fhorschig@chromium.org

[commit-bot: I haz the power, 2017-03-16 12:10:35]

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-03-16 12:14:10]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489666217546540,
"parent_rev": "0b8414c122248ea1db697651dee163f7517216dd", "commit_rev":
"f84f9f576720345c30d23204e6329ed2204d1903"}

[commit-bot: I haz the power, 2017-03-16 12:14:51]

Description was changed from

==========
Introduce flag to disable static popular sites.

In order to disable the global static defaults while having popular
sites enabled, a separate flag is necessary.

BUG=700872
==========

to

==========
Introduce flag to disable static popular sites.

In order to disable the global static defaults while having popular
sites enabled, a separate flag is necessary.

BUG=700872

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

[commit-bot: I haz the power, 2017-03-16 12:14:52]

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/f84f9f576720345c30d23204e632...