Enabled client side sampling for user metrics reporting.

Enabled client side sampling for user metrics reporting.

This does not include reporting sampling rate as a histogram value.

BUG=609987
Committed: https://crrev.com/a6bd8b9eb0836ceac85a9fe7e719b1c4da1d26ae
Cr-Commit-Position: refs/heads/master@{#407858}

[jwd, 2016-07-18 14:51:49]

Patchset #2 (id:20001) has been deleted

[jwd, 2016-07-18 14:51:59]

Patchset #2 (id:40001) has been deleted

[jwd, 2016-07-18 14:53:47]

Description was changed from

==========
Enabled client side sampling for user metrics reporting.

BUG=
==========

to

==========
Enabled client side sampling for user metrics reporting.

This does not include reporting sampling rate as a histogram value.

BUG=609987
==========

[jwd, 2016-07-18 14:54:17]

jwd@chromium.org changed reviewers:
+ asvitkine@google.com, gayane@chromium.org

[jwd, 2016-07-19 15:13:11]

jwd@chromium.org changed reviewers:
+ asvitkine@chromium.org
- asvitkine@google.com

[jwd, 2016-07-19 15:13:33]

Adding the correct Alexei account...

[Alexei Svitkine (slow), 2016-07-19 15:23:59]

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

https://codereview.chromium.org/1951553002/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_services_manager_client.h:26: extern const
base::Feature kMetricsReportingFeature;
If this will not be used outside the file, just put it in the anon namespace in
the file and don't declare it in the header.

https://codereview.chromium.org/1951553002/diff/60001/chrome/browser/metrics/...
File chrome/browser/metrics/metrics_reporting_state.cc (right):

https://codereview.chromium.org/1951553002/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/metrics_reporting_state.cc:71:
g_browser_process->GetMetricsServicesManager()->UpdateUploadPermissions(true);
Shouldn't this be updated_pref value and not |true|?

Can this have a unit test?

[jwd, 2016-07-19 21:21:49]

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

https://codereview.chromium.org/1951553002/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/chrome_metrics_services_manager_client.h:26: extern const
base::Feature kMetricsReportingFeature;
On 2016/07/19 15:23:59, Alexei Svitkine (slow) wrote:
> If this will not be used outside the file, just put it in the anon namespace
in
> the file and don't declare it in the header.

It's going to be used by a metrics provider for reporting the sampling rate.

https://codereview.chromium.org/1951553002/diff/60001/chrome/browser/metrics/...
File chrome/browser/metrics/metrics_reporting_state.cc (right):

https://codereview.chromium.org/1951553002/diff/60001/chrome/browser/metrics/...
chrome/browser/metrics/metrics_reporting_state.cc:71:
g_browser_process->GetMetricsServicesManager()->UpdateUploadPermissions(true);
On 2016/07/19 15:23:59, Alexei Svitkine (slow) wrote:
> Shouldn't this be updated_pref value and not |true|?
No, the true refers to the "can upload" permission, which is the android
connection type related stuff. It's the same as
https://cs.chromium.org/chromium/src/chrome/browser/chrome_browser_main.cc?l=807
done on startup.

SetMetricsReporting is only called from  InitiateMetricsReportingChange, which
isn't called on Android. Android settings use a totally different method, and
doesn't actually update the running services.

I had considered having an unsupported platform error at the top of this
function for android, and getting rid of the if !defined block.
> 
> Can this have a unit test?
Probably yes, I'll do that.

[Alexei Svitkine (slow), 2016-07-26 17:22:48]

lgtm

[jwd, 2016-07-26 17:25:34]

The CQ bit was checked by jwd@chromium.org

[commit-bot: I haz the power, 2016-07-26 17:25:53]

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-07-26 18:37:09]

Description was changed from

==========
Enabled client side sampling for user metrics reporting.

This does not include reporting sampling rate as a histogram value.

BUG=609987
==========

to

==========
Enabled client side sampling for user metrics reporting.

This does not include reporting sampling rate as a histogram value.

BUG=609987
==========

[commit-bot: I haz the power, 2016-07-26 18:37:10]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-07-26 18:40:22]

Description was changed from

==========
Enabled client side sampling for user metrics reporting.

This does not include reporting sampling rate as a histogram value.

BUG=609987
==========

to

==========
Enabled client side sampling for user metrics reporting.

This does not include reporting sampling rate as a histogram value.

BUG=609987

Committed: https://crrev.com/a6bd8b9eb0836ceac85a9fe7e719b1c4da1d26ae
Cr-Commit-Position: refs/heads/master@{#407858}
==========

[commit-bot: I haz the power, 2016-07-26 18:40:23]

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