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/browser/chromeos/policy/browser_policy_connector_chromeos.h"
+#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
+#include "chrome/common/pref_names.h"
 #include "chrome/installer/util/google_update_settings.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 metrics reporting status to |enabled| and returns whatever is
+// the result of the update.
+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;
+}
 
-  // Only change the pref if GoogleUpdateSettings::GetCollectStatsConsent
-  // succeeds.
-  enabled = update_pref;
-
+// Does the necessary changes for MetricsReportingEnabled changes which needs
+// 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 (metrics) {
-    if (enabled)
+    if (updated_pref)
       metrics->Start();
     else
       metrics->Stop();
   }
+#if !defined(OS_CHROMEOS)

[Alexei Svitkine (slow), 2014/09/15 21:36:44]

I would also expand this ifdef to be !ANDROID too.

[gayane -on leave until 09-2017, 2014/09/16 17:28:52]

Done.

+  g_browser_process->local_state()->SetBoolean(
+      prefs::kMetricsReportingEnabled, updated_pref);
+#endif
+  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);
+}
 
-  return enabled;
+} // namespace
+
+void InitiateMetricsReportingChange(
+    bool enabled,
+    const OnMetricsReportingCallbackType& callback_fn) {
+  if (!IsMetricsReportingUserChangable()) {
+    if (!callback_fn.is_null()) {
+      callback_fn.Run(
+          ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
+    }
+    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() {
+#if defined(GOOGLE_CHROME_BUILD)
+#if defined(OS_CHROMEOS)
+  policy::BrowserPolicyConnectorChromeOS* connector =
+        g_browser_process->platform_part()->browser_policy_connector_chromeos();
+  return connector->IsEnterpriseManaged();
+#else
+  const PrefService* pref_service = g_browser_process->local_state();
+#if defined(OS_ANDROID)

[Alexei Svitkine (slow), 2014/09/15 20:59:43]

Why is all of this needed?

I thought this function only gets called on non-CrOS non-Android platforms
currently (and your comment for this function mentions that it shouldn't be used
on those).

Is that not the case? (If so, can you clarify when it gets called on those other
platforms).

[gayane -on leave until 09-2017, 2014/09/15 21:09:17]

So the thing is that initially the plan was to use this function for Checkbox
setup for non-Chrome non-Andriod, but then we added a check in
InitiateMetricsReportingChange. This function can be called and is called from
ChromeOS right now. I am not sure about Andrioid. I added it just in case.

Changed the comment for this function.

[Alexei Svitkine (slow), 2014/09/15 21:36:44]

I see. But InitiateMetricsReportingChange() doesn't touch the pref on those
platforms yet. So I suggest just ifdefing the code in
InitiateMetricsReportingChange() to correspond to the ifdef around setting
prefs::kMetricsReportingEnabled and revert this function back to the simpler
version.

(In future CLs when we make this code support those platforms completely, we can
expand this - but I'm hoping that can be done with fewer ifdefs.)

[gayane -on leave until 09-2017, 2014/09/16 17:28:52]

Done.

+  const PrefService::Preference* pref =
+      pref_service->FindPreference(prefs::kCrashReportingEnabled);
+#else
+  const PrefService::Preference* pref =
+      pref_service->FindPreference(prefs::kMetricsReportingEnabled);
+#endif
+  return pref && !pref->IsManaged();
+#endif
+#else
+  return true;
+#endif
+
+}