diff options
-rw-r--r-- | chrome/browser/metrics/cloned_install_detector.cc | 10 | ||||
-rw-r--r-- | chrome/browser/metrics/cloned_install_detector.h | 1 | ||||
-rw-r--r-- | chrome/browser/metrics/cloned_install_detector_unittest.cc | 19 | ||||
-rw-r--r-- | chrome/browser/metrics/metrics_service.cc | 29 | ||||
-rw-r--r-- | chrome/browser/metrics/metrics_service.h | 9 | ||||
-rw-r--r-- | chrome/browser/metrics/metrics_service_unittest.cc | 39 | ||||
-rw-r--r-- | chrome/common/pref_names.cc | 5 | ||||
-rw-r--r-- | chrome/common/pref_names.h | 1 | ||||
-rw-r--r-- | tools/metrics/histograms/histograms.xml | 15 |
9 files changed, 121 insertions, 7 deletions
diff --git a/chrome/browser/metrics/cloned_install_detector.cc b/chrome/browser/metrics/cloned_install_detector.cc index 3ecfd2c..db52b84 100644 --- a/chrome/browser/metrics/cloned_install_detector.cc +++ b/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); diff --git a/chrome/browser/metrics/cloned_install_detector.h b/chrome/browser/metrics/cloned_install_detector.h index 19bc658..ced164d 100644 --- a/chrome/browser/metrics/cloned_install_detector.h +++ b/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 diff --git a/chrome/browser/metrics/cloned_install_detector_unittest.cc b/chrome/browser/metrics/cloned_install_detector_unittest.cc index a50cbbf..dcc8bf6 100644 --- a/chrome/browser/metrics/cloned_install_detector_unittest.cc +++ b/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 diff --git a/chrome/browser/metrics/metrics_service.cc b/chrome/browser/metrics/metrics_service.cc index a313c45..7155c40 100644 --- a/chrome/browser/metrics/metrics_service.cc +++ b/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. diff --git a/chrome/browser/metrics/metrics_service.h b/chrome/browser/metrics/metrics_service.h index f50934d..9ee89e2 100644 --- a/chrome/browser/metrics/metrics_service.h +++ b/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, diff --git a/chrome/browser/metrics/metrics_service_unittest.cc b/chrome/browser/metrics/metrics_service_unittest.cc index 3ca255b..ae110db 100644 --- a/chrome/browser/metrics/metrics_service_unittest.cc +++ b/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); +} diff --git a/chrome/common/pref_names.cc b/chrome/common/pref_names.cc index 610d5d0..678ee6d 100644 --- a/chrome/common/pref_names.cc +++ b/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) diff --git a/chrome/common/pref_names.h b/chrome/common/pref_names.h index aad3239..d79f0a5 100644 --- a/chrome/common/pref_names.h +++ b/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. diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 90a30e2..bad41e4 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/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 |