Make --force-fieldtrials not activate them in the browser process.

Make --force-fieldtrials not activate them in the browser process.

This is to make this testing flag more consistent with what happens
with a server-side field trial, so that the field trial doesn't
appear in about:version until client code explicitly queries its
state via a group() call.

Does not change the behaviour of the --force-fieldtrials flag
when passes to the renderer process, where it does need to
activate the trials so that they show up in crash reports, etc.

BUG=255142
TEST=Start a debug build of Chrome with the command line
flag --force-fieldtrials=Bat/Man/ and check that it doesn't
show up in chrome://version.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=209795

[Ilya Sherman, 2013-06-28 00:58:07]

LGTM % nits.

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial.cc
File base/metrics/field_trial.cc (right):

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial....
base/metrics/field_trial.cc:394: // crash reports.
nit: Please move this comment inside the if-stmt, or else tweak it to be clear
that this only happens if |activate_trials| is set (and maybe why the behavior
sometimes isn't desirable).

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial.h
File base/metrics/field_trial.h (right):

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial....
base/metrics/field_trial.h:398: bool activate_trials);
nit: Please define an enumerated constant for this, rather than passing in a
boolean, which is hard to interpret at the call sites.  Alternately, since there
seems to be only one client of this code that actually wants to activate all of
the field trials, maybe it makes sense to move the activation logic out of this
method?

[Alexei Svitkine (slow), 2013-06-28 17:05:14]

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial.cc
File base/metrics/field_trial.cc (right):

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial....
base/metrics/field_trial.cc:394: // crash reports.
On 2013/06/28 00:58:07, Ilya Sherman wrote:
> nit: Please move this comment inside the if-stmt, or else tweak it to be clear
> that this only happens if |activate_trials| is set (and maybe why the behavior
> sometimes isn't desirable).

Done.

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial.h
File base/metrics/field_trial.h (right):

https://codereview.chromium.org/17945002/diff/66001/base/metrics/field_trial....
base/metrics/field_trial.h:398: bool activate_trials);
On 2013/06/28 00:58:07, Ilya Sherman wrote:
> nit: Please define an enumerated constant for this, rather than passing in a
> boolean, which is hard to interpret at the call sites.  Alternately, since
there
> seems to be only one client of this code that actually wants to activate all
of
> the field trials, maybe it makes sense to move the activation logic out of
this
> method?

Done.

[Alexei Svitkine (slow), 2013-06-28 17:11:37]

+thakis for chrome/
+jam for content/

[Nico, 2013-06-28 19:53:38]

chrome/ lgtm

https://codereview.chromium.org/17945002/diff/78002/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/17945002/diff/78002/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main.cc:615: const bool result =
base::FieldTrialList::CreateTrialsFromString(
optional nit: const looks weird here (since the variable is in scope for only
two lines)

[jam, 2013-06-28 22:17:59]

lgtm

[Alexei Svitkine (slow), 2013-07-02 18:14:25]

https://codereview.chromium.org/17945002/diff/78002/chrome/browser/chrome_bro...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/17945002/diff/78002/chrome/browser/chrome_bro...
chrome/browser/chrome_browser_main.cc:615: const bool result =
base::FieldTrialList::CreateTrialsFromString(
On 2013/06/28 19:53:38, Nico wrote:
> optional nit: const looks weird here (since the variable is in scope for only
> two lines)

Done.

[commit-bot: I haz the power, 2013-07-02 18:38:05]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/asvitkine@chromium.org/17945002/93001

[commit-bot: I haz the power, 2013-07-02 22:25:07]

Change committed as 209795