[OsSettingsMetrics] Add metric handling for pref based setting

Adds a new helper function that converts a (pref, prefValue) pair to
a (setting, settingChangeValue) pair that can be used to log metrics.
Uses this helper function to automatically capture setting changes
for all pref based settings. As a first example, implements metrics
for the kKeyboardFunctionKeys on the Device page.

Bug:  1133553

Change-Id: Ia1f0118e27bb7e0f610bfe1d75ba0ef324260f7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2459846
Commit-Queue: Daniel Classon <dclasson@google.com>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816692}

chrome/browser/resources/settings/chromeos/BUILD.gn

@@ -196,6 +196,7 @@ group("closure_compile") {
     ":os_page_visibility",
     ":os_route",
     ":os_settings_routes",
+    ":pref_to_setting_metric_converter",
     ":route_origin_behavior",
     ":search_handler",
     "ambient_mode_page:closure_compile",
@@ -267,6 +268,10 @@ js_library("os_settings_routes") {
   ]
 }
 
+js_library("pref_to_setting_metric_converter") {
+  deps = [ "//chrome/browser/ui/webui/settings/chromeos/constants:mojom_js_library_for_compile" ]
+}
+
 js_library("route_origin_behavior") {
   deps = [
     ":os_route",
@@ -343,6 +348,7 @@ js_type_check("closure_compile_local_module") {
     #  ":os_settings.m",
     #  ":os_settings_icons_css.m",
     ":os_settings_routes.m",
+    ":pref_to_setting_metric_converter.m",
     ":route_origin_behavior.m",
 
     #":search_handler.m",
@@ -411,6 +417,12 @@ js_library("os_settings_routes.m") {
   extra_deps = [ ":modulize" ]
 }
 
+js_library("pref_to_setting_metric_converter.m") {
+  sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/pref_to_setting_metric_converter.m.js" ]
+  deps = [ "//chrome/browser/ui/webui/settings/chromeos/constants:mojom_js_library_for_compile" ]
+  extra_deps = [ ":modulize" ]
+}
+
 js_library("route_origin_behavior.m") {
   sources = [ "$root_gen_dir/chrome/browser/resources/settings/chromeos/route_origin_behavior.m.js" ]
   deps = [
@@ -497,13 +509,14 @@ polymer_modulizer("os_settings_icons_css") {
 
 js_modulizer("modulize") {
   input_files = [
+    "deep_linking_behavior.js",
     "metrics_recorder.js",
     "os_settings_routes.js",
     "route_origin_behavior.js",
     "search_handler.js",
     "os_route.js",
     "os_page_visibility.js",
-    "deep_linking_behavior.js",
+    "pref_to_setting_metric_converter.js",
   ]
   namespace_rewrites = os_settings_namespace_rewrites
 }

chrome/browser/resources/settings/chromeos/os_settings_resources_v3.grdp

@@ -623,6 +623,11 @@
            file="page_visibility.js"
            preprocess="true"
            compress="false" type="BINDATA" />
+  <include name="IDR_OS_SETTINGS_PREF_TO_SETTING_METRIC_CONVERTER_M_JS"
+           file="${root_gen_dir}/chrome/browser/resources/settings/chromeos/pref_to_setting_metric_converter.m.js"
+           use_base_dir="false"
+           compress="false"
+           type="BINDATA" />
   <include name="IDR_OS_SETTINGS_MULTIDEVICE_PAGE_M_JS"
            file="${root_gen_dir}/chrome/browser/resources/settings/chromeos/multidevice_page/multidevice_page.m.js"
            use_base_dir="false"

chrome/browser/resources/settings/chromeos/os_settings_ui/BUILD.gn

@@ -13,6 +13,7 @@ js_library("os_settings_ui") {
     "..:metrics_recorder",
     "..:os_page_visibility",
     "..:os_route",
+    "..:pref_to_setting_metric_converter",
     "../..:global_scroll_target_behavior",
     "../..:router",
     "../../prefs",

chrome/browser/resources/settings/chromeos/os_settings_ui/os_settings_ui.html

@@ -16,10 +16,13 @@
 <link rel="import" href="../../i18n_setup.html">
 <link rel="import" href="../../settings_shared_css.html">
 <link rel="import" href="../os_page_visibility.html">
+<link rel="import" href="../pref_to_setting_metric_converter.html">
 <link rel="import" href="../../prefs/prefs.html">
 <link rel="import" href="../os_route.html">
 <link rel="import" href="../../router.html">
 <link rel="import" href="../../settings_vars_css.html">
+<script src="chrome://os-settings/constants/setting.mojom-lite.js"></script>
+<script src="chrome://os-settings/search/user_action_recorder.mojom-lite.js"></script>
 
 <dom-module id="os-settings-ui">
   <template>

chrome/browser/resources/settings/chromeos/os_settings_ui/os_settings_ui.js

@@ -125,9 +125,16 @@ cr.define('settings', function() {
      */
     activeRoute_: null,
 
+    /**
+     * Converts prefs to settings metrics to help record pref changes.
+     * @private {PrefToSettingMetricConverter}
+     */
+    prefToSettingMetricConverter_: null,
+
     /** @override */
     created() {
       settings.Router.getInstance().initializeRouteFromUrl();
+      this.prefToSettingMetricConverter_ = new PrefToSettingMetricConverter();
     },
 
     /**
@@ -350,10 +357,26 @@ cr.define('settings', function() {
     },
 
     /**
+     * @param {!CustomEvent<!{prefKey: string, prefValue: *}>} e
      * @private
      */
-    onSettingChange_() {
-      settings.recordSettingChange();
+    onSettingChange_(e) {
+      const {prefKey, prefValue} = e.detail;
+      const settingMetric =
+          this.prefToSettingMetricConverter_.convertPrefToSettingMetric(
+              prefKey, prefValue);
+
+      // New metrics for this setting pref have not yet been implemented.
+      if (!settingMetric) {
+        settings.recordSettingChange();
+        return;
+      }
+
+      const setting = /** @type {!chromeos.settings.mojom.Setting} */ (
+          settingMetric.setting);
+      const value = /** @type {!chromeos.settings.mojom.SettingChangeValue} */ (
+          settingMetric.value);
+      settings.recordSettingChange(setting, value);
     },
 
     /**

chrome/browser/resources/settings/chromeos/pref_to_setting_metric_converter.html

+<script src="chrome://os-settings/constants/setting.mojom-lite.js"></script>
+<script src="chrome://os-settings/search/user_action_recorder.mojom-lite.js"></script>
+<script src="pref_to_setting_metric_converter.js"></script>
\ No newline at end of file

chrome/browser/resources/settings/chromeos/pref_to_setting_metric_converter.js

+/* Copyright 2020 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. */
+
+/**
+ * @fileoverview Helper class for converting a given settings pref to a pair of
+ * setting ID and setting change value. Used to record metrics about changes
+ * to pref-based settings.
+ */
+
+// clang-format off
+// #import 'chrome://resources/mojo/mojo/public/js/mojo_bindings_lite.js'
+// #import '../constants/setting.mojom-lite.js';
+// #import '../search/user_action_recorder.mojom-lite.js';
+// clang-format on
+
+/* export */ class PrefToSettingMetricConverter {
+  /**
+   * @param {string} prefKey
+   * @param {*} prefValue
+   * @return {?{setting: !chromeos.settings.mojom.Setting, value:
+   *     !chromeos.settings.mojom.SettingChangeValue}}
+   */
+  convertPrefToSettingMetric(prefKey, prefValue) {
+    switch (prefKey) {
+      // device_page/keyboard.js
+      case 'settings.language.send_function_keys':
+        return {
+          setting: chromeos.settings.mojom.Setting.kKeyboardFunctionKeys,
+          value: {boolValue: /** @type {boolean} */ (prefValue)}
+        };
+
+      // pref to setting metric not implemented.
+      default:
+        return null;
+    }
+  }
+}

chrome/browser/resources/settings/os_settings_resources.grd

@@ -985,6 +985,12 @@
       <structure name="IDR_OS_SETTINGS_OS_PAGE_VISIBILITY_JS"
                  file="chromeos/os_page_visibility.js"
                  compress="false" type="chrome_html" />
+      <structure name="IDR_OS_SETTINGS_PREF_TO_SETTING_METRIC_CONVERTER_HTML"
+                 file="chromeos/pref_to_setting_metric_converter.html"
+                 compress="false" type="chrome_html" />
+      <structure name="IDR_OS_SETTINGS_PREF_TO_SETTING_METRIC_CONVERTER_JS"
+                 file="chromeos/pref_to_setting_metric_converter.js"
+                 compress="false" type="chrome_html" />
       <!-- TODO(jamescook): Remove after sync settings is forked. -->
       <structure name="IDR_OS_SETTINGS_PERSONALIZATION_OPTIONS_HTML"
                  file="privacy_page/personalization_options.html"

chrome/browser/resources/settings/prefs/prefs.js

@@ -213,7 +213,9 @@ Polymer({
     // settingsPrivate.setPref and potentially trigger an IPC loop.)
     if (!deepEqual(prefStoreValue, prefObj.value)) {
       // <if expr="chromeos">
-      this.fire('user-action-setting-change');
+      this.fire(
+          'user-action-setting-change',
+          {prefKey: key, prefValue: prefObj.value});
       // </if>
 
       this.settingsApi_.setPref(

chrome/browser/ui/webui/settings/chromeos/device_section.cc

@@ -10,6 +10,7 @@
 #include "ash/public/cpp/stylus_utils.h"
 #include "base/command_line.h"
 #include "base/feature_list.h"
+#include "base/metrics/histogram_functions.h"
 #include "base/no_destructor.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/utf_string_conversions.h"
@@ -842,8 +843,15 @@ std::string DeviceSection::GetSectionPath() const {
 
 bool DeviceSection::LogMetric(mojom::Setting setting,
                               base::Value& value) const {
-  // Unimplemented.
-  return false;
+  switch (setting) {
+    case mojom::Setting::kKeyboardFunctionKeys:
+      base::UmaHistogramBoolean("ChromeOS.Settings.Device.KeyboardFunctionKeys",
+                                value.GetBool());
+      return true;
+
+    default:
+      return false;
+  }
 }
 
 void DeviceSection::RegisterHierarchy(HierarchyGenerator* generator) const {

chrome/browser/ui/webui/settings/chromeos/settings_user_action_tracker.h

@@ -42,6 +42,8 @@ class SettingsUserActionTracker : public mojom::UserActionRecorder {
                            TestRecordSettingChangedBool);
   FRIEND_TEST_ALL_PREFIXES(SettingsUserActionTrackerTest,
                            TestRecordSettingChangedInt);
+  FRIEND_TEST_ALL_PREFIXES(SettingsUserActionTrackerTest,
+                           TestRecordSettingChangedBoolPref);
 
   // mojom::UserActionRecorder:
   void RecordPageFocus() override;

chrome/browser/ui/webui/settings/chromeos/settings_user_action_tracker_unittest.cc

@@ -33,6 +33,8 @@ class SettingsUserActionTrackerTest : public testing::Test {
   void SetUp() override {
     fake_hierarchy_.AddSettingMetadata(mojom::Section::kBluetooth,
                                        mojom::Setting::kBluetoothOnOff);
+    fake_hierarchy_.AddSettingMetadata(mojom::Section::kDevice,
+                                       mojom::Setting::kKeyboardFunctionKeys);
     fake_hierarchy_.AddSettingMetadata(mojom::Section::kPeople,
                                        mojom::Setting::kAddAccount);
   }
@@ -86,5 +88,29 @@ TEST_F(SettingsUserActionTrackerTest, TestRecordSettingChangedInt) {
               mojom::Setting::kAddAccount);
 }
 
+TEST_F(SettingsUserActionTrackerTest, TestRecordSettingChangedBoolPref) {
+  // Record that the keyboard function keys setting was toggled off. This
+  // setting is controlled by a pref and uses the pref-to-setting-metric
+  // converter flow.
+  tracker_.RecordSettingChangeWithDetails(
+      mojom::Setting::kKeyboardFunctionKeys,
+      mojom::SettingChangeValue::NewBoolValue(false));
+
+  // The umbrella metric for which setting was changed should be updated. Note
+  // that kKeyboardFunctionKeys has enum value of 411.
+  histogram_tester_.ExpectTotalCount("ChromeOS.Settings.SettingChanged",
+                                     /*count=*/1);
+  histogram_tester_.ExpectBucketCount("ChromeOS.Settings.SettingChanged",
+                                      /*sample=*/411,
+                                      /*count=*/1);
+
+  // The LogMetric fn in the Device section should have been called.
+  const FakeOsSettingsSection* device_section =
+      static_cast<const FakeOsSettingsSection*>(
+          fake_sections_.GetSection(mojom::Section::kDevice));
+  EXPECT_TRUE(device_section->logged_metrics().back() ==
+              mojom::Setting::kKeyboardFunctionKeys);
+}
+
 }  // namespace settings.
 }  // namespace chromeos.

tools/metrics/histograms/histograms_xml/chromeos/histograms.xml

@@ -548,6 +548,17 @@ incoming reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="ChromeOS.Settings.Device.KeyboardFunctionKeys"
+    enum="BooleanToggled" expires_after="2021-09-30">
+  <owner>khorimoto@chromium.org</owner>
+  <owner>hsuregan@chromium.org</owner>
+  <owner>cros-customization@google.com</owner>
+  <summary>
+    Records when a user changes the kKeyboardFunctionKeys setting on the Device
+    page.
+  </summary>
+</histogram>
+
 <histogram name="ChromeOS.Settings.Languages.Browser.Interaction"
     enum="SettingsLanguagesPageBrowserInteraction" expires_after="2021-03-31">
   <owner>myy@chromium.org</owner>