Prevent GetDeviceId from invoking its callback multiple times in failure cases.

chrome/browser/prefs/pref_metrics_service.cc

Index: chrome/browser/prefs/pref_metrics_service.cc
diff --git a/chrome/browser/prefs/pref_metrics_service.cc b/chrome/browser/prefs/pref_metrics_service.cc
index 47eca5b09ca9dfbf7903ebe9bf0f4d93944374d7..38159199513d45e215d957828d706a282120f5af 100644
--- a/chrome/browser/prefs/pref_metrics_service.cc
+++ b/chrome/browser/prefs/pref_metrics_service.cc
@@ -80,7 +80,6 @@ PrefMetricsService::PrefMetricsService(Profile* profile)
       profile_name_(profile_->GetPath().AsUTF8Unsafe()),
       tracked_pref_paths_(kTrackedPrefs),
       tracked_pref_path_count_(arraysize(kTrackedPrefs)),
-      checked_tracked_prefs_(false),
       weak_factory_(this) {
   pref_hash_seed_ = ResourceBundle::GetSharedInstance().GetRawDataResource(
       IDR_PREF_HASH_SEED_BIN).as_string();
@@ -118,7 +117,6 @@ PrefMetricsService::PrefMetricsService(Profile* profile,
       device_id_(device_id),
       tracked_pref_paths_(tracked_pref_paths),
       tracked_pref_path_count_(tracked_pref_path_count),
-      checked_tracked_prefs_(false),
       weak_factory_(this) {
   CheckTrackedPreferences();
 }
@@ -260,10 +258,23 @@ void PrefMetricsService::LogIntegerPrefChange(int boundary_value,
 }
 
 void PrefMetricsService::GetDeviceIdCallback(const std::string& device_id) {
+#if defined(ENABLE_DCHECK)
+  // Make sure this callback is only called once.
+  static bool called = false;

[Jeffrey Yasskin, 2013/11/21 22:23:22]

This is dangerous because it's process-wide. How sure are you that the user will
never create a PrefMetricsService for more than one profile in the same process?

[gab, 2013/11/22 01:52:13]

Oops, indeed, this is currently only called for the first profile, but we need
to change this soon, so I'll do it correctly, thanks!

+  DCHECK(!called);
+#endif
+
+#if !defined(OS_WIN) || defined(ENABLE_RLZ)
+  // Always expect a non-empty device ID, except on Windows if RLZ is disabled.

[Jeffrey Yasskin, 2013/11/21 22:23:22]

This has too many negatives. Can you make it easier to understand?

[gab, 2013/11/22 01:52:13]

Done.

+  DCHECK(!device_id.empty());
+#endif
+
   device_id_ = device_id;
-  // On Aura, this seems to be called twice.
-  if (!checked_tracked_prefs_)
-    CheckTrackedPreferences();
+  CheckTrackedPreferences();
+
+#if defined(ENABLE_DCHECK)
+  called = true;

[Jeffrey Yasskin, 2013/11/21 22:23:22]

Move this just after the DCHECK(!called) to reduce the number of #ifs.

[gab, 2013/11/22 01:52:13]

Done.

+#endif
 }
 
 // To detect changes to Preferences that happen outside of Chrome, we hash
@@ -273,8 +284,6 @@ void PrefMetricsService::GetDeviceIdCallback(const std::string& device_id) {
 // profile. To make the system more resistant to spoofing, pref values are
 // hashed with the pref path and the device id.
 void PrefMetricsService::CheckTrackedPreferences() {
-  DCHECK(!checked_tracked_prefs_);
-
   const base::DictionaryValue* pref_hash_dicts =
       local_state_->GetDictionary(prefs::kProfilePreferenceHashes);
   // Get the hashed prefs dictionary if it exists. If it doesn't, it will be
@@ -337,8 +346,6 @@ void PrefMetricsService::CheckTrackedPreferences() {
     }
   }
 
-  checked_tracked_prefs_ = true;
-
   // Now that we've checked the incoming preferences, register for change
   // notifications, unless this is test code.
   // TODO(bbudge) Fix failing browser_tests and so we can remove this test.