One-time reset of UMA client ids and low entropy source.

One-time reset of UMA client ids and low entropy source. 

As we've observed a "cloned install" effect in the wild, 
doing this will de-clone such installs. (Which will work 
in conjunction with other measures to prevent new instances 
of cloned installs appearing). 

Renames client id and low entropy source prefs so that they 
are re-generated the next time Chrome runs. Makes the code 
clear the old prefs, so that they are cleaned up in 
existing local state files (in a future milestone, we can 
remove them completely - http://crbug.com/357704).

(Note: Does not rename kMetricsPermutedEntropyCache as 
that pref is reset when the low entropy id is re-generated.) 

BUG=357618
TEST=Running an UMA-enabled build should result in local state 
getting new values for user_experience_metrics.low_entropy_source2 
and user_experience_metrics.client_id2 and removal of prefs 
low_entropy_source and client_id.
NOTRY=true

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

[Alexei Svitkine (slow), 2014-03-28 15:30:37]

Rob/Jesse, PTAL.

One thing I think that's worth discussing is whether we want to reset
kMetricsClientIDTimestamp as well.

In this CL, I'm going with "we don't", since we treat that value as "UMA enabled
date". If you guys agree, then I think we can rename the pref constant name to
something like kMetricsReportingEnabledTimestamp.

[jwd, 2014-03-28 15:47:51]

https://codereview.chromium.org/217083002/diff/20001/chrome/browser/metrics/m...
File chrome/browser/metrics/metrics_service.cc (right):

https://codereview.chromium.org/217083002/diff/20001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:652:
pref->ClearPref(prefs::kMetricsOldClientID);
Can you log a boolean histogram when a user migrates.

[rkaplow, 2014-03-28 17:20:26]

lgtm

agree with not resetting enabled data timestamp & renaming

https://codereview.chromium.org/217083002/diff/20001/chrome/browser/metrics/m...
File chrome/browser/metrics/metrics_service.cc (right):

https://codereview.chromium.org/217083002/diff/20001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:649: // Make a note of when the user
opted in to UMA.
nit: I find "make a note" still a bit informal considering this is being logged
via uma_enabled_date. I'd reword more

https://codereview.chromium.org/217083002/diff/20001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/217083002/diff/20001/chrome/common/pref_names...
chrome/common/pref_names.cc:1365: const char kMetricsClientID[] =
"user_experience_metrics.client_id2";
Understand this is temp but would still add a comment explaining the 2s here.

[Alexei Svitkine (slow), 2014-03-28 17:39:14]

https://codereview.chromium.org/217083002/diff/20001/chrome/browser/metrics/m...
File chrome/browser/metrics/metrics_service.cc (right):

https://codereview.chromium.org/217083002/diff/20001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:649: // Make a note of when the user
opted in to UMA.
On 2014/03/28 17:20:26, rkaplow wrote:
> nit: I find "make a note" still a bit informal considering this is being
logged
> via uma_enabled_date. I'd reword more

Agreed. Done.

https://codereview.chromium.org/217083002/diff/20001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:652:
pref->ClearPref(prefs::kMetricsOldClientID);
On 2014/03/28 15:47:52, Jesse Doherty wrote:
> Can you log a boolean histogram when a user migrates.

Done.

https://codereview.chromium.org/217083002/diff/20001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/217083002/diff/20001/chrome/common/pref_names...
chrome/common/pref_names.cc:1365: const char kMetricsClientID[] =
"user_experience_metrics.client_id2";
On 2014/03/28 17:20:26, rkaplow wrote:
> Understand this is temp but would still add a comment explaining the 2s here.

Agree about the comment, though the 2 is here to stay.

Added comment to explain this.

[Alexei Svitkine (slow), 2014-03-28 17:41:32]

+thestig, for chrome/common/ OWNERS
+isherman, for metrics OWNERS

[Ilya Sherman, 2014-03-28 18:23:06]

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
File chrome/browser/metrics/metrics_service.cc (right):

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:486: // TODO(asvitkine): Remove these
once a couple of releases have passed.
nit: Please add a specific milestone target to this TODO, and file a bug, so
that future readers of the code don't have to dig through history to figure out
when this TODO was added.

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:648: // Record the timestamp of when
the user opted in to UMA.
nit: Move this inside the if branch, since it doesn't apply to the else branch.

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:653:
UMA_HISTOGRAM_BOOLEAN("UMA.MigratedClientID", true);
You don't seem to ever emit a |false| value to this histogram.  What's the
purpose of tracking this as a histogram?

https://codereview.chromium.org/217083002/diff/40001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/217083002/diff/40001/chrome/common/pref_names...
chrome/common/pref_names.cc:1377: // values have been cleaned up.
nit: Same thing here.

[jwd, 2014-03-28 18:25:54]

LGTM
Are we going to wait for the clump prevention cl to submit this?

[Alexei Svitkine (slow), 2014-03-28 18:40:19]

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
File chrome/browser/metrics/metrics_service.cc (right):

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:486: // TODO(asvitkine): Remove these
once a couple of releases have passed.
On 2014/03/28 18:23:06, Ilya Sherman wrote:
> nit: Please add a specific milestone target to this TODO, and file a bug, so
> that future readers of the code don't have to dig through history to figure
out
> when this TODO was added.

Done. The milestone is on the bug.

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:648: // Record the timestamp of when
the user opted in to UMA.
On 2014/03/28 18:23:06, Ilya Sherman wrote:
> nit: Move this inside the if branch, since it doesn't apply to the else
branch.

Done.

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:653:
UMA_HISTOGRAM_BOOLEAN("UMA.MigratedClientID", true);
On 2014/03/28 18:23:06, Ilya Sherman wrote:
> You don't seem to ever emit a |false| value to this histogram.  What's the
> purpose of tracking this as a histogram?

This will allow analysis of when users migrated their ids. (We don't care about
the actual counts of this, since all UMA users on M35 will be affected by this.)
But we may want to know when a given "new" UMA client id we see is a result of
migration.

https://codereview.chromium.org/217083002/diff/40001/chrome/common/pref_names.cc
File chrome/common/pref_names.cc (right):

https://codereview.chromium.org/217083002/diff/40001/chrome/common/pref_names...
chrome/common/pref_names.cc:1377: // values have been cleaned up.
On 2014/03/28 18:23:06, Ilya Sherman wrote:
> nit: Same thing here.

Done.

[Alexei Svitkine (slow), 2014-03-28 18:58:27]

On 2014/03/28 18:25:54, Jesse Doherty wrote:
> LGTM
> Are we going to wait for the clump prevention cl to submit this?

I don't think the two CLs need to block on each other as long as both can make
it in to the same milestone.

[Ilya Sherman, 2014-03-28 19:22:48]

LGTM

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
File chrome/browser/metrics/metrics_service.cc (right):

https://codereview.chromium.org/217083002/diff/40001/chrome/browser/metrics/m...
chrome/browser/metrics/metrics_service.cc:653:
UMA_HISTOGRAM_BOOLEAN("UMA.MigratedClientID", true);
On 2014/03/28 18:40:19, Alexei Svitkine wrote:
> On 2014/03/28 18:23:06, Ilya Sherman wrote:
> > You don't seem to ever emit a |false| value to this histogram.  What's the
> > purpose of tracking this as a histogram?
> 
> This will allow analysis of when users migrated their ids. (We don't care
about
> the actual counts of this, since all UMA users on M35 will be affected by
this.)
> But we may want to know when a given "new" UMA client id we see is a result of
> migration.

Fair 'nuff.

[Alexei Svitkine (slow), 2014-03-28 19:29:56]

+jhawkins for chrome/ OWNERS
thestig to cc

[James Hawkins, 2014-03-28 19:31:55]

lgtm

[Alexei Svitkine (slow), 2014-03-28 19:33:19]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2014-03-28 19:34:26]

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

[Alexei Svitkine (slow), 2014-03-28 19:59:08]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2014-03-28 21:12:16]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[commit-bot: I haz the power, 2014-03-28 21:12:20]

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

[commit-bot: I haz the power, 2014-03-28 23:44:55]

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

[commit-bot: I haz the power, 2014-03-28 23:44:56]

Commit queue rejected this change because the description was changed
between the time the change entered the commit queue and the time it
was ready to commit. You can safely check the commit box again.

[Ilya Sherman, 2014-03-28 23:56:01]

The CQ bit was checked by isherman@chromium.org

[commit-bot: I haz the power, 2014-03-29 00:01:29]

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

[commit-bot: I haz the power, 2014-03-29 01:43:39]

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

[commit-bot: I haz the power, 2014-03-29 01:43:40]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_rel

[Alexei Svitkine (slow), 2014-03-29 02:07:39]

On 2014/03/29 01:43:40, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on linux_chromium_chromeos_rel

Looks like a flake (MultiProfileFileManagerBrowserTest.* failed both with patch
and without).

Setting NOTRY=true. :\

[Alexei Svitkine (slow), 2014-03-29 02:08:11]

The CQ bit was checked by asvitkine@chromium.org

[commit-bot: I haz the power, 2014-03-29 02:08:13]

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

[Lei Zhang, 2014-03-29 02:09:23]

Yes, known issue on IRC...

On Fri, Mar 28, 2014 at 7:07 PM,  <asvitkine@chromium.org> wrote:
> On 2014/03/29 01:43:40, I haz the power (commit-bot) wrote:
>>
>> Try jobs failed on following builders:
>>    tryserver.chromium on linux_chromium_chromeos_rel
>
>
> Looks like a flake (MultiProfileFileManagerBrowserTest.* failed both with
> patch
> and without).
>
> Setting NOTRY=true. :\
>
> https://codereview.chromium.org/217083002/

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[commit-bot: I haz the power, 2014-03-29 03:08:40]

Change committed as 260349