Consolidates accessing and setting the UMA pref to be within metrics code

chrome/browser/metrics/metrics_reporting_state.cc

Index: chrome/browser/metrics/metrics_reporting_state.cc
diff --git a/chrome/browser/metrics/metrics_reporting_state.cc b/chrome/browser/metrics/metrics_reporting_state.cc
index 6c72ba66de08962bcf8ecef206143fdbd2e00fe4..b386f567a062d06f44a8535209f3c2ca5f7a0bc2 100644
--- a/chrome/browser/metrics/metrics_reporting_state.cc
+++ b/chrome/browser/metrics/metrics_reporting_state.cc
@@ -4,33 +4,89 @@
 
 #include "chrome/browser/metrics/metrics_reporting_state.h"
 
-#include "base/threading/thread_restrictions.h"
+#include "base/callback.h"
+#include "base/metrics/histogram.h"
+#include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/installer/util/google_update_settings.h"
+#include "chromeos/settings/cros_settings_names.h"
 #include "components/metrics/metrics_service.h"
+#include "content/public/browser/browser_thread.h"
 
-bool ResolveMetricsReportingEnabled(bool enabled) {
-  // GoogleUpdateSettings touches the disk from the UI thread. MetricsService
-  // also calls GoogleUpdateSettings below. http://crbug/62626
-  base::ThreadRestrictions::ScopedAllowIO allow_io;
+namespace {
 
-  GoogleUpdateSettings::SetCollectStatsConsent(enabled);
-  bool update_pref = GoogleUpdateSettings::GetCollectStatsConsent();
+enum MetricsReportingChangeHistogramValue {
+  METRICS_REPORTING_ENABLED,
+  METRICS_REPORTING_DISABLED,

[Alexei Svitkine (slow), 2014/09/05 15:22:48]

I'd put this entry first.

I expect over time for us to expand this histogram to break down the different
ways the user can enable UMA, while I expect the disable option will always be
just one, so better keep it first in the list.

[gayane -on leave until 09-2017, 2014/09/05 18:30:20]

Done.

+  METRICS_REPORTING_MAX,
+};
+
+void RecordMetricsReportingHistogramValue(
+    MetricsReportingChangeHistogramValue value) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "MetricsReporting", value, METRICS_REPORTING_MAX);

[Alexei Svitkine (slow), 2014/09/05 15:22:48]

Histograms are generally named of the form Foo.Bar.

In this case, I'd use the prefix "UMA." since its about the core UMA system.

You also need to modify histograms.xml to document the histogram.

[gayane -on leave until 09-2017, 2014/09/05 18:30:20]

Done.

+}
 
-  if (enabled != update_pref)
+// Tries to set crash stats upload consent to |enabled|.
+bool SetGoogleUpdateSettings(bool enabled) {
+  GoogleUpdateSettings::SetCollectStatsConsent(enabled);
+  bool updated_pref = GoogleUpdateSettings::GetCollectStatsConsent();
+  bool success = (enabled == updated_pref);
+  if (!success)
     DVLOG(1) << "Unable to set crash report status to " << enabled;
 
+  return success;
+}
+
+// Does the necessary changes for MetricsReportingEnabled changes which needs
+// to be done in the main thread.
+void SetMetricsReporting(bool enabled,
+                         base::Callback<void(bool)> callback_fn,
+                         bool success) {
   // Only change the pref if GoogleUpdateSettings::GetCollectStatsConsent
   // succeeds.
-  enabled = update_pref;
-
   metrics::MetricsService* metrics = g_browser_process->metrics_service();
-  if (metrics) {
-    if (enabled)
-      metrics->Start();
-    else
-      metrics->Stop();
+  if (success) {
+    if (metrics) {
+      if (enabled)
+        metrics->Start();
+      else
+        metrics->Stop();
+    }
+#if !defined(OS_CHROMEOS)
+    g_browser_process->local_state()->SetBoolean(
+        prefs::kMetricsReportingEnabled, enabled);
+#endif
+    RecordMetricsReportingHistogramValue(enabled ? METRICS_REPORTING_ENABLED :
+        METRICS_REPORTING_DISABLED);
   }
+  if (!callback_fn.is_null())
+    callback_fn.Run(success);
+}
+
+} // namespace
+
+void InitiateMetricsReportingChange(bool enabled,
+                                    base::Callback<void(bool)> callback_fn) {
+  if (!IsMetricsReportingUserChangable()) {
+    if (!callback_fn.is_null()) callback_fn.Run(false);
+    return;
+  }
+
+  // Posts on FILE thread as SetGoogleUpdateSettings modifies the local state.
+  content::BrowserThread::PostTaskAndReplyWithResult(
+      content::BrowserThread::FILE,
+      FROM_HERE,
+      base::Bind(&SetGoogleUpdateSettings, enabled),
+      base::Bind(&SetMetricsReporting, enabled, callback_fn));
+}
+
+bool IsMetricsReportingUserChangable() {
+  const PrefService* pref_service = g_browser_process->local_state();
 
-  return enabled;
+  const PrefService::Preference* pref =
+      pref_service->FindPreference(prefs::kMetricsReportingEnabled);
+  return pref && pref->IsManaged();
 }