Reset low entropy sources after toggling UMA enable state

Currently, we only reset the client id in the local pref when
toggling UMA enable state. We should also reset all low entropy sources
as well to be more consistent.

When a user (re)enables UMA, we immediately generate a new client id,
and start including it in UMA reports. However, we don't have to
generate a new low entropy source immediately, because we can't change
the trial assignments for the current session and we will set
`client_id_was_used_for_trial_assignment` to false on these records
so they can be excluded from most analysis. Instead, the low entropy
sources are removed from local state, which will cause new ones to be
generated and used in the next session.

Bug: 1053529
Change-Id: Ie1437e5799fea8f854e9a1bbcfbefee7652c9847
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355396
Commit-Queue: Weilun Shi <sweilun@chromium.org>
Reviewed-by: Steven Holte <holte@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Auto-Submit: Weilun Shi <sweilun@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799263}

chrome/browser/metrics/metrics_reporting_state.cc

@@ -13,6 +13,7 @@
 #include "chrome/common/pref_names.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "components/crash/core/common/crash_keys.h"
+#include "components/metrics/entropy_state.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/metrics_services_manager/metrics_services_manager.h"
@@ -110,11 +111,12 @@ void UpdateMetricsPrefsOnPermissionChange(bool metrics_enabled) {
     // before a user opts in and all reported data is accurate.
     g_browser_process->metrics_service()->ClearSavedStabilityMetrics();
   } else {
-    // Clear the client id pref when opting out.
-    // Note: Clearing client id will not affect the running state (e.g. field
-    // trial randomization), as the pref is only read on startup.
+    // Clear the client id and low entropy sources pref when opting out.
+    // Note: This will not affect the running state (e.g. field trial
+    // randomization), as the pref is only read on startup.
     g_browser_process->local_state()->ClearPref(
         metrics::prefs::kMetricsClientID);
+    metrics::EntropyState::ClearPrefs(g_browser_process->local_state());
     g_browser_process->local_state()->ClearPref(
         metrics::prefs::kMetricsReportingEnabledTimestamp);
     crash_keys::ClearMetricsClientId();

components/metrics/entropy_state.cc

@@ -41,10 +41,10 @@ EntropyState::EntropyState(PrefService* local_state)
 // static
 constexpr int EntropyState::kLowEntropySourceNotSet;
 
-void EntropyState::ClearPrefs() {
-  DCHECK_EQ(kLowEntropySourceNotSet, low_entropy_source_);
-  local_state_->ClearPref(prefs::kMetricsLowEntropySource);
-  local_state_->ClearPref(prefs::kMetricsOldLowEntropySource);
+// static
+void EntropyState::ClearPrefs(PrefService* local_state) {
+  local_state->ClearPref(prefs::kMetricsLowEntropySource);
+  local_state->ClearPref(prefs::kMetricsOldLowEntropySource);
 }
 
 // static

components/metrics/entropy_state.h

@@ -25,7 +25,7 @@ class EntropyState final {
   EntropyState& operator=(const EntropyState&) = delete;
 
   // Clears low_entropy_source and old_low_entropy_source in the prefs.
-  void ClearPrefs();
+  static void ClearPrefs(PrefService* local_state);
 
   // Registers low_entropy_source and old_low_entropy_source in the prefs.
   static void RegisterPrefs(PrefRegistrySimple* registry);
@@ -46,7 +46,7 @@ class EntropyState final {
   int GetOldLowEntropySource();
 
  private:
-  FRIEND_TEST_ALL_PREFIXES(EntropyStateTest, LowEntropySource0NotReset);
+  FRIEND_TEST_ALL_PREFIXES(EntropyStateTest, LowEntropySourceNotReset);
   FRIEND_TEST_ALL_PREFIXES(EntropyStateTest, HaveNoLowEntropySource);
   FRIEND_TEST_ALL_PREFIXES(EntropyStateTest, HaveOnlyNewLowEntropySource);
   FRIEND_TEST_ALL_PREFIXES(EntropyStateTest, HaveOnlyOldLowEntropySource);

components/metrics/entropy_state_unittest.cc

@@ -26,7 +26,7 @@ class EntropyStateTest : public testing::Test {
   TestingPrefServiceSimple prefs_;
 };
 
-TEST_F(EntropyStateTest, LowEntropySource0NotReset) {
+TEST_F(EntropyStateTest, LowEntropySourceNotReset) {
   EntropyState entropy_state(&prefs_);
   // Get the low entropy source once, to initialize it.
   entropy_state.GetLowEntropySource();
@@ -145,4 +145,14 @@ TEST_F(EntropyStateTest, CorruptOldLowEntropySources) {
   }
 }
 
+TEST_F(EntropyStateTest, ClearPrefs) {
+  prefs_.SetInteger(prefs::kMetricsLowEntropySource, 1234);
+  prefs_.SetInteger(prefs::kMetricsOldLowEntropySource, 5678);
+
+  EntropyState::ClearPrefs(&prefs_);
+
+  EXPECT_FALSE(prefs_.HasPrefPath(prefs::kMetricsLowEntropySource));
+  EXPECT_FALSE(prefs_.HasPrefPath(prefs::kMetricsOldLowEntropySource));
+}
+
 }  // namespace metrics

components/metrics/metrics_state_manager.cc

@@ -404,7 +404,7 @@ void MetricsStateManager::ResetMetricsIDsIfNecessary() {
   DCHECK(client_id_.empty());
 
   local_state_->ClearPref(prefs::kMetricsClientID);
-  entropy_state_.ClearPrefs();
+  EntropyState::ClearPrefs(local_state_);
 
   // Also clear the backed up client info.
   store_client_info_.Run(ClientInfo());