Move uniformity trials to its own file.

Move uniformity trials to its own file.

BUG=178748
TEST=No user visible changes.

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

[SteveT, 2013-02-27 16:19:12]

Alexei: PTAL, as discussed.

Rohit: Does this look reasonable given what you need?

[Alexei Svitkine (slow), 2013-02-27 16:24:54]

LGTM with as nit.

I'm assuming you just moved the code and didn't change any of the logic (I
didn't look carefully to check that)

https://codereview.chromium.org/12334124/diff/1/chrome/common/metrics/variati...
File chrome/common/metrics/variations/uniformity_field_trials.cc (right):

https://codereview.chromium.org/12334124/diff/1/chrome/common/metrics/variati...
chrome/common/metrics/variations/uniformity_field_trials.cc:106:
chrome_variations::UNIFORMITY_5_PERCENT_BASE,
Nit: Indent 2? Alternatively, indent 4 the kStartDate in the function above.

[rohitrao (ping after 24h), 2013-02-27 16:26:40]

This looks reasonable.  I will simply call
chrome_variations::SetupUniformityFieldTrials() as part of startup.  Unlike
desktop, I will not create a ChromeBrowserFieldTrials object at all.

There will come a time when we will want some of the browser field trials on
mobile as well.  I think it's ok to wait until we have a concrete example before
trying to solve that problem, though.

[SteveT, 2013-02-27 16:33:48]

Alexei: Yes, no functionality was touched at all. We simply moved everything
into common/.

One more question for you, Alexei. We still activate the dynamic uniformity
trials from chrome_browser_field_trials. I think we're cool with leaving that
there?

Rohit: Yeah, I think this will help reduce the nasty ifdefs in your patch for
now. We'll have to talk about which of the other field trials should live in
both.

https://codereview.chromium.org/12334124/diff/1/chrome/common/metrics/variati...
File chrome/common/metrics/variations/uniformity_field_trials.cc (right):

https://codereview.chromium.org/12334124/diff/1/chrome/common/metrics/variati...
chrome/common/metrics/variations/uniformity_field_trials.cc:106:
chrome_variations::UNIFORMITY_5_PERCENT_BASE,
Haha, I had originally made this indent 2 but then moved it back to 4 because I
didn't want to touch any of the existing code.

But yes, I believe 2 is correct as per a discussion we had in another CL.

[Alexei Svitkine (slow), 2013-02-27 16:37:10]

On 2013/02/27 16:33:48, SteveT wrote:
> Alexei: Yes, no functionality was touched at all. We simply moved everything
> into common/.
> 
> One more question for you, Alexei. We still activate the dynamic uniformity
> trials from chrome_browser_field_trials. I think we're cool with leaving that
> there?

Hmm, it might be good to move to this file too, specifically these ones:

base::FieldTrialList::FindValue("UMA-Dynamic-Binary-Uniformity-Trial");
base::FieldTrialList::FindValue("UMA-Dynamic-Uniformity-Trial");
base::FieldTrialList::FindValue("Test0PercentDefault");

We kind of want them to validate things from the server-side from time to time.

Though, we can do it once VariationsService is supported - i.e. in a later CL if
you prefer.

[SteveT, 2013-02-27 16:39:25]

Given that we're making this refactor to help the mobile teams get things
running offline, and we're not yet supporting the online case yet, I'd say let's
avoid mixing priorities and move that later when we're ready to test the
VariationsService.

(and we won't forget, since we're always testing first with those dynamic
uniformity trials)

[rohitrao (ping after 24h), 2013-02-27 16:41:14]

> Given that we're making this refactor to help the mobile teams get things
> running offline, and we're not yet supporting the online case yet, I'd say
let's
> avoid mixing priorities and move that later when we're ready to test the
> VariationsService.

Agreed.  I suspect that I will start looking at online trials somewhat soon, but
there were still a few prerequisites that need to be in place first, so I'm
totally fine with revisiting this when the time comes.

(LGTM if you were waiting for official approval from me too.)

[commit-bot: I haz the power, 2013-02-27 16:42:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevet@chromium.org/12334124/6001

[commit-bot: I haz the power, 2013-02-27 16:42:30]

Presubmit check for 12334124-6001 failed and returned exit status 1.

INFO:root:Found 4 file(s).

Running presubmit commit checks ...
Running /b/commit-queue/workdir/chromium/PRESUBMIT.py
Running /b/commit-queue/workdir/chromium/chrome/PRESUBMIT.py

** Presubmit ERRORS **
Missing LGTM from an OWNER for these files:
    chrome/chrome_common.gypi

Presubmit checks took 1.1s to calculate.

[SteveT, 2013-02-27 16:43:59]

R+sky for OWNERS approval. PTAL, thanks!

[sky, 2013-02-27 18:12:45]

LGTM

[commit-bot: I haz the power, 2013-02-27 18:16:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/stevet@chromium.org/12334124/6001

[commit-bot: I haz the power, 2013-02-27 20:22:19]

Change committed as 185018

[tfarina, 2013-03-05 03:30:38]

https://codereview.chromium.org/12334124/diff/6001/chrome/common/metrics/vari...
File chrome/common/metrics/variations/uniformity_field_trials.h (right):

https://codereview.chromium.org/12334124/diff/6001/chrome/common/metrics/vari...
chrome/common/metrics/variations/uniformity_field_trials.h:8: #include
"base/time.h"
I think you could have forward declare this,

https://codereview.chromium.org/12334124/diff/6001/chrome/common/metrics/vari...
chrome/common/metrics/variations/uniformity_field_trials.h:15: void
SetupUniformityFieldTrials(const base::Time& install_date);
and passed by value, since Time just holds a int64 type.