diff options
Diffstat (limited to 'chrome/browser')
-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 |
6 files changed, 103 insertions, 4 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); +} |