Remove OnRecordingDisabled() metrics client interface.

chrome/browser/metrics/metrics_reporting_state.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/metrics/metrics_reporting_state.h"
 
 #include "base/callback.h"
 #include "base/metrics/histogram.h"
 #include "build/build_config.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/common/crash_keys.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/metrics_services_manager/metrics_services_manager.h"
 #include "components/prefs/pref_registry_simple.h"
 #include "components/prefs/pref_service.h"
 #include "content/public/browser/browser_thread.h"
 
 #if defined(OS_CHROMEOS)
@@ skipping 31 matching lines @@
 // to be done in the main thread.
 // As arguments this function gets:
 //  |to_update_pref| which indicates what the desired update should be,
 //  |callback_fn| is the callback function to be called in the end
 //  |updated_pref| is the result of attempted update.
 // Update considers to be successful if |to_update_pref| and |updated_pref| are
 // the same.
 void SetMetricsReporting(bool to_update_pref,
                          const OnMetricsReportingCallbackType& callback_fn,
                          bool updated_pref) {
-  metrics::MetricsService* metrics = g_browser_process->metrics_service();
-
 #if !defined(OS_ANDROID)
   g_browser_process->local_state()->SetBoolean(
       metrics::prefs::kMetricsReportingEnabled, updated_pref);
 #endif  // !defined(OS_ANDROID)
 
-  // Clear the client id pref when opting out. Note: Mirrors code in
-  // uma_session_stats.cc. TODO(asvitkine): Unify.
-  if (!updated_pref) {
-    // Note: Clearing client id 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);
-    g_browser_process->local_state()->ClearPref(
-        metrics::prefs::kMetricsReportingEnabledTimestamp);
-  }
+  UpdateMetricsPrefsOnPermissionChange(updated_pref);
 
   // Uses the current state of whether reporting is enabled to enable services.
   g_browser_process->GetMetricsServicesManager()->UpdateUploadPermissions(true);
 
-  // When a user opts in to the metrics reporting service, the previously
-  // collected data should be cleared to ensure that nothing is reported before
-  // a user opts in and all reported data is accurate.
-  // TODO(asvitkine): This logic should be added to uma_session_stats.cc too.
-  if (updated_pref && metrics)
-    metrics->ClearSavedStabilityMetrics();
-
   if (to_update_pref == updated_pref) {
     RecordMetricsReportingHistogramValue(updated_pref ?
         METRICS_REPORTING_ENABLED : METRICS_REPORTING_DISABLED);
   } else {
     RecordMetricsReportingHistogramValue(METRICS_REPORTING_ERROR);
   }
   if (!callback_fn.is_null())
     callback_fn.Run(updated_pref);
 }
 
@@ skipping 30 matching lines @@
   }
 #endif
   // Posts to FILE thread as SetGoogleUpdateSettings does IO operations.
   content::BrowserThread::PostTaskAndReplyWithResult(
       content::BrowserThread::FILE,
       FROM_HERE,
       base::Bind(&SetGoogleUpdateSettings, enabled),
       base::Bind(&SetMetricsReporting, enabled, callback_fn));
 }
 
+void UpdateMetricsPrefsOnPermissionChange(bool metrics_enabled) {

[jwd, 2016/08/16 20:30:04]

Didn't think to ask this in the last review, but this seems like this might
belong in the MetricsStateManager, at least the pref related stuff. Can anything
be moved there?

[Alexei Svitkine (slow), 2016/08/16 21:07:50]

It's definitely a good question. There's a couple of issues - one is that it's
hard to get a pointer the the MetricsStateManager from the two callers and
MetricsStateManager is inside the component which currently can't depend on
crash_keys to clear the crash id.

+  if (metrics_enabled) {
+    // When a user opts in to the metrics reporting service, the previously
+    // collected data should be cleared to ensure that nothing is reported
+    // 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.
+    g_browser_process->local_state()->ClearPref(
+        metrics::prefs::kMetricsClientID);
+    g_browser_process->local_state()->ClearPref(
+        metrics::prefs::kMetricsReportingEnabledTimestamp);
+    crash_keys::ClearMetricsClientId();
+  }
+}
+
 bool IsMetricsReportingPolicyManaged() {
   const PrefService* pref_service = g_browser_process->local_state();
   const PrefService::Preference* pref =
       pref_service->FindPreference(metrics::prefs::kMetricsReportingEnabled);
   return pref && pref->IsManaged();
 }
 
 // TODO(gayane): Add unittest which will check that observer on device settings
 // will trigger this function and kMetricsReportinEnabled as well as metrics
 // service state will be updated accordingly.
 void SetupMetricsStateForChromeOS() {
 #if defined(OS_CHROMEOS)
   chromeos::CrosSettings::Get()->AddSettingsObserver(
       chromeos::kStatsReportingPref, base::Bind(&OnDeviceSettingChange));
 
   OnDeviceSettingChange();
 #endif // defined(OS_CHROMEOS)
 }