Use kMetricsReportingEnabled instead of kStatsReporingPref on metrics side

chrome/browser/ui/webui/options/browser_options_handler.cc

Index: chrome/browser/ui/webui/options/browser_options_handler.cc
diff --git a/chrome/browser/ui/webui/options/browser_options_handler.cc b/chrome/browser/ui/webui/options/browser_options_handler.cc
index ab08941ddff6b200a15a71c5a05e3972ee17b16e..fc52f5548fb99203af030a81002674b02a4cae56 100644
--- a/chrome/browser/ui/webui/options/browser_options_handler.cc
+++ b/chrome/browser/ui/webui/options/browser_options_handler.cc
@@ -569,11 +569,9 @@ void BrowserOptionsHandler::GetLocalizedValues(base::DictionaryValue* values) {
 
   values->SetString("doNotTrackLearnMoreURL", chrome::kDoNotTrackLearnMoreURL);
 
-#if !defined(OS_CHROMEOS)
   values->SetBoolean(
       "metricsReportingEnabledAtStart",
       ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled());
-#endif
 
 #if defined(OS_CHROMEOS)
   // TODO(pastarmovj): replace this with a call to the CrosSettings list
@@ -2147,14 +2145,18 @@ void BrowserOptionsHandler::SetupExtensionControlledIndicators() {
 }
 
 void BrowserOptionsHandler::SetupMetricsReportingCheckbox() {
-  // This function does not work for ChromeOS and non-official builds.
-#if !defined(OS_CHROMEOS) && defined(GOOGLE_CHROME_BUILD)
+// As the metrics and crash reporting checkbox only exists for official builds
+// it doesn't need to be set up for non-official builds.
+#if defined(GOOGLE_CHROME_BUILD)
   bool checked =
       ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled();
-  bool disabled = !IsMetricsReportingUserChangable();
-
-  SetMetricsReportingCheckbox(checked, disabled);
-#endif
+  bool policy_managed = IsMetricsReportingPolicyManaged();
+  bool owner_managed = false;
+#if defined(OS_CHROMEOS)
+  owner_managed = !IsDeviceOwnerProfile();
+#endif  // defined(OS_CHROMEOS)
+  SetMetricsReportingCheckbox(checked, policy_managed, owner_managed);
+#endif  // defined(GOOGLE_CHROME_BUILD)
 }
 
 void BrowserOptionsHandler::HandleMetricsReportingChange(
@@ -2162,23 +2164,42 @@ void BrowserOptionsHandler::HandleMetricsReportingChange(
   bool enable;
   if (!args->GetBoolean(0, &enable))
     return;
+  // Decline the change if current user shouldn't be able to change metrics
+  // reporting.
+  if (!IsDeviceOwnerProfile() || IsMetricsReportingPolicyManaged()) {
+    NotifyUIOfMetricsReportingChange(
+        ChromeMetricsServiceAccessor::IsMetricsAndCrashReportingEnabled());
+    return;
+  }
 
+// For Chrome OS updating device settings will notify an observer to update
+// metrics pref, however we still need to call |InitiateMetricsReportingChange|
+// with a proper callback so that UI gets updated in case of failure to update
+// the metrics pref.
+// TODO(gayane): Don't call |InitiateMetricsReportingChange| twice for Chrome
+// OS.

[Mattias Nissler (ping if slow), 2015/11/06 13:05:09]

Sorry, but this is not just a cosmetic issue about a function getting called
twice. You should only ever call InitiateMetricsReportingChange() once device
settings say metrics reporting is on. Please update the comment to state that
metrics reporting state can only change after the device settings has flipped to
true. Only then can you have full confidence that the change was initiated by
the device owner.

The TODO should also reference a bug.

I'm fine with addressing these points in subsequent CLs, given that you have the
check above to stop any action in case we're not the device owner.

[gayane -on leave until 09-2017, 2015/11/06 21:41:13]

Done.

+#if defined(OS_CHROMEOS)
+  chromeos::CrosSettings::Get()->SetBoolean(chromeos::kStatsReportingPref,
+                                            enable);
+#endif  // defined(OS_CHROMEOS)
   InitiateMetricsReportingChange(
       enable,
-      base::Bind(&BrowserOptionsHandler::MetricsReportingChangeCallback,
+      base::Bind(&BrowserOptionsHandler::NotifyUIOfMetricsReportingChange,
                  base::Unretained(this)));
 }
 
-void BrowserOptionsHandler::MetricsReportingChangeCallback(bool enabled) {
-  SetMetricsReportingCheckbox(enabled, !IsMetricsReportingUserChangable());
+void BrowserOptionsHandler::NotifyUIOfMetricsReportingChange(bool enabled) {
+  SetMetricsReportingCheckbox(enabled, IsMetricsReportingPolicyManaged(),
+                              !IsDeviceOwnerProfile());
 }
 
 void BrowserOptionsHandler::SetMetricsReportingCheckbox(bool checked,
-                                                        bool disabled) {
+                                                        bool policy_managed,
+                                                        bool owner_managed) {
   web_ui()->CallJavascriptFunction(
       "BrowserOptions.setMetricsReportingCheckboxState",
-      base::FundamentalValue(checked),
-      base::FundamentalValue(disabled));
+      base::FundamentalValue(checked), base::FundamentalValue(policy_managed),
+      base::FundamentalValue(owner_managed));
 }
 
 void BrowserOptionsHandler::OnPolicyUpdated(const policy::PolicyNamespace& ns,
@@ -2190,4 +2211,12 @@ void BrowserOptionsHandler::OnPolicyUpdated(const policy::PolicyNamespace& ns,
     SetupMetricsReportingCheckbox();
 }
 
+bool BrowserOptionsHandler::IsDeviceOwnerProfile() {
+#if defined(OS_CHROMEOS)
+  return chromeos::ProfileHelper::IsOwnerProfile(Profile::FromWebUI(web_ui()));
+#else
+  return true;
+#endif
+}
+
 }  // namespace options