summaryrefslogtreecommitdiffstats
path: root/chrome/browser
diff options
context:
space:
mode:
Diffstat (limited to 'chrome/browser')
-rw-r--r--chrome/browser/metrics/cloned_install_detector.cc10
-rw-r--r--chrome/browser/metrics/cloned_install_detector.h1
-rw-r--r--chrome/browser/metrics/cloned_install_detector_unittest.cc19
-rw-r--r--chrome/browser/metrics/metrics_service.cc29
-rw-r--r--chrome/browser/metrics/metrics_service.h9
-rw-r--r--chrome/browser/metrics/metrics_service_unittest.cc39
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);
+}