Resetting metrics ids on clump detection.

When a cloned install is detected the metrics client id and low entropy source are reset on the next start of Chrome. This is done by setting a pref in local state. The next time either the client id or low entropy source are loaded from the local state, they are first reset.

BUG=343273
NOTRY=true

Review URL: https://codereview.chromium.org/213363004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@260348 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/metrics/cloned_install_detector.cc

@@ -72,8 +72,14 @@ void ClonedInstallDetector::SaveMachineId(
 
   MachineIdState id_state = ID_NO_STORED_VALUE;
   if (local_state->HasPrefPath(prefs::kMetricsMachineId)) {
-    id_state = local_state->GetInteger(prefs::kMetricsMachineId) == hashed_id ?
-        ID_UNCHANGED : ID_CHANGED;
+    if (local_state->GetInteger(prefs::kMetricsMachineId) != hashed_id) {
+      id_state = ID_CHANGED;
+      // TODO(jwd): Use a callback to set the reset pref. That way
+      // ClonedInstallDetector doesn't need to know about this pref.
+      local_state->SetBoolean(prefs::kMetricsResetIds, true);
+    } else {
+      id_state = ID_UNCHANGED;
+    }
   }
 
   LogMachineIdState(id_state);

chrome/browser/metrics/cloned_install_detector.h

@@ -34,6 +34,7 @@ class ClonedInstallDetector {
 
  private:
   FRIEND_TEST_ALL_PREFIXES(ClonedInstallDetectorTest, SaveId);
+  FRIEND_TEST_ALL_PREFIXES(ClonedInstallDetectorTest, DetectClone);
 
   // Converts raw_id into a 24-bit hash and stores the hash in |local_state|.
   // |raw_id| is not a const ref because it's passed from a cross-thread post

chrome/browser/metrics/cloned_install_detector_unittest.cc

@@ -6,6 +6,7 @@
 
 #include "base/prefs/testing_pref_service.h"
 #include "chrome/browser/metrics/machine_id_provider.h"
+#include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/common/pref_names.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -19,7 +20,7 @@ const int kTestHashedId = 2216819;
 
 }  // namespace
 
-// TODO(jwd): Change this test to test the full flow and histogram outputs. It
+// TODO(jwd): Change these test to test the full flow and histogram outputs. It
 // should also remove the need to make the test a friend of
 // ClonedInstallDetector.
 TEST(ClonedInstallDetectorTest, SaveId) {
@@ -34,4 +35,20 @@ TEST(ClonedInstallDetectorTest, SaveId) {
   EXPECT_EQ(kTestHashedId, prefs.GetInteger(prefs::kMetricsMachineId));
 }
 
+TEST(ClonedInstallDetectorTest, DetectClone) {
+  TestingPrefServiceSimple prefs;
+  ClonedInstallDetector::RegisterPrefs(prefs.registry());
+  MetricsService::RegisterPrefs(prefs.registry());
+
+  // Save a machine id that will cause a clone to be detected.
+  prefs.SetInteger(prefs::kMetricsMachineId, kTestHashedId + 1);
+
+  scoped_ptr<ClonedInstallDetector> detector(
+      new ClonedInstallDetector(MachineIdProvider::CreateInstance()));
+
+  detector->SaveMachineId(&prefs, kTestRawId);
+
+  EXPECT_TRUE(prefs.GetBoolean(prefs::kMetricsResetIds));
+}
+
 }  // namespace metrics

chrome/browser/metrics/metrics_service.cc

@@ -435,6 +435,7 @@ class MetricsMemoryDetails : public MemoryDetails {
 // static
 void MetricsService::RegisterPrefs(PrefRegistrySimple* registry) {
   DCHECK(IsSingleThreaded());
+  registry->RegisterBooleanPref(prefs::kMetricsResetIds, false);
   registry->RegisterStringPref(prefs::kMetricsClientID, std::string());
   registry->RegisterIntegerPref(prefs::kMetricsLowEntropySource,
                                 kLowEntropySourceNotSet);
@@ -521,7 +522,8 @@ void MetricsService::DiscardOldStabilityStats(PrefService* local_state) {
 }
 
 MetricsService::MetricsService()
-    : recording_active_(false),
+    : metrics_ids_reset_check_performed_(false),
+      recording_active_(false),
       reporting_active_(false),
       test_mode_active_(false),
       state_(INITIALIZED),
@@ -633,6 +635,9 @@ MetricsService::CreateEntropyProvider(ReportingState reporting_state) {
 void MetricsService::ForceClientIdCreation() {
   if (!client_id_.empty())
     return;
+
+  ResetMetricsIDsIfNecessary();
+
   PrefService* pref = g_browser_process->local_state();
   client_id_ = pref->GetString(prefs::kMetricsClientID);
   if (!client_id_.empty())
@@ -1185,6 +1190,26 @@ void MetricsService::GetUptimes(PrefService* pref,
   }
 }
 
+void MetricsService::ResetMetricsIDsIfNecessary() {
+  if (metrics_ids_reset_check_performed_)
+    return;
+
+  metrics_ids_reset_check_performed_ = true;
+
+  PrefService* local_state = g_browser_process->local_state();
+  if (!local_state->GetBoolean(prefs::kMetricsResetIds))
+    return;
+
+  UMA_HISTOGRAM_BOOLEAN("UMA.MetricsIDsReset", true);
+
+  DCHECK(client_id_.empty());
+  DCHECK_EQ(kLowEntropySourceNotSet, low_entropy_source_);
+
+  local_state->ClearPref(prefs::kMetricsClientID);
+  local_state->ClearPref(prefs::kMetricsLowEntropySource);
+  local_state->ClearPref(prefs::kMetricsResetIds);
+}
+
 int MetricsService::GetLowEntropySource() {
   // Note that the default value for the low entropy source and the default pref
   // value are both kLowEntropySourceNotSet, which is used to identify if the
@@ -1192,6 +1217,8 @@ int MetricsService::GetLowEntropySource() {
   if (low_entropy_source_ != kLowEntropySourceNotSet)
     return low_entropy_source_;
 
+  ResetMetricsIDsIfNecessary();
+
   PrefService* local_state = g_browser_process->local_state();
   const CommandLine* command_line(CommandLine::ForCurrentProcess());
   // Only try to load the value from prefs if the user did not request a reset.

chrome/browser/metrics/metrics_service.h

@@ -368,6 +368,10 @@ class MetricsService
                   base::TimeDelta* incremental_uptime,
                   base::TimeDelta* uptime);
 
+  // Reset the client id and low entropy source if the kMetricsResetMetricIDs
+  // pref is true.
+  void ResetMetricsIDsIfNecessary();
+
   // Returns the low entropy source for this client. This is a random value
   // that is non-identifying amongst browser clients. This method will
   // generate the entropy source value if it has not been called before.
@@ -523,6 +527,10 @@ class MetricsService
 
   content::NotificationRegistrar registrar_;
 
+  // Set to true when |ResetMetricsIDsIfNecessary| is called for the first time.
+  // This prevents multiple resets within the same Chrome session.
+  bool metrics_ids_reset_check_performed_;
+
   // Indicate whether recording and reporting are currently happening.
   // These should not be set directly, but by calling SetRecording and
   // SetReporting.
@@ -639,6 +647,7 @@ class MetricsService
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest,
                            PermutedEntropyCacheClearedWhenLowEntropyReset);
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, RegisterSyntheticTrial);
+  FRIEND_TEST_ALL_PREFIXES(MetricsServiceTest, ResetMetricsIDs);
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceBrowserTest,
                            CheckLowEntropySourceUsed);
   FRIEND_TEST_ALL_PREFIXES(MetricsServiceReportingTest,

chrome/browser/metrics/metrics_service_unittest.cc

@@ -374,3 +374,42 @@ TEST_F(MetricsServiceTest, CrashReportingEnabled) {
   EXPECT_FALSE(MetricsServiceHelper::IsCrashReportingEnabled());
 #endif // defined(GOOGLE_CHROME_BUILD)
 }
+
+// Check that setting the kMetricsResetIds pref to true causes the client id to
+// be reset. We do not check that the low entropy source is reset because we
+// cannot ensure that metrics service won't generate the same id again.
+TEST_F(MetricsServiceTest, ResetMetricsIDs) {
+  // Set an initial client id in prefs. It should not be possible for the
+  // metrics service to generate this id randomly.
+  const std::string kInitialClientId = "initial client id";
+  GetLocalState()->SetString(prefs::kMetricsClientID, kInitialClientId);
+
+  // Make sure the initial client id isn't reset by the metrics service.
+  {
+    MetricsService service;
+    service.ForceClientIdCreation();
+    EXPECT_TRUE(service.metrics_ids_reset_check_performed_);
+    EXPECT_EQ(kInitialClientId, service.client_id_);
+  }
+
+
+  // Set the reset pref to cause the IDs to be reset.
+  GetLocalState()->SetBoolean(prefs::kMetricsResetIds, true);
+
+  // Cause the actual reset to happen.
+  {
+    MetricsService service;
+    service.ForceClientIdCreation();
+    EXPECT_TRUE(service.metrics_ids_reset_check_performed_);
+    EXPECT_NE(kInitialClientId, service.client_id_);
+
+    service.GetLowEntropySource();
+
+    EXPECT_FALSE(GetLocalState()->GetBoolean(prefs::kMetricsResetIds));
+  }
+
+  std::string new_client_id =
+      GetLocalState()->GetString(prefs::kMetricsClientID);
+
+  EXPECT_NE(kInitialClientId, new_client_id);
+}

chrome/common/pref_names.cc

@@ -1383,6 +1383,11 @@ const char kMetricsReportingEnabled[] =
 // stored locally and never transmitted in metrics reports.
 const char kMetricsMachineId[] = "user_experience_metrics.machine_id";
 
+// Boolean that indicates a cloned install has been detected and the metrics
+// client id and low entropy source should be reset.
+const char kMetricsResetIds[] =
+    "user_experience_metrics.reset_metrics_ids";
+
 // Boolean that specifies whether or not crash reports are sent
 // over the network for analysis.
 #if defined(OS_ANDROID)

chrome/common/pref_names.h

@@ -438,6 +438,7 @@ extern const char kMetricsPermutedEntropyCache[];
 extern const char kMetricsClientIDTimestamp[];
 extern const char kMetricsReportingEnabled[];
 extern const char kMetricsMachineId[];
+extern const char kMetricsResetIds[];
 // Android has it's own metric / crash reporting implemented in Android
 // Java code so kMetricsReportingEnabled doesn't make sense. We use this
 // to inform crashes_ui that we have enabled crash reporting.

tools/metrics/histograms/histograms.xml

@@ -27796,12 +27796,21 @@ other types of suffix sets.
 </histogram>
 
 <histogram name="UMA.MachineIdState" enum="UmaMachineIdState">
-  <sumary>
+  <owner>jwd@chromium.org</owner>
+  <summary>
     Tracks if the machine ID is generated successfully and if it changes from
     one run to the next. The machine ID is a 24-bit hash of machine
     characteristics. It is expected to change if an install of Chrome is copied
     to multiple machines. This check happens once per browser startup.
-  </sumary>
+  </summary>
+</histogram>
+
+<histogram name="UMA.MetricsIDsReset" enum="BooleanHit">
+  <owner>jwd@chromium.org</owner>
+  <summary>
+    A count of the number of times the metrics ids (client id and low entropy
+    source) have been reset due to a cloned install being detected.
+  </summary>
 </histogram>
 
 <histogram name="UMA.Perf.GetData" enum="GetPerfDataOutcome">
@@ -28137,7 +28146,7 @@ other types of suffix sets.
 </histogram>
 
 <histogram name="Variations.SeedDateChange" enum="VariationsSeedDateChange">
-  <owner>Please list the metric's owners. Add more owner tags as needed.</owner>
+  <owner>jwd@chromium.org</owner>
   <summary>
     Counts if a response from the variations server is the first response of the
     day or not. This is counted when a new valid seed or a 304 is received. The