Use kMetricsReportingEnabled instead of kStatsReporingPref on metrics side

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/callback.h"
 #include "base/metrics/histogram.h"
 #include "base/prefs/pref_service.h"
 #include "chrome/browser/browser_process.h"
 #include "chrome/browser/metrics/chrome_metrics_service_accessor.h"
 #include "chrome/installer/util/google_update_settings.h"
 #include "components/metrics/metrics_pref_names.h"
 #include "components/metrics/metrics_service.h"
 #include "content/public/browser/browser_thread.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/settings/cros_settings.h"
+#include "chromeos/settings/cros_settings_names.h"
+#endif  // defined(OS_CHROMEOS)
+
 namespace {
 
 enum MetricsReportingChangeHistogramValue {
   METRICS_REPORTING_ERROR,
   METRICS_REPORTING_DISABLED,
   METRICS_REPORTING_ENABLED,
   METRICS_REPORTING_MAX
 };
 
 void RecordMetricsReportingHistogramValue(
@@ skipping 24 matching lines @@
 void SetMetricsReporting(bool to_update_pref,
                          const OnMetricsReportingCallbackType& callback_fn,
                          bool updated_pref) {
   metrics::MetricsService* metrics = g_browser_process->metrics_service();
   if (metrics) {
     if (updated_pref)
       metrics->Start();
     else
       metrics->Stop();
   }
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
+
+#if !defined(OS_ANDROID)
   g_browser_process->local_state()->SetBoolean(
       metrics::prefs::kMetricsReportingEnabled, updated_pref);
-#endif
+#endif  // !defined(OS_ANDROID)
+
   // When a user opts in to the metrics reporting service, the previously
   // collected data should be cleared to ensure that nothing is reported before
   // a user opts in and all reported data is accurate.
   if (updated_pref && metrics)
     metrics->ClearSavedStabilityMetrics();
 
   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);
 }
 
+#if defined(OS_CHROMEOS)
+void onDeviceSettingChange() {

[Alexei Svitkine (slow), 2015/11/03 23:18:23]

Nit: Capitalize. Also, add a comment.

[gayane -on leave until 09-2017, 2015/11/05 18:53:32]

Done.

+  bool enable_metrics = false;
+  chromeos::CrosSettings::Get()->GetBoolean(chromeos::kStatsReportingPref,
+                                            &enable_metrics);
+  InitiateMetricsReportingChange(enable_metrics,
+                                 OnMetricsReportingCallbackType());
+}
+#endif
 } // namespace

[Alexei Svitkine (slow), 2015/11/03 23:18:23]

Nit: Add an empty line above.

[gayane -on leave until 09-2017, 2015/11/05 18:53:32]

Done.

 
+// TODO(gayane): Instead of checking policy before setting the metrics pref set
+// the pref and register for notifications for the rest of the changes.
 void InitiateMetricsReportingChange(
     bool enabled,
     const OnMetricsReportingCallbackType& callback_fn) {
-#if !defined(OS_CHROMEOS) && !defined(OS_ANDROID)
-  if (!IsMetricsReportingUserChangable()) {
+#if !defined(OS_ANDROID)
+  if (IsMetricsReportingPolicyManaged()) {
     if (!callback_fn.is_null()) {
       callback_fn.Run(
           ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled());
     }
     return;
   }
 #endif
   // 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() {
+bool IsMetricsReportingPolicyManaged() {
   const PrefService* pref_service = g_browser_process->local_state();
   const PrefService::Preference* pref =
       pref_service->FindPreference(metrics::prefs::kMetricsReportingEnabled);
-  return pref && !pref->IsManaged();
+  return pref && pref->IsManaged();
 }
+
+void SetupMetricsStateForChromeOS() {
+#if defined(OS_CHROMEOS)
+  chromeos::CrosSettings::Get()->AddSettingsObserver(
+      chromeos::kStatsReportingPref, base::Bind(&onDeviceSettingChange));

[Alexei Svitkine (slow), 2015/11/03 23:18:23]

Is it possible to add a unit test for this?

i.e. make the test change the CrOS pref and verify whether the other pref was
updated and metrics service was started or stopped?

[gayane -on leave until 09-2017, 2015/11/05 18:53:32]

As to discussion, I added a TODO to add unittests after refactoring of
metrics_reporting_state

+
+  onDeviceSettingChange();
+#endif // defined(OS_CHROMEOS)
+}