Move EntropyProvider Source From MetricsService to MetricsServicesManager

Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG=636518
Committed: https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa
Cr-Commit-Position: refs/heads/master@{#418866}

[robliao, 2016-09-14 19:40:35]

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

[commit-bot: I haz the power, 2016-09-14 19:41:27]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[robliao, 2016-09-14 19:55:51]

robliao@chromium.org changed reviewers:
+ asvitkine@chromium.org

[commit-bot: I haz the power, 2016-09-14 20:15:52]

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

[commit-bot: I haz the power, 2016-09-14 20:15:53]

Dry run: Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[robliao, 2016-09-14 20:24:08]

Description was changed from

==========
Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG=642473
==========

to

==========
Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG=636518
==========

[Alexei Svitkine (slow), 2016-09-14 20:26:38]

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
File ios/chrome/browser/ios_chrome_main_parts.mm (right):

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
ios/chrome/browser/ios_chrome_main_parts.mm:197: // randomization.
Can you add the DCHECK() that's in the non-iOS code here too, for consistency?
Thanks!

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
ios/chrome/browser/ios_chrome_main_parts.mm:225: // Must initialize metrics
after labs have been converted into switches,
Nit: "labs" terminology is pretty outdated. Given you're moving this, change
"labs" to "about:flags". Here and in the non-iOS one. In the non-iOS code move
the comment to the same place as here, please.

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
ios/chrome/browser/ios_chrome_main_parts.mm:229: 
Nit: Make it consistent with the non-iOS code - either an empty line on both or
no empty line in both. (I don't have a preference either way).

[Alexei Svitkine (slow), 2016-09-14 20:26:49]

LGTM % comments above

Thanks!

[robliao, 2016-09-14 20:28:31]

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

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

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[robliao, 2016-09-14 20:44:04]

Patchset #3 (id:40001) has been deleted

[robliao, 2016-09-14 20:51:24]

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

[commit-bot: I haz the power, 2016-09-14 20:51:51]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[robliao, 2016-09-14 20:54:42]

robliao@chromium.org changed reviewers:
+ brettw@chromium.org, sdefresne@chromium.org

[robliao, 2016-09-14 20:54:42]

sdefresne@chromium.org: Please review changes in 
ios/*

brettw@chromium.org: Please review changes in 
chrome/browser/*

Thanks!

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
File ios/chrome/browser/ios_chrome_main_parts.mm (right):

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
ios/chrome/browser/ios_chrome_main_parts.mm:197: // randomization.
On 2016/09/14 20:26:37, Alexei Svitkine (very slow) wrote:
> Can you add the DCHECK() that's in the non-iOS code here too, for consistency?
> Thanks!

Done.

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
ios/chrome/browser/ios_chrome_main_parts.mm:225: // Must initialize metrics
after labs have been converted into switches,
On 2016/09/14 20:26:38, Alexei Svitkine (very slow) wrote:
> Nit: "labs" terminology is pretty outdated. Given you're moving this, change
> "labs" to "about:flags". Here and in the non-iOS one. In the non-iOS code move
> the comment to the same place as here, please.

Done.

https://codereview.chromium.org/2333283005/diff/1/ios/chrome/browser/ios_chro...
ios/chrome/browser/ios_chrome_main_parts.mm:229: 
On 2016/09/14 20:26:38, Alexei Svitkine (very slow) wrote:
> Nit: Make it consistent with the non-iOS code - either an empty line on both
or
> no empty line in both. (I don't have a preference either way).

This was lost in a file I forgot to save! It's there now.

[brettw, 2016-09-14 22:22:03]

lgtm

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:803: ->CreateEntropyProvider()
A nice and easy follow-up to this patch would be to fix FieldTrialList to take a
unique_ptr which would improve this code.

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_services_manager_client.h (right):

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_services_manager_client.h:69:
CreateEntropyProvider() override;
I think the function name should be indented 4 spaces here. Now it looks like
two functions.

https://codereview.chromium.org/2333283005/diff/60001/components/metrics_serv...
File components/metrics_services_manager/metrics_services_manager.h (right):

https://codereview.chromium.org/2333283005/diff/60001/components/metrics_serv...
components/metrics_services_manager/metrics_services_manager.h:54:
CreateEntropyProvider();
Indent 4 spaces as above.

https://codereview.chromium.org/2333283005/diff/60001/ios/chrome/browser/ios_...
File ios/chrome/browser/ios_chrome_main_parts.mm (right):

https://codereview.chromium.org/2333283005/diff/60001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_main_parts.mm:201: ->CreateEntropyProvider()
Ditto with unfortunate unique_ptr usage.

[commit-bot: I haz the power, 2016-09-14 22:30:51]

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

[commit-bot: I haz the power, 2016-09-14 22:30:52]

Dry run: This issue passed the CQ dry run.

[sdefresne, 2016-09-15 11:30:07]

lgtm

https://codereview.chromium.org/2333283005/diff/60001/components/metrics_serv...
File components/metrics_services_manager/metrics_services_manager.h (right):

https://codereview.chromium.org/2333283005/diff/60001/components/metrics_serv...
components/metrics_services_manager/metrics_services_manager.h:54:
CreateEntropyProvider();
On 2016/09/14 22:22:03, brettw (ping on IM after 24h) wrote:
> Indent 4 spaces as above.

I think this is correctly indented (at least this is how clang-format indents
methods).

[robliao, 2016-09-15 14:31:18]

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/chrome_b...
File chrome/browser/chrome_browser_main.cc (right):

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/chrome_b...
chrome/browser/chrome_browser_main.cc:803: ->CreateEntropyProvider()
On 2016/09/14 22:22:03, brettw (ping on IM after 24h) wrote:
> A nice and easy follow-up to this patch would be to fix FieldTrialList to take
a
> unique_ptr which would improve this code.

Agreed to that.

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/metrics/...
File chrome/browser/metrics/chrome_metrics_services_manager_client.h (right):

https://codereview.chromium.org/2333283005/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_services_manager_client.h:69:
CreateEntropyProvider() override;
On 2016/09/14 22:22:03, brettw (ping on IM after 24h) wrote:
> I think the function name should be indented 4 spaces here. Now it looks like
> two functions.

Referring to 
https://codereview.chromium.org/2333283005/diff/60001/components/metrics_serv...
on this

https://codereview.chromium.org/2333283005/diff/60001/components/metrics_serv...
File components/metrics_services_manager/metrics_services_manager.h (right):

https://codereview.chromium.org/2333283005/diff/60001/components/metrics_serv...
components/metrics_services_manager/metrics_services_manager.h:54:
CreateEntropyProvider();
On 2016/09/15 11:30:07, sdefresne wrote:
> On 2016/09/14 22:22:03, brettw (ping on IM after 24h) wrote:
> > Indent 4 spaces as above.
> 
> I think this is correctly indented (at least this is how clang-format indents
> methods).

The stated rule:
If you break after the return type of a function declaration or definition, do
not indent.
https://google.github.io/styleguide/cppguide.html#Function_Declarations_and_D...

Personally, I would prefer the 4 space indent here.

https://codereview.chromium.org/2333283005/diff/60001/ios/chrome/browser/ios_...
File ios/chrome/browser/ios_chrome_main_parts.mm (right):

https://codereview.chromium.org/2333283005/diff/60001/ios/chrome/browser/ios_...
ios/chrome/browser/ios_chrome_main_parts.mm:201: ->CreateEntropyProvider()
On 2016/09/14 22:22:03, brettw (ping on IM after 24h) wrote:
> Ditto with unfortunate unique_ptr usage.

Agreed.

[robliao, 2016-09-15 14:33:15]

The CQ bit was checked by robliao@chromium.org

[robliao, 2016-09-15 14:33:15]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2333283005/#ps60001
(title: "CR Feedback")

[commit-bot: I haz the power, 2016-09-15 14:33:38]

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-15 14:37:57]

Description was changed from

==========
Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG=636518
==========

to

==========
Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG=636518
==========

[commit-bot: I haz the power, 2016-09-15 14:37:58]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-09-15 14:40:24]

Description was changed from

==========
Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG=636518
==========

to

==========
Move EntropyProvider Source From MetricsService to MetricsServicesManager

This paves the way to allow Variations initialization to occur without
MetricsService initialization. Deferring MetricsService inititalization
is needed for the Browser Task Scheduler work as the underlying
metrics providers will post asynchronous tasks.

BUG=636518

Committed: https://crrev.com/4bee5d90c00f8b89305d7157659c7567d9843afa
Cr-Commit-Position: refs/heads/master@{#418866}
==========

[commit-bot: I haz the power, 2016-09-15 14:40:26]

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