Fix ChromeMetricsServiceAccessor::IsMetricsReportingEnabled on CrOs

Since MetricsServicesManager::IsMetricsReportingEnabled is correct, it is
called directly.

BUG=457430

Review URL: https://codereview.chromium.org/916133003

Cr-Commit-Position: refs/heads/master@{#317058}

6 files changed, 38 insertions, 45 deletions

diff --git a/chrome/browser/chrome_browser_main.cc b/chrome/browser/chrome_browser_main.cc
index 5dced63..bcba855 100644
--- a/chrome/browser/chrome_browser_main.cc
+++ b/chrome/browser/chrome_browser_main.cc
@@ -676,11 +676,7 @@ void ChromeBrowserMainParts::StartMetricsRecording() {
   g_browser_process->metrics_service()->CheckForClonedInstall(
       BrowserThread::GetMessageLoopProxyForThread(BrowserThread::FILE));
 
-  bool may_record = g_browser_process->GetMetricsServicesManager()->
-      IsMetricsReportingEnabled();
-
-  g_browser_process->GetMetricsServicesManager()->UpdatePermissions(
-      may_record, true);
+  g_browser_process->GetMetricsServicesManager()->UpdateUploadPermissions(true);
 }
 
 void ChromeBrowserMainParts::RecordBrowserStartupTime() {
diff --git a/chrome/browser/metrics/chrome_metrics_service_accessor.cc b/chrome/browser/metrics/chrome_metrics_service_accessor.cc
index 271af31..546ac0cd 100644
--- a/chrome/browser/metrics/chrome_metrics_service_accessor.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_accessor.cc
@@ -4,8 +4,11 @@
 
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 
+#include "base/command_line.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/metrics/metrics_services_manager.h"
+#include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/variations/metrics_util.h"
@@ -15,28 +18,33 @@
 #endif
 
 // static
+// TODO(asvitkine): This function does not report the correct value on Android,
+// see http://crbug.com/362192.
 bool ChromeMetricsServiceAccessor::IsMetricsReportingEnabled() {
-  bool result = false;
-  const PrefService* local_state = g_browser_process->local_state();
-  if (local_state) {
-    const PrefService::Preference* uma_pref =
-        local_state->FindPreference(prefs::kMetricsReportingEnabled);
-    if (uma_pref) {
-      bool success = uma_pref->GetValue()->GetAsBoolean(&result);
-      DCHECK(success);
-    }
+  // If the user permits metrics reporting with the checkbox in the
+  // prefs, we turn on recording.  We disable metrics completely for
+  // non-official builds, or when field trials are forced.
+  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
+          switches::kForceFieldTrials)) {
+    return false;
   }
-  return result;
-}
 
-bool ChromeMetricsServiceAccessor::IsCrashReportingEnabled() {
+  bool enabled = false;
 #if defined(GOOGLE_CHROME_BUILD)
 #if defined(OS_CHROMEOS)
-  bool reporting_enabled = false;
   chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
-                                            &reporting_enabled);
-  return reporting_enabled;
-#elif defined(OS_ANDROID)
+                                            &enabled);
+#else
+  enabled = g_browser_process->local_state()->
+      GetBoolean(prefs::kMetricsReportingEnabled);
+#endif  // #if defined(OS_CHROMEOS)
+#endif  // defined(GOOGLE_CHROME_BUILD)
+  return enabled;
+}
+
+bool ChromeMetricsServiceAccessor::IsCrashReportingEnabled() {
+#if defined(GOOGLE_CHROME_BUILD)
+#if defined(OS_ANDROID)
   // Android has its own settings for metrics / crash uploading.
   const PrefService* prefs = g_browser_process->local_state();
   return prefs->GetBoolean(prefs::kCrashReportingEnabled);
diff --git a/chrome/browser/metrics/chrome_metrics_service_accessor.h b/chrome/browser/metrics/chrome_metrics_service_accessor.h
index 2b45a56..8e2dddf 100644
--- a/chrome/browser/metrics/chrome_metrics_service_accessor.h
+++ b/chrome/browser/metrics/chrome_metrics_service_accessor.h
@@ -66,6 +66,7 @@ class ChromeMetricsServiceAccessor : public metrics::MetricsServiceAccessor {
   friend class options::BrowserOptionsHandler;
   friend void InitiateMetricsReportingChange(
       bool, const OnMetricsReportingCallbackType&);
+  friend class MetricsServicesManager;
 
   FRIEND_TEST_ALL_PREFIXES(ChromeMetricsServiceAccessorTest,
                            MetricsReportingEnabled);
diff --git a/chrome/browser/metrics/chrome_metrics_service_accessor_unittest.cc b/chrome/browser/metrics/chrome_metrics_service_accessor_unittest.cc
index dc0271b..7c93160 100644
--- a/chrome/browser/metrics/chrome_metrics_service_accessor_unittest.cc
+++ b/chrome/browser/metrics/chrome_metrics_service_accessor_unittest.cc
@@ -26,6 +26,7 @@ class ChromeMetricsServiceAccessorTest : public testing::Test {
 };
 
 TEST_F(ChromeMetricsServiceAccessorTest, MetricsReportingEnabled) {
+#if defined(GOOGLE_CHROME_BUILD)
 #if !defined(OS_CHROMEOS)
   GetLocalState()->SetBoolean(prefs::kMetricsReportingEnabled, false);
   EXPECT_FALSE(ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
@@ -38,6 +39,10 @@ TEST_F(ChromeMetricsServiceAccessorTest, MetricsReportingEnabled) {
   // device settings for metrics reporting.
   EXPECT_FALSE(ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
 #endif
+#else
+  // Metrics Reporting is never enabled when GOOGLE_CHROME_BUILD is undefined.
+  EXPECT_FALSE(ChromeMetricsServiceAccessor::IsMetricsReportingEnabled());
+#endif
 }
 
 TEST_F(ChromeMetricsServiceAccessorTest, CrashReportingEnabled) {
diff --git a/chrome/browser/metrics/metrics_services_manager.cc b/chrome/browser/metrics/metrics_services_manager.cc
index 8f9384f..4fdcf3f 100644
--- a/chrome/browser/metrics/metrics_services_manager.cc
+++ b/chrome/browser/metrics/metrics_services_manager.cc
@@ -10,6 +10,7 @@
 #include "base/logging.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/browser/metrics/chrome_metrics_service_client.h"
 #include "chrome/browser/metrics/variations/variations_service.h"
 #include "chrome/browser/profiles/profile.h"
@@ -109,8 +110,7 @@ metrics::MetricsStateManager* MetricsServicesManager::GetMetricsStateManager() {
   if (!metrics_state_manager_) {
     metrics_state_manager_ = metrics::MetricsStateManager::Create(
         local_state_,
-        base::Bind(&MetricsServicesManager::IsMetricsReportingEnabled,
-                   base::Unretained(this)),
+        base::Bind(&ChromeMetricsServiceAccessor::IsMetricsReportingEnabled),
         base::Bind(&PostStoreMetricsClientInfo),
         base::Bind(&GoogleUpdateSettings::LoadMetricsClientInfo));
   }
@@ -178,24 +178,7 @@ void MetricsServicesManager::UpdatePermissions(bool may_record,
   GetRapporService()->Update(GetRapporRecordingLevel(may_record), may_upload);
 }
 
-// TODO(asvitkine): This function does not report the correct value on Android,
-// see http://crbug.com/362192.
-bool MetricsServicesManager::IsMetricsReportingEnabled() const {
-  // If the user permits metrics reporting with the checkbox in the
-  // prefs, we turn on recording.  We disable metrics completely for
-  // non-official builds, or when field trials are forced.
-  if (base::CommandLine::ForCurrentProcess()->HasSwitch(
-          switches::kForceFieldTrials))
-    return false;
-
-  bool enabled = false;
-#if defined(GOOGLE_CHROME_BUILD)
-#if defined(OS_CHROMEOS)
-  chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
-                                            &enabled);
-#else
-  enabled = local_state_->GetBoolean(prefs::kMetricsReportingEnabled);
-#endif  // #if defined(OS_CHROMEOS)
-#endif  // defined(GOOGLE_CHROME_BUILD)
-  return enabled;
+void MetricsServicesManager::UpdateUploadPermissions(bool may_upload) {
+  return UpdatePermissions(
+      ChromeMetricsServiceAccessor::IsMetricsReportingEnabled(), may_upload);
 }
diff --git a/chrome/browser/metrics/metrics_services_manager.h b/chrome/browser/metrics/metrics_services_manager.h
index 5e789327..fb5de48 100644
--- a/chrome/browser/metrics/metrics_services_manager.h
+++ b/chrome/browser/metrics/metrics_services_manager.h
@@ -56,8 +56,8 @@ class MetricsServicesManager {
   // metrics change.
   void UpdatePermissions(bool may_record, bool may_upload);
 
-  // Returns true iff metrics reporting is enabled.
-  bool IsMetricsReportingEnabled() const;
+  // Update the managed services when permissions for uploading metrics change.
+  void UpdateUploadPermissions(bool may_upload);
 
   // Returns true iff Rappor reporting is enabled.
   bool IsRapporEnabled(bool metrics_enabled) const;