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

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/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/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 {
 
+enum MetricsReportingChangeHistogramValue {
+  METRICS_REPORTING_ERROR,
+  METRICS_REPORTING_DISABLED,
+  METRICS_REPORTING_ENABLED,
+  METRICS_REPORTING_MAX
+};
+
+void RecordMetricsReportingHistogramValue(
+    MetricsReportingChangeHistogramValue value) {
+  UMA_HISTOGRAM_ENUMERATION(
+      "UMA.MetricsReporting.Toggle", value, METRICS_REPORTING_MAX);
+}
+
+// Tries to set crash stats upload consent to |enabled|.
+bool SetGoogleUpdateSettings(bool enabled) {
   GoogleUpdateSettings::SetCollectStatsConsent(enabled);
-  bool update_pref = GoogleUpdateSettings::GetCollectStatsConsent();
+  bool updated_pref = GoogleUpdateSettings::GetCollectStatsConsent();
+  if (enabled != updated_pref)
+    DVLOG(1) << "Unable to set metrics reporting status to " << enabled;
 
-  if (enabled != update_pref)
-    DVLOG(1) << "Unable to set crash report status to " << enabled;
+  return updated_pref;
+}
 
+// Does the necessary changes for MetricsReportingEnabled changes which needs
+// to be done in the main thread.
+void SetMetricsReporting(bool enabled,
+                         const base::Callback<void(bool)> callback_fn,
+                         bool updated_pref) {

[Alexei Svitkine (slow), 2014/09/09 20:30:04]

Do we need both updated_pref and enabled params?

I think we should just use the last param and remove the first one.

[gayane -on leave until 09-2017, 2014/09/10 16:58:27]

We need it for understanding the success of the change to record to histogram.
Other option would be to record the error in the previous function and here
record whatever is the new value.

   // Only change the pref if GoogleUpdateSettings::GetCollectStatsConsent
   // succeeds.
-  enabled = update_pref;
-
   metrics::MetricsService* metrics = g_browser_process->metrics_service();
   if (metrics) {
-    if (enabled)
+    if (updated_pref)
       metrics->Start();
     else
       metrics->Stop();
   }
+#if !defined(OS_CHROMEOS)
+  g_browser_process->local_state()->SetBoolean(
+      prefs::kMetricsReportingEnabled, enabled);
+#endif
+  if (enabled == updated_pref) {
+    RecordMetricsReportingHistogramValue(enabled ? METRICS_REPORTING_ENABLED :
+        METRICS_REPORTING_DISABLED);
+  } else {
+    RecordMetricsReportingHistogramValue(METRICS_REPORTING_ERROR);
+  }
+  if (!callback_fn.is_null())
+    callback_fn.Run(updated_pref);
+}
 
-  return enabled;
+} // namespace
+
+void InitiateMetricsReportingChange(
+    bool enabled,
+    const base::Callback<void(bool)> callback_fn) {
+  if (!IsMetricsReportingUserChangable()) {
+    if (!callback_fn.is_null())
+      callback_fn.Run(false);

[Alexei Svitkine (slow), 2014/09/09 20:30:05]

This isn't correct anymore since the value is now the uma_enabled state, not
success. Please fix.

[gayane -on leave until 09-2017, 2014/09/10 16:58:27]

Done.

+    return;
+  }
+
+  // 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));
 }
+
+bool IsMetricsReportingUserChangable() {
+  const PrefService* pref_service = g_browser_process->local_state();
+
+  const PrefService::Preference* pref =
+      pref_service->FindPreference(prefs::kMetricsReportingEnabled);
+  return pref && pref->IsManaged();
+}