Fix ChromeMetricsServiceAccessor::IsMetricsReportingEnabled on CrOs

chrome/browser/metrics/chrome_metrics_service_accessor.cc

 // Copyright 2014 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/chrome_metrics_service_accessor.h"
 
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/metrics/metrics_services_manager.h"
 #include "chrome/common/pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "components/variations/metrics_util.h"
 
 #if defined(OS_CHROMEOS)
 #include "chrome/browser/chromeos/settings/cros_settings.h"
 #endif
 
 // static
 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);
-    }
-  }
-  return result;
+  return g_browser_process->GetMetricsServicesManager()->
+      IsMetricsReportingEnabled();

[Alexei Svitkine (slow), 2015/02/11 21:58:26]

IsMetricsReportingEnabled() on MetricsServicesManager needs to be private.

It's an oversight that it currently isn't.

Can you make it private and adjust this code accordingly?

For example, you can actually move its implementation into this class and have
MetricsServicesManager call this version.

[Alexei Svitkine (slow), 2015/02/11 22:06:11]

I just took a glance, and only place that relies on IsMetricsReportingEnabled()
not being private is a call from chrome_browser_main.cc which ends up calling
MetricsServicesManager::UpdatePermissions() on the metrics services manager - so
you can add a new version of UpdatePermissions function that does that all
within MetricsServicesManager.

[anthonyvd, 2015/02/12 15:48:54]

Done.

[anthonyvd, 2015/02/12 15:48:55]

Done.

 }
 
 bool ChromeMetricsServiceAccessor::IsCrashReportingEnabled() {
 #if defined(GOOGLE_CHROME_BUILD)
-#if defined(OS_CHROMEOS)
-  bool reporting_enabled = false;
-  chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
-                                            &reporting_enabled);

[Alexei Svitkine (slow), 2015/02/11 21:58:26]

Why did you remove this?

[anthonyvd, 2015/02/12 15:48:55]

It was a fix for ChromeMetricsServiceAccessor::IsMetricsReportingEnabled on
ChromeOS.  Now that it's working properly, the special ChromeOS branch isn't
needed anymore (IsMetricsReportingEnabled has the exact same code).

-  return reporting_enabled;
-#elif defined(OS_ANDROID)
+#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);
 #else
   return ChromeMetricsServiceAccessor::IsMetricsReportingEnabled();
 #endif
 #else
   return false;
 #endif
 }
 
 // static
 bool ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrial(
     const std::string& trial_name,
     const std::string& group_name) {
   return RegisterSyntheticFieldTrialWithNameHash(metrics::HashName(trial_name),
                                                  group_name);
 }
 
 // static
 bool ChromeMetricsServiceAccessor::RegisterSyntheticFieldTrialWithNameHash(
     uint32_t trial_name_hash,
     const std::string& group_name) {
   return metrics::MetricsServiceAccessor::RegisterSyntheticFieldTrial(
       g_browser_process->metrics_service(),
       trial_name_hash,
       metrics::HashName(group_name));
 }