[CrOS Bluetooth] Add HidConnected metric to legacy HID screen.

Create hid_detection_utils.h that will contain HID detection screen
metrics-related logic. Emit HidConnected metric for each HID connected
while the HID detection screen is shown in the pre-revamp HID detection
screen.

Bug: b/229012996
Test: HIDDetectionScreen*Test
Change-Id: Ifbed7530d2a23f404d685347e35845bf0cff601b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3632655
Reviewed-by: Chad Duffin <chadduffin@chromium.org>
Reviewed-by: Roman Sorokin <rsorokin@chromium.org>
Reviewed-by: Reilly Grant <reillyg@chromium.org>
Commit-Queue: Gordon Seto <gordonseto@google.com>
Cr-Commit-Position: refs/heads/main@{#1002262}

ash/components/hid_detection/BUILD.gn

@@ -12,6 +12,8 @@ static_library("hid_detection") {
     "bluetooth_hid_detector.h",
     "bluetooth_hid_detector_impl.cc",
     "bluetooth_hid_detector_impl.h",
+    "hid_detection_utils.cc",
+    "hid_detection_utils.h",
   ]
 
   deps = [
@@ -22,6 +24,7 @@ static_library("hid_detection") {
     "//components/device_event_log",
     "//device/bluetooth",
     "//mojo/public/cpp/bindings",
+    "//services/device/public/mojom",
   ]
 }
 

ash/components/hid_detection/DEPS

@@ -1,4 +1,5 @@
 include_rules = [
   "+chromeos/services/bluetooth_config",
   "+components/device_event_log",
+  "+services/device/public",
 ]

ash/components/hid_detection/hid_detection_utils.cc

@@ -0,0 +1,67 @@
+// Copyright 2022 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 "ash/components/hid_detection/hid_detection_utils.h"
+
+#include "base/metrics/histogram_functions.h"
+#include "components/device_event_log/device_event_log.h"
+#include "third_party/abseil-cpp/absl/types/optional.h"
+
+namespace ash::hid_detection {
+namespace {
+
+using InputDeviceType = device::mojom::InputDeviceType;
+
+absl::optional<HidType> GetHidType(
+    const device::mojom::InputDeviceInfo& device) {
+  if (device.is_touchscreen || device.is_tablet)
+    return HidType::kTouchscreen;
+
+  if (device.is_mouse || device.is_touchpad) {
+    switch (device.type) {
+      case InputDeviceType::TYPE_BLUETOOTH:
+        return HidType::kBluetoothPointer;
+      case InputDeviceType::TYPE_USB:
+        return HidType::kUsbPointer;
+      case InputDeviceType::TYPE_SERIO:
+        return HidType::kSerialPointer;
+      case InputDeviceType::TYPE_UNKNOWN:
+        return HidType::kUnknownPointer;
+    }
+  }
+
+  if (device.is_keyboard) {
+    switch (device.type) {
+      case InputDeviceType::TYPE_BLUETOOTH:
+        return HidType::kBluetoothKeyboard;
+      case InputDeviceType::TYPE_USB:
+        return HidType::kUsbKeyboard;
+      case InputDeviceType::TYPE_SERIO:
+        return HidType::kSerialKeyboard;
+      case InputDeviceType::TYPE_UNKNOWN:
+        return HidType::kUnknownKeyboard;
+    }
+  }
+
+  return absl::nullopt;
+}
+
+}  // namespace
+
+void RecordHidConnected(const device::mojom::InputDeviceInfo& device) {
+  absl::optional<HidType> hid_type = GetHidType(device);
+
+  // If |device| is not relevant (i.e. an accelerometer, joystick, etc), don't
+  // emit metric.
+  if (!hid_type.has_value()) {
+    HID_LOG(DEBUG) << "HidConnected not logged for device " << device.id
+                   << " because it doesn't have a relevant device type.";
+    return;
+  }
+
+  base::UmaHistogramEnumeration("OOBE.HidDetectionScreen.HidConnected",
+                                hid_type.value());
+}
+
+}  // namespace ash::hid_detection

ash/components/hid_detection/hid_detection_utils.h

@@ -0,0 +1,33 @@
+// Copyright 2022 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.
+
+#ifndef ASH_COMPONENTS_HID_DETECTION_HID_DETECTION_UTILS_H_
+#define ASH_COMPONENTS_HID_DETECTION_HID_DETECTION_UTILS_H_
+
+#include "services/device/public/mojom/input_service.mojom.h"
+
+namespace ash::hid_detection {
+
+// This enum is tied directly to the HidType UMA enum defined in
+// //tools/metrics/histograms/enums.xml, and should always reflect it (do not
+// change one without changing the other).
+enum class HidType {
+  kTouchscreen = 0,
+  kUsbKeyboard = 1,
+  kUsbPointer = 2,
+  kSerialKeyboard = 3,
+  kSerialPointer = 4,
+  kBluetoothKeyboard = 5,
+  kBluetoothPointer = 6,
+  kUnknownKeyboard = 7,
+  kUnknownPointer = 8,
+  kMaxValue = kUnknownPointer
+};
+
+// Record each HID that is connected while the HID detection screen is shown.
+void RecordHidConnected(const device::mojom::InputDeviceInfo& device);
+
+}  // namespace ash::hid_detection
+
+#endif  // ASH_COMPONENTS_HID_DETECTION_HID_DETECTION_UTILS_H_

chrome/browser/ash/login/screens/hid_detection_screen.cc

@@ -7,6 +7,7 @@
 #include <memory>
 #include <utility>
 
+#include "ash/components/hid_detection/hid_detection_utils.h"
 #include "ash/constants/ash_switches.h"
 #include "base/bind.h"
 #include "base/callback_helpers.h"
@@ -475,6 +476,8 @@ void HIDDetectionScreen::InputDeviceAdded(InputDeviceInfoPtr info) {
     SetKeyboardDeviceName(info_ref->name);
     SendKeyboardDeviceNotification();
   }
+  DCHECK(!info_ref.is_null());
+  hid_detection::RecordHidConnected(*info_ref);
 }
 
 void HIDDetectionScreen::InputDeviceAddedForTesting(InputDeviceInfoPtr info) {

chrome/browser/ash/login/screens/hid_detection_screen_browsertest.cc

@@ -2,10 +2,12 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "ash/components/hid_detection/hid_detection_utils.h"
 #include "ash/constants/ash_switches.h"
 #include "base/bind.h"
 #include "base/callback_helpers.h"
 #include "base/run_loop.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/scoped_chromeos_version_info.h"
 #include "chrome/browser/ash/login/login_wizard.h"
 #include "chrome/browser/ash/login/screens/base_screen.h"
@@ -31,6 +33,7 @@ namespace ash {
 namespace {
 
 using ::testing::_;
+using HidType = hid_detection::HidType;
 
 const test::UIPath kHidContinueButton = {"hid-detection",
                                          "hid-continue-button"};
@@ -89,6 +92,11 @@ class HIDDetectionScreenChromeboxTest : public OobeBaseTest {
         ->get_exit_result_for_testing();
   }
 
+  void AssertHidConnectedCount(HidType hid_type, int count) {
+    histogram_tester_.ExpectBucketCount("OOBE.HidDetectionScreen.HidConnected",
+                                        hid_type, count);
+  }
+
   test::HIDControllerMixin hid_controller_{&mixin_host_};
 
  private:
@@ -98,6 +106,8 @@ class HIDDetectionScreenChromeboxTest : public OobeBaseTest {
   // Chromeboxes.
   base::test::ScopedChromeOSVersionInfo version_{"DEVICETYPE=CHROMEBOX",
                                                  base::Time::Now()};
+
+  base::HistogramTester histogram_tester_;
 };
 
 IN_PROC_BROWSER_TEST_F(HIDDetectionScreenChromeboxTest, NoDevicesConnected) {
@@ -117,11 +127,13 @@ IN_PROC_BROWSER_TEST_F(HIDDetectionScreenChromeboxTest, MouseKeyboardStates) {
   // or touchscreen, the keyboard only reports usb and bluetooth states.
   hid_controller_.AddMouse(device::mojom::InputDeviceType::TYPE_SERIO);
   test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+  AssertHidConnectedCount(HidType::kSerialPointer, /*count=*/1);
 
   hid_controller_.AddKeyboard(device::mojom::InputDeviceType::TYPE_SERIO);
   EXPECT_EQ("connected", handler()->mouse_state_for_test());
   EXPECT_EQ("usb", handler()->keyboard_state_for_test());
   test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+  AssertHidConnectedCount(HidType::kSerialKeyboard, /*count=*/1);
 
   // Remove generic devices, add usb devices.
   hid_controller_.RemoveDevices();
@@ -132,6 +144,8 @@ IN_PROC_BROWSER_TEST_F(HIDDetectionScreenChromeboxTest, MouseKeyboardStates) {
   EXPECT_EQ("usb", handler()->mouse_state_for_test());
   EXPECT_EQ("usb", handler()->keyboard_state_for_test());
   test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+  AssertHidConnectedCount(HidType::kUsbKeyboard, /*count=*/1);
+  AssertHidConnectedCount(HidType::kUsbPointer, /*count=*/1);
 
   // Remove usb devices, add bluetooth devices.
   hid_controller_.RemoveDevices();
@@ -141,6 +155,8 @@ IN_PROC_BROWSER_TEST_F(HIDDetectionScreenChromeboxTest, MouseKeyboardStates) {
   EXPECT_EQ("paired", handler()->mouse_state_for_test());
   EXPECT_EQ("paired", handler()->keyboard_state_for_test());
   test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+  AssertHidConnectedCount(HidType::kBluetoothKeyboard, /*count=*/1);
+  AssertHidConnectedCount(HidType::kBluetoothPointer, /*count=*/1);
 }
 
 // Test that if there is any Bluetooth device connected on HID screen, the
@@ -228,11 +244,15 @@ class HIDDetectionSkipTest : public HIDDetectionScreenChromeboxTest {
     hid_controller_.ConnectUSBDevices();
   }
   ~HIDDetectionSkipTest() override = default;
+
+ protected:
+  base::HistogramTester histogram_tester;
 };
 
 IN_PROC_BROWSER_TEST_F(HIDDetectionSkipTest, BothDevicesPreConnected) {
   OobeScreenWaiter(WelcomeView::kScreenId).Wait();
   EXPECT_FALSE(GetExitResult().has_value());
+  histogram_tester.ExpectTotalCount("OOBE.HidDetectionScreen.HidConnected", 0);
 }
 
 class HIDDetectionDeviceOwnedTest : public HIDDetectionScreenChromeboxTest {
@@ -342,9 +362,46 @@ IN_PROC_BROWSER_TEST_F(HIDDetectionScreenChromebaseTest, TouchscreenDetected) {
   test::OobeJS().ExpectEnabledPath(kHidContinueButton);
 
   hid_controller_.ConnectUSBDevices();
+  test::OobeJS().CreateVisibilityWaiter(true, kHidMouseTick)->Wait();
+  test::OobeJS().CreateVisibilityWaiter(true, kHidKeyboardTick)->Wait();
+
   hid_controller_.RemoveDevices();
+  test::OobeJS().CreateVisibilityWaiter(false, kHidMouseTick)->Wait();
+  test::OobeJS().CreateVisibilityWaiter(false, kHidKeyboardTick)->Wait();
+
+  test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+}
+
+class HIDDetectionScreenPreConnectedDeviceTest
+    : public HIDDetectionScreenChromeboxTest {
+ public:
+  HIDDetectionScreenPreConnectedDeviceTest() {
+    hid_controller_.set_wait_until_idle_after_device_update(false);
+    hid_controller_.AddMouse(device::mojom::InputDeviceType::TYPE_USB);
+  }
+};
+
+IN_PROC_BROWSER_TEST_F(HIDDetectionScreenPreConnectedDeviceTest,
+                       MousePreConnected) {
+  // Continue button should be enabled if at least one device is connected.
+  OobeScreenWaiter(HIDDetectionView::kScreenId).Wait();
+  test::OobeJS().ExpectHiddenPath(kHidTouchscreenEntry);
+  test::OobeJS().CreateVisibilityWaiter(true, kHidMouseTick)->Wait();
+  test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+  AssertHidConnectedCount(HidType::kUsbPointer, /*count=*/0);
+  AssertHidConnectedCount(HidType::kUsbKeyboard, /*count=*/0);
 
+  hid_controller_.AddKeyboard(device::mojom::InputDeviceType::TYPE_USB);
+  test::OobeJS().CreateVisibilityWaiter(true, kHidKeyboardTick)->Wait();
+  test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+  AssertHidConnectedCount(HidType::kUsbPointer, /*count=*/0);
+  AssertHidConnectedCount(HidType::kUsbKeyboard, /*count=*/1);
+
+  hid_controller_.AddKeyboard(device::mojom::InputDeviceType::TYPE_USB);
+  test::OobeJS().CreateVisibilityWaiter(true, kHidKeyboardTick)->Wait();
   test::OobeJS().ExpectEnabledPath(kHidContinueButton);
+  AssertHidConnectedCount(HidType::kUsbPointer, /*count=*/0);
+  AssertHidConnectedCount(HidType::kUsbKeyboard, /*count=*/2);
 }
 
 }  // namespace ash

chrome/browser/chromeos/BUILD.gn

@@ -99,6 +99,7 @@ source_set("chromeos") {
     "//ash/components/enhanced_network_tts/mojom",
     "//ash/components/fwupd",
     "//ash/components/geolocation",
+    "//ash/components/hid_detection:hid_detection",
     "//ash/components/login/auth",
     "//ash/components/login/session",
     "//ash/components/multidevice",

chrome/test/BUILD.gn

@@ -4002,6 +4002,7 @@ if (!is_android) {
         "//ash/components/drivefs:test_support",
         "//ash/components/drivefs/mojom",
         "//ash/components/geolocation",
+        "//ash/components/hid_detection:hid_detection",
         "//ash/components/login/auth",
         "//ash/components/login/auth:challenge_response_key",
         "//ash/components/login/auth:test_support",

tools/metrics/histograms/enums.xml

@@ -46684,6 +46684,22 @@ Called by update_gpu_driver_bug_workaround_entries.py.-->
   <int value="2" label="Both devices, pointing and keyboard, detected."/>
 </enum>
 
+<enum name="HidType">
+  <summary>
+    Possible device types of HIDs interfaced with in the OOBE HID detection
+    screen
+  </summary>
+  <int value="0" label="Touchscreen"/>
+  <int value="1" label="USB Keyboard"/>
+  <int value="2" label="USB Pointer"/>
+  <int value="3" label="Serial Keyboard"/>
+  <int value="4" label="Serial Pointer"/>
+  <int value="5" label="Bluetooth Keyboard"/>
+  <int value="6" label="Bluetooth Pointer"/>
+  <int value="7" label="Unknown Keyboard"/>
+  <int value="8" label="Unknown Pointer"/>
+</enum>
+
 <enum name="HintCacheStoreEntryType">
   <summary>
     Possible store entry types contained within the HintCacheStore.

tools/metrics/histograms/metadata/oobe/histograms.xml

@@ -168,6 +168,18 @@ chromium-metrics-reviews@google.com.
   </summary>
 </histogram>
 
+<histogram name="OOBE.HidDetectionScreen.HidConnected" enum="HidType"
+    expires_after="2023-05-06">
+  <owner>gordonseto@google.com</owner>
+  <owner>cros-connectivity@google.com</owner>
+  <summary>
+    Records the type of a connected human interface device (HID). This metric is
+    emitted each time a HID is connected during the HID detection screen in
+    OOBE. It does not include devices connected before the screen is shown or
+    after the screen is hidden.
+  </summary>
+</histogram>
+
 <histogram name="OOBE.MarketingOptInScreen.BackendConnector"
     enum="MarketingOptInBackendConnectorEvent" expires_after="2022-11-30">
   <owner>rrsilva@google.com</owner>