cros_healthd: Migrate AddUsbObserver to new interface

BUG=b:268407683 TEST=autoninja -C chrome TEST=autoninja -C browser_tests TEST=CQ Change-Id: I82390aa4206a519ee23315b536d691dd7a9d6139 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4273949 Reviewed-by: Leonid Baraz <lbaraz@chromium.org> Commit-Queue: Kerker Yang <kerker@chromium.org> Reviewed-by: Chung-sheng Wu <chungsheng@google.com> Cr-Commit-Position: refs/heads/main@{#1108736}
chromium · Feb 23, 2023 · 72764ec · 72764ec
1 parent 40a4e45
commit 72764ec
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 56 deletions.
diff --git a/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_browsertest.cc b/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_browsertest.cc
@@ -138,6 +138,14 @@ class UsbEventsBrowserTest : public ::policy::DevicePolicyCrosBrowserTest {
     return telemetry_info;
   }
 
+  void EmitUsbAddEventForTesting() {
+    cros_healthd::mojom::UsbEventInfo info;
+    info.state = cros_healthd::mojom::UsbEventInfo::State::kAdd;
+    cros_healthd::FakeCrosHealthd::Get()->EmitEventForCategory(
+        cros_healthd::mojom::EventCategoryEnum::kUsb,
+        cros_healthd::mojom::EventInfo::NewUsbEventInfo(info.Clone()));
+  }
+
   const AccountId test_account_id_ = AccountId::FromUserEmailGaiaId(
       kTestUserEmail,
       signin::GetTestGaiaIdForEmail(kTestUserEmail));
@@ -163,7 +171,7 @@ IN_PROC_BROWSER_TEST_F(UsbEventsBrowserTest,
       usb_telemetry);
 
   // Any USB event should trigger event driven telemetry collection
-  cros_healthd::FakeCrosHealthd::Get()->EmitUsbAddEventForTesting();
+  EmitUsbAddEventForTesting();
 
   Record record = std::get<1>(missive_observer_.GetNextEnqueuedRecord());
   MetricData record_data;
@@ -192,7 +200,7 @@ IN_PROC_BROWSER_TEST_F(
   chromeos::MissiveClientTestObserver missive_observer_(
       Destination::PERIPHERAL_EVENTS);
 
-  cros_healthd::FakeCrosHealthd::Get()->EmitUsbAddEventForTesting();
+  EmitUsbAddEventForTesting();
   std::tuple<Priority, Record> entry =
       missive_observer_.GetNextEnqueuedRecord();
   Record record = std::get<1>(entry);
@@ -253,7 +261,7 @@ IN_PROC_BROWSER_TEST_F(
 
   LoginUnaffiliatedUser();
 
-  cros_healthd::FakeCrosHealthd::Get()->EmitUsbAddEventForTesting();
+  EmitUsbAddEventForTesting();
   EXPECT_TRUE(NoUsbEventsEnqueued(
       chromeos::MissiveClient::Get()->GetTestInterface()->GetEnqueuedRecords(
           Priority::SECURITY)));
@@ -266,7 +274,7 @@ IN_PROC_BROWSER_TEST_F(
 
   LoginAffiliatedUser();
 
-  cros_healthd::FakeCrosHealthd::Get()->EmitUsbAddEventForTesting();
+  EmitUsbAddEventForTesting();
 
   // Shouldn't be any USB event related records in the MissiveClient queue
   EXPECT_TRUE(NoUsbEventsEnqueued(
@@ -281,7 +289,7 @@ IN_PROC_BROWSER_TEST_F(
 
   LoginUnaffiliatedUser();
 
-  cros_healthd::FakeCrosHealthd::Get()->EmitUsbAddEventForTesting();
+  EmitUsbAddEventForTesting();
 
   // Shouldn't be any USB event related records in the MissiveClient queue
   EXPECT_TRUE(NoUsbEventsEnqueued(

diff --git a/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_observer.cc b/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_observer.cc
@@ -11,38 +11,45 @@ namespace reporting {
 using ::ash::cros_healthd::mojom::UsbEventInfoPtr;
 
 UsbEventsObserver::UsbEventsObserver()
-    : MojoServiceEventsObserverBase<
-          ash::cros_healthd::mojom::CrosHealthdUsbObserver>(this) {}
+    : MojoServiceEventsObserverBase<ash::cros_healthd::mojom::EventObserver>(
+          this) {}
 
 UsbEventsObserver::~UsbEventsObserver() = default;
-void UsbEventsObserver::OnAdd(UsbEventInfoPtr info) {
-  MetricData metric_data;
-  metric_data.mutable_event_data()->set_type(MetricEventType::USB_ADDED);
-  FillUsbTelemetry(metric_data.mutable_telemetry_data()
-                       ->mutable_peripherals_telemetry()
-                       ->add_usb_telemetry(),
-                   std::move(info));
-  OnEventObserved(std::move(metric_data));
-}
 
-void UsbEventsObserver::OnRemove(UsbEventInfoPtr info) {
+void UsbEventsObserver::OnEvent(
+    const ash::cros_healthd::mojom::EventInfoPtr info) {
+  if (!info->is_usb_event_info()) {
+    return;
+  }
+  const auto& usb_event_info = info->get_usb_event_info();
   MetricData metric_data;
-  metric_data.mutable_event_data()->set_type(MetricEventType::USB_REMOVED);
+
+  switch (usb_event_info->state) {
+    case ash::cros_healthd::mojom::UsbEventInfo::State::kAdd:
+      metric_data.mutable_event_data()->set_type(MetricEventType::USB_ADDED);
+      break;
+    case ash::cros_healthd::mojom::UsbEventInfo::State::kRemove:
+      metric_data.mutable_event_data()->set_type(MetricEventType::USB_REMOVED);
+      break;
+    case ash::cros_healthd::mojom::UsbEventInfo::State::kUnmappedEnumField:
+      return;
+  }
   FillUsbTelemetry(metric_data.mutable_telemetry_data()
                        ->mutable_peripherals_telemetry()
                        ->add_usb_telemetry(),
-                   std::move(info));
+                   usb_event_info);
   OnEventObserved(std::move(metric_data));
 }
 
 void UsbEventsObserver::AddObserver() {
   ash::cros_healthd::ServiceConnection::GetInstance()
       ->GetEventService()
-      ->AddUsbObserver(BindNewPipeAndPassRemote());
+      ->AddEventObserver(ash::cros_healthd::mojom::EventCategoryEnum::kUsb,
+                         BindNewPipeAndPassRemote());
 }
 
 void UsbEventsObserver::FillUsbTelemetry(UsbTelemetry* data,
-                                         UsbEventInfoPtr info) {
+                                         const UsbEventInfoPtr& info) {
   data->set_vendor(info->vendor);
   data->set_name(info->name);
   data->set_pid(info->pid);

diff --git a/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_observer.h b/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_observer.h
@@ -12,10 +12,9 @@ namespace reporting {
 
 using ::ash::cros_healthd::mojom::UsbEventInfoPtr;
 
-class UsbEventsObserver
-    : public MojoServiceEventsObserverBase<
-          ash::cros_healthd::mojom::CrosHealthdUsbObserver>,
-      public ash::cros_healthd::mojom::CrosHealthdUsbObserver {
+class UsbEventsObserver : public MojoServiceEventsObserverBase<
+                              ash::cros_healthd::mojom::EventObserver>,
+                          public ash::cros_healthd::mojom::EventObserver {
  public:
   UsbEventsObserver();
 
@@ -24,17 +23,15 @@ class UsbEventsObserver
 
   ~UsbEventsObserver() override;
 
-  // ash::cros_healthd::mojom::CrosHealthdUsbObserver:
-  void OnAdd(UsbEventInfoPtr info) override;
-
-  void OnRemove(UsbEventInfoPtr info) override;
+  // ash::cros_healthd::mojom::EventObserver:
+  void OnEvent(const ash::cros_healthd::mojom::EventInfoPtr info) override;
 
  protected:
   // CrosHealthdEventsObserverBase
   void AddObserver() override;
 
  private:
-  void FillUsbTelemetry(UsbTelemetry* data, UsbEventInfoPtr info);
+  void FillUsbTelemetry(UsbTelemetry* data, const UsbEventInfoPtr& info);
 };
 
 }  // namespace reporting

diff --git a/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_observer_unittest.cc b/chrome/browser/ash/policy/reporting/metrics_reporting/usb/usb_events_observer_unittest.cc
@@ -46,12 +46,6 @@ class UsbEventsObserverTest : public ::testing::Test {
 
   void TearDown() override { ::ash::cros_healthd::FakeCrosHealthd::Shutdown(); }
 
-  UsbEventInfoPtr test_usb_event_info = UsbEventInfo::New(kTestVendor,
-                                                          kTestName,
-                                                          kTestVid,
-                                                          kTestPid,
-                                                          kTestCategories);
-
  private:
   base::test::TaskEnvironment task_environment_;
   ::ash::mojo_service_manager::FakeMojoServiceManager fake_service_manager_;
@@ -68,7 +62,10 @@ TEST_F(UsbEventsObserverTest, UsbOnRemove) {
 
   usb_observer.SetOnEventObservedCallback(std::move(cb));
   usb_observer.SetReportingEnabled(true);
-  usb_observer.OnRemove(std::move(test_usb_event_info));
+  usb_observer.OnEvent(
+      ::ash::cros_healthd::mojom::EventInfo::NewUsbEventInfo(UsbEventInfo::New(
+          kTestVendor, kTestName, kTestVid, kTestPid, kTestCategories,
+          ::ash::cros_healthd::mojom::UsbEventInfo::State::kRemove)));
 
   UsbTelemetry usb_telemetry =
       metric_data.telemetry_data().peripherals_telemetry().usb_telemetry(
@@ -107,7 +104,10 @@ TEST_F(UsbEventsObserverTest, UsbOnAdd) {
 
   usb_observer.SetOnEventObservedCallback(std::move(cb));
   usb_observer.SetReportingEnabled(true);
-  usb_observer.OnAdd(std::move(test_usb_event_info));
+  usb_observer.OnEvent(
+      ::ash::cros_healthd::mojom::EventInfo::NewUsbEventInfo(UsbEventInfo::New(
+          kTestVendor, kTestName, kTestVid, kTestPid, kTestCategories,
+          ::ash::cros_healthd::mojom::UsbEventInfo::State::kAdd)));
 
   UsbTelemetry usb_telemetry =
       metric_data.telemetry_data().peripherals_telemetry().usb_telemetry().at(
@@ -144,7 +144,11 @@ TEST_F(UsbEventsObserverTest, UsbOnAddUsingFakeCrosHealthd) {
   usb_observer.SetOnEventObservedCallback(result_metric_data.repeating_cb());
   usb_observer.SetReportingEnabled(true);
 
-  ::ash::cros_healthd::FakeCrosHealthd::Get()->EmitUsbAddEventForTesting();
+  ::ash::cros_healthd::mojom::UsbEventInfo info;
+  info.state = ::ash::cros_healthd::mojom::UsbEventInfo::State::kAdd;
+  ::ash::cros_healthd::FakeCrosHealthd::Get()->EmitEventForCategory(
+      ::ash::cros_healthd::mojom::EventCategoryEnum::kUsb,
+      ::ash::cros_healthd::mojom::EventInfo::NewUsbEventInfo(info.Clone()));
 
   const auto metric_data = result_metric_data.result();
 

diff --git a/chromeos/ash/services/cros_healthd/public/cpp/fake_cros_healthd.cc b/chromeos/ash/services/cros_healthd/public/cpp/fake_cros_healthd.cc
@@ -153,17 +153,6 @@ void FakeCrosHealthd::SetCallbackDelay(base::TimeDelta delay) {
   callback_delay_ = delay;
 }
 
-void FakeCrosHealthd::EmitUsbAddEventForTesting() {
-  // Flush the receiver, so any pending observers are registered before the
-  // event is emitted.
-  event_provider_.FlushForTesting();
-
-  mojom::UsbEventInfo info;
-  for (auto& observer : usb_observers_) {
-    observer->OnAdd(info.Clone());
-  }
-}
-
 void FakeCrosHealthd::EmitEventForCategory(mojom::EventCategoryEnum category,
                                            mojom::EventInfoPtr info) {
   // Flush the receiver, so any pending observers are registered before the
@@ -741,7 +730,7 @@ void FakeCrosHealthd::AddThunderboltObserver(
 
 void FakeCrosHealthd::AddUsbObserver(
     mojo::PendingRemote<mojom::CrosHealthdUsbObserver> observer) {
-  usb_observers_.Add(std::move(observer));
+  NOTREACHED();
 }
 
 void FakeCrosHealthd::AddEventObserver(

diff --git a/chromeos/ash/services/cros_healthd/public/cpp/fake_cros_healthd.h b/chromeos/ash/services/cros_healthd/public/cpp/fake_cros_healthd.h
@@ -149,9 +149,6 @@ class FakeCrosHealthd final : public mojom::CrosHealthdDiagnosticsService,
   // Adds a delay before the passed callback is called.
   void SetCallbackDelay(base::TimeDelta delay);
 
-  // Calls the USB event OnAdd on all registered USB observers.
-  void EmitUsbAddEventForTesting();
-
   // Calls the `OnEvent` method with `info` on all observers registered for
   // `category`.
   void EmitEventForCategory(mojom::EventCategoryEnum category,
@@ -355,8 +352,6 @@ class FakeCrosHealthd final : public mojom::CrosHealthdDiagnosticsService,
   // Collection of registered network observers.
   mojo::RemoteSet<chromeos::network_health::mojom::NetworkEventsObserver>
       network_observers_;
-  // Collection of registered USB observers.
-  mojo::RemoteSet<mojom::CrosHealthdUsbObserver> usb_observers_;
   // Collection of registered general observers grouped by category.
   std::map<mojom::EventCategoryEnum, mojo::RemoteSet<mojom::EventObserver>>
       event_observers_;