[Telemetry] Introduce the unified onRoutineFinished extension event

In the past, callers of the telemetry extension routine v2 APIs had to
listen to one event for each type of routine. For example, there were
`onMemoryRoutineFinished` and `onVolumeButtonRoutineFinished` events. It
was a pain for extension developers to integrate so many event APIs.

This change unifies all routine finished events into one
`onRoutineFinished` event. Clients can thus listen to only one event,
read the `uuid` to learn which particular routine the event corresponds
to and get the detail of a finished routine.

To achieve this, we translate Mojo unions into dictionaries containing
nullable fields, each field corresponds to a member type of the union.

Bug: b:320394027, b:331540565
Test: unit_tests -ozone-platform=headless --gtest_filter="*Diagnostics*"
Test: browser_tests -ozone-platform=headless --gtest_filter="*Telemetry*"
Change-Id: Ia518c469dd33b485fbec2d1d6725eb2e9ab86aac
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5391170
Commit-Queue: Wei-Luan Wang <weiluanwang@google.com>
Reviewed-by: Ethan Cheng <yycheng@google.com>
Reviewed-by: Kelvin Jiang <kelvinjiang@chromium.org>
Reviewed-by: Denny Huang <dennyh@google.com>
Reviewed-by: Tim <tjudkins@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1280067}

chrome/browser/chromeos/extensions/telemetry/api/routines/diagnostic_routine_converters.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/chromeos/extensions/telemetry/api/routines/diagnostic_routine_converters.h"
 
 #include <cstdint>
+#include <optional>
 #include <vector>
 
 #include "base/notreached.h"
@@ -17,6 +18,30 @@ namespace chromeos::converters::routines {
 namespace {
 namespace cx_diag = api::os_diagnostics;
 namespace crosapi = ::crosapi::mojom;
+
+std::optional<cx_diag::RoutineFinishedDetailUnion> ConvertRoutineDetailUnionPtr(
+    crosapi::TelemetryDiagnosticRoutineDetailPtr input) {
+  if (input.is_null()) {
+    return std::nullopt;
+  }
+  cx_diag::RoutineFinishedDetailUnion detail;
+  switch (input->which()) {
+    case crosapi::TelemetryDiagnosticRoutineDetail::Tag::kUnrecognizedArgument:
+      LOG(WARNING) << "Got unknown routine detail";
+      return std::nullopt;
+    case crosapi::TelemetryDiagnosticRoutineDetail::Tag::kMemory:
+      detail.memory = ConvertPtr(std::move(input->get_memory()));
+      return detail;
+    case crosapi::TelemetryDiagnosticRoutineDetail::Tag::kVolumeButton:
+      // This member type in the union is kept only for backward compatibility.
+      // There is no such a field in the web IDL definition.
+      return std::nullopt;
+    case crosapi::TelemetryDiagnosticRoutineDetail::Tag::kFan:
+      detail.fan = ConvertPtr(std::move(input->get_fan()));
+      return detail;
+  }
+}
+
 }  // namespace
 
 namespace unchecked {
@@ -60,37 +85,60 @@ cx_diag::MemtesterResult UncheckedConvertPtr(
   return result;
 }
 
-cx_diag::MemoryRoutineFinishedInfo UncheckedConvertPtr(
+cx_diag::LegacyMemoryRoutineFinishedInfo UncheckedConvertPtr(
     crosapi::TelemetryDiagnosticMemoryRoutineDetailPtr input,
     base::Uuid uuid,
     bool has_passed) {
-  cx_diag::MemoryRoutineFinishedInfo result;
-
+  cx_diag::LegacyMemoryRoutineFinishedInfo result;
   result.uuid = uuid.AsLowercaseString();
   result.has_passed = has_passed;
-  result.bytes_tested = input->bytes_tested;
-  result.result = ConvertPtr(std::move(input->result));
-
+  // Construct the non-legacy detail to ensure the content is the same between
+  // the legacy and the non-legacy ones.
+  cx_diag::MemoryRoutineFinishedDetail detail =
+      UncheckedConvertPtr(std::move(input));
+  result.bytes_tested = std::move(detail.bytes_tested);
+  result.result = std::move(detail.result);
   return result;
 }
 
-cx_diag::VolumeButtonRoutineFinishedInfo UncheckedConvertPtr(
+cx_diag::LegacyVolumeButtonRoutineFinishedInfo UncheckedConvertPtr(
     crosapi::TelemetryDiagnosticVolumeButtonRoutineDetailPtr input,
     base::Uuid uuid,
     bool has_passed) {
-  cx_diag::VolumeButtonRoutineFinishedInfo result;
-
+  cx_diag::LegacyVolumeButtonRoutineFinishedInfo result;
   result.uuid = uuid.AsLowercaseString();
   result.has_passed = has_passed;
-
   return result;
 }
 
-cx_diag::FanRoutineFinishedInfo UncheckedConvertPtr(
+cx_diag::LegacyFanRoutineFinishedInfo UncheckedConvertPtr(
     crosapi::TelemetryDiagnosticFanRoutineDetailPtr input,
     base::Uuid uuid,
     bool has_passed) {
-  cx_diag::FanRoutineFinishedInfo result;
+  cx_diag::LegacyFanRoutineFinishedInfo result;
+  result.uuid = uuid.AsLowercaseString();
+  result.has_passed = has_passed;
+  // Construct the non-legacy detail to ensure the content is the same between
+  // the legacy and the non-legacy ones.
+  cx_diag::FanRoutineFinishedDetail detail =
+      UncheckedConvertPtr(std::move(input));
+  result.passed_fan_ids = std::move(detail.passed_fan_ids);
+  result.failed_fan_ids = std::move(detail.failed_fan_ids);
+  result.fan_count_status = std::move(detail.fan_count_status);
+  return result;
+}
+
+cx_diag::MemoryRoutineFinishedDetail UncheckedConvertPtr(
+    crosapi::TelemetryDiagnosticMemoryRoutineDetailPtr input) {
+  cx_diag::MemoryRoutineFinishedDetail result;
+  result.bytes_tested = input->bytes_tested;
+  result.result = ConvertPtr(std::move(input->result));
+  return result;
+}
+
+cx_diag::FanRoutineFinishedDetail UncheckedConvertPtr(
+    crosapi::TelemetryDiagnosticFanRoutineDetailPtr input) {
+  cx_diag::FanRoutineFinishedDetail result;
 
   std::vector<int> passed_fan_ids = {};
   for (const auto& passed_fan_id : input->passed_fan_ids) {
@@ -105,8 +153,17 @@ cx_diag::FanRoutineFinishedInfo UncheckedConvertPtr(
   result.failed_fan_ids = failed_fan_ids;
 
   result.fan_count_status = Convert(input->fan_count_status);
+  return result;
+}
+
+cx_diag::RoutineFinishedInfo UncheckedConvertPtr(
+    crosapi::TelemetryDiagnosticRoutineStateFinishedPtr input,
+    base::Uuid uuid,
+    bool has_passed) {
+  cx_diag::RoutineFinishedInfo result;
   result.uuid = uuid.AsLowercaseString();
   result.has_passed = has_passed;
+  result.detail = ConvertRoutineDetailUnionPtr(std::move(input->detail));
   return result;
 }
 

chrome/browser/chromeos/extensions/telemetry/api/routines/diagnostic_routine_converters.h

@@ -40,21 +40,38 @@ api::os_diagnostics::RoutineWaitingInfo UncheckedConvertPtr(
 api::os_diagnostics::MemtesterResult UncheckedConvertPtr(
     crosapi::mojom::TelemetryDiagnosticMemtesterResultPtr input);
 
-api::os_diagnostics::MemoryRoutineFinishedInfo UncheckedConvertPtr(
+// For legacy finished events.
+// TODO(b/331540565): Remove this function after the legacy event is removed.
+api::os_diagnostics::LegacyMemoryRoutineFinishedInfo UncheckedConvertPtr(
     crosapi::mojom::TelemetryDiagnosticMemoryRoutineDetailPtr input,
     base::Uuid uuid,
     bool has_passed);
 
-api::os_diagnostics::VolumeButtonRoutineFinishedInfo UncheckedConvertPtr(
+// For legacy finished events.
+// TODO(b/331540565): Remove this function after the legacy event is removed.
+api::os_diagnostics::LegacyVolumeButtonRoutineFinishedInfo UncheckedConvertPtr(
     crosapi::mojom::TelemetryDiagnosticVolumeButtonRoutineDetailPtr input,
     base::Uuid uuid,
     bool has_passed);
 
-api::os_diagnostics::FanRoutineFinishedInfo UncheckedConvertPtr(
+// For legacy finished events.
+// TODO(b/331540565): Remove this function after the legacy event is removed.
+api::os_diagnostics::LegacyFanRoutineFinishedInfo UncheckedConvertPtr(
     crosapi::mojom::TelemetryDiagnosticFanRoutineDetailPtr input,
     base::Uuid uuid,
     bool has_passed);
 
+api::os_diagnostics::MemoryRoutineFinishedDetail UncheckedConvertPtr(
+    crosapi::mojom::TelemetryDiagnosticMemoryRoutineDetailPtr input);
+
+api::os_diagnostics::FanRoutineFinishedDetail UncheckedConvertPtr(
+    crosapi::mojom::TelemetryDiagnosticFanRoutineDetailPtr input);
+
+api::os_diagnostics::RoutineFinishedInfo UncheckedConvertPtr(
+    crosapi::mojom::TelemetryDiagnosticRoutineStateFinishedPtr input,
+    base::Uuid uuid,
+    bool has_passed);
+
 }  // namespace unchecked
 
 api::os_diagnostics::ExceptionReason Convert(

chrome/browser/chromeos/extensions/telemetry/api/routines/diagnostic_routine_converters_unittest.cc

@@ -71,7 +71,7 @@ TEST(TelemetryExtensionDiagnosticRoutineConvertersTest, RoutineWaitingInfo) {
 }
 
 TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
-     MemoryRoutineFinishedInfo) {
+     LegacyMemoryRoutineFinishedInfo) {
   constexpr bool kHasPassed = true;
   constexpr uint32_t kBytesTested = 42;
   const base::Uuid kUuid = base::Uuid::GenerateRandomV4();
@@ -108,7 +108,7 @@ TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
 }
 
 TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
-     VolumeButtonRoutineFinishedInfo) {
+     LegacyVolumeButtonRoutineFinishedInfo) {
   constexpr bool kHasPassed = true;
   const base::Uuid kUuid = base::Uuid::GenerateRandomV4();
 
@@ -124,7 +124,7 @@ TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
 }
 
 TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
-     FanRoutineFinishedInfo) {
+     LegacyFanRoutineFinishedInfo) {
   auto input = crosapi::TelemetryDiagnosticFanRoutineDetail::New();
   input->passed_fan_ids = {0};
   input->failed_fan_ids = {1};
@@ -164,6 +164,150 @@ TEST(TelemetryExtensionDiagnosticRoutineConvertersTest, MemtesterResult) {
                            cx_diag::MemtesterTestItemEnum::kCompareSub));
 }
 
+TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
+     RoutineFinishedInfoWithoutDetail) {
+  constexpr bool kHasPassed = true;
+  const base::Uuid kUuid = base::Uuid::GenerateRandomV4();
+
+  auto input = crosapi::TelemetryDiagnosticRoutineStateFinished::New();
+  input->detail = nullptr;
+
+  auto result = ConvertPtr(std::move(input), kUuid, kHasPassed);
+
+  ASSERT_TRUE(result.uuid.has_value());
+  EXPECT_EQ(*result.uuid, kUuid.AsLowercaseString());
+
+  ASSERT_TRUE(result.has_passed.has_value());
+  EXPECT_EQ(*result.has_passed, kHasPassed);
+
+  EXPECT_FALSE(result.detail.has_value());
+}
+
+TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
+     RoutineFinishedInfoWithUnrecognizedArgument) {
+  constexpr bool kHasPassed = true;
+  const base::Uuid kUuid = base::Uuid::GenerateRandomV4();
+
+  auto input = crosapi::TelemetryDiagnosticRoutineStateFinished::New();
+  input->detail =
+      crosapi::TelemetryDiagnosticRoutineDetail::NewUnrecognizedArgument(false);
+
+  auto result = ConvertPtr(std::move(input), kUuid, kHasPassed);
+
+  ASSERT_TRUE(result.uuid.has_value());
+  EXPECT_EQ(*result.uuid, kUuid.AsLowercaseString());
+
+  ASSERT_TRUE(result.has_passed.has_value());
+  EXPECT_EQ(*result.has_passed, kHasPassed);
+
+  EXPECT_FALSE(result.detail.has_value());
+}
+
+TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
+     RoutineFinishedInfoWithMemoryDetail) {
+  constexpr bool kHasPassed = true;
+  constexpr uint32_t kBytesTested = 42;
+  const base::Uuid kUuid = base::Uuid::GenerateRandomV4();
+
+  auto detail = crosapi::TelemetryDiagnosticMemoryRoutineDetail::New();
+  detail->bytes_tested = kBytesTested;
+
+  auto memtester_result = crosapi::TelemetryDiagnosticMemtesterResult::New();
+  memtester_result->passed_items = {
+      crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareDIV,
+      crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareMUL};
+  memtester_result->failed_items = {
+      crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareAND,
+      crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareSUB};
+  detail->result = std::move(memtester_result);
+
+  auto input = crosapi::TelemetryDiagnosticRoutineStateFinished::New();
+  input->detail =
+      crosapi::TelemetryDiagnosticRoutineDetail::NewMemory(std::move(detail));
+
+  auto result = ConvertPtr(std::move(input), kUuid, kHasPassed);
+
+  ASSERT_TRUE(result.uuid.has_value());
+  EXPECT_EQ(*result.uuid, kUuid.AsLowercaseString());
+
+  ASSERT_TRUE(result.has_passed.has_value());
+  EXPECT_EQ(*result.has_passed, kHasPassed);
+
+  ASSERT_TRUE(result.detail.has_value());
+  ASSERT_TRUE(result.detail->memory.has_value());
+
+  ASSERT_TRUE(result.detail->memory->bytes_tested.has_value());
+  EXPECT_EQ(*result.detail->memory->bytes_tested, kBytesTested);
+
+  ASSERT_TRUE(result.detail->memory->result.has_value());
+  EXPECT_THAT(
+      result.detail->memory->result->passed_items,
+      testing::ElementsAre(cx_diag::MemtesterTestItemEnum::kCompareDiv,
+                           cx_diag::MemtesterTestItemEnum::kCompareMul));
+  EXPECT_THAT(
+      result.detail->memory->result->failed_items,
+      testing::ElementsAre(cx_diag::MemtesterTestItemEnum::kCompareAnd,
+                           cx_diag::MemtesterTestItemEnum::kCompareSub));
+}
+
+TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
+     RoutineFinishedInfoWithVolumeButtonDetail) {
+  constexpr bool kHasPassed = true;
+  const base::Uuid kUuid = base::Uuid::GenerateRandomV4();
+
+  auto detail = crosapi::TelemetryDiagnosticVolumeButtonRoutineDetail::New();
+
+  auto input = crosapi::TelemetryDiagnosticRoutineStateFinished::New();
+  input->detail = crosapi::TelemetryDiagnosticRoutineDetail::NewVolumeButton(
+      std::move(detail));
+
+  auto result = ConvertPtr(std::move(input), kUuid, kHasPassed);
+
+  ASSERT_TRUE(result.uuid.has_value());
+  EXPECT_EQ(*result.uuid, kUuid.AsLowercaseString());
+
+  ASSERT_TRUE(result.has_passed.has_value());
+  EXPECT_EQ(*result.has_passed, kHasPassed);
+
+  EXPECT_FALSE(result.detail.has_value());
+}
+
+TEST(TelemetryExtensionDiagnosticRoutineConvertersTest,
+     RoutineFinishedInfoWithFanDetail) {
+  constexpr bool kHasPassed = true;
+  const base::Uuid kUuid = base::Uuid::GenerateRandomV4();
+
+  auto detail = crosapi::TelemetryDiagnosticFanRoutineDetail::New();
+  detail->passed_fan_ids = {0};
+  detail->failed_fan_ids = {1};
+  detail->fan_count_status =
+      crosapi::TelemetryDiagnosticHardwarePresenceStatus::kMatched;
+
+  auto input = crosapi::TelemetryDiagnosticRoutineStateFinished::New();
+  input->detail =
+      crosapi::TelemetryDiagnosticRoutineDetail::NewFan(std::move(detail));
+
+  auto result = ConvertPtr(std::move(input), kUuid, kHasPassed);
+
+  ASSERT_TRUE(result.uuid.has_value());
+  EXPECT_EQ(*result.uuid, kUuid.AsLowercaseString());
+
+  ASSERT_TRUE(result.has_passed.has_value());
+  EXPECT_EQ(*result.has_passed, kHasPassed);
+
+  ASSERT_TRUE(result.detail.has_value());
+  ASSERT_TRUE(result.detail->fan.has_value());
+
+  ASSERT_TRUE(result.detail->fan->passed_fan_ids.has_value());
+  EXPECT_THAT(*result.detail->fan->passed_fan_ids, testing::ElementsAre(0));
+
+  ASSERT_TRUE(result.detail->fan->failed_fan_ids.has_value());
+  EXPECT_THAT(*result.detail->fan->failed_fan_ids, testing::ElementsAre(1));
+
+  EXPECT_THAT(result.detail->fan->fan_count_status,
+              cx_diag::HardwarePresenceStatus::kMatched);
+}
+
 TEST(TelemetryExtensionDiagnosticRoutineConvertersTest, ExceptionReason) {
   EXPECT_EQ(
       Convert(crosapi::TelemetryExtensionException::Reason::kUnmappedEnumField),

chrome/browser/chromeos/extensions/telemetry/api/routines/diagnostic_routine_observation.cc

@@ -25,7 +25,8 @@ namespace {
 namespace crosapi = ::crosapi::mojom;
 namespace cx_diag = api::os_diagnostics;
 
-std::unique_ptr<extensions::Event> CreateEventForFinishedVolumeButtonRoutine(
+std::unique_ptr<extensions::Event>
+CreateEventForLegacyFinishedVolumeButtonRoutine(
     bool has_passed,
     base::Uuid uuid,
     content::BrowserContext* browser_context) {
@@ -38,7 +39,7 @@ std::unique_ptr<extensions::Event> CreateEventForFinishedVolumeButtonRoutine(
       base::Value::List().Append(finished_info.ToValue()), browser_context);
 }
 
-std::unique_ptr<extensions::Event> GetEventForFinishedRoutine(
+std::unique_ptr<extensions::Event> GetEventForLegacyFinishedRoutine(
     crosapi::TelemetryDiagnosticRoutineStateFinishedPtr finished,
     base::Uuid uuid,
     content::BrowserContext* browser_context,
@@ -47,8 +48,8 @@ std::unique_ptr<extensions::Event> GetEventForFinishedRoutine(
   // The volume button routine has no detail.
   if (argument_tag_for_legacy_finished_events ==
       crosapi::TelemetryDiagnosticRoutineArgument::Tag::kVolumeButton) {
-    return CreateEventForFinishedVolumeButtonRoutine(finished->has_passed, uuid,
-                                                     browser_context);
+    return CreateEventForLegacyFinishedVolumeButtonRoutine(
+        finished->has_passed, uuid, browser_context);
   }
 
   switch (finished->detail->which()) {
@@ -83,6 +84,19 @@ std::unique_ptr<extensions::Event> GetEventForFinishedRoutine(
   NOTREACHED_NORETURN();
 }
 
+std::unique_ptr<extensions::Event> GetEventForFinishedRoutine(
+    crosapi::TelemetryDiagnosticRoutineStateFinishedPtr finished,
+    base::Uuid uuid,
+    content::BrowserContext* browser_context) {
+  bool has_passed = finished->has_passed;
+  auto finished_info =
+      converters::routines::ConvertPtr(std::move(finished), uuid, has_passed);
+  return std::make_unique<extensions::Event>(
+      extensions::events::OS_DIAGNOSTICS_ON_ROUTINE_FINISHED,
+      cx_diag::OnRoutineFinished::kEventName,
+      base::Value::List().Append(finished_info.ToValue()), browser_context);
+}
+
 }  // namespace
 
 DiagnosticRoutineObservation::DiagnosticRoutineObservation(
@@ -99,6 +113,7 @@ DiagnosticRoutineObservation::~DiagnosticRoutineObservation() = default;
 void DiagnosticRoutineObservation::OnRoutineStateChange(
     crosapi::TelemetryDiagnosticRoutineStatePtr state) {
   std::unique_ptr<extensions::Event> event;
+  std::unique_ptr<extensions::Event> legacy_finished_event;
   switch (state->state_union->which()) {
     case crosapi::TelemetryDiagnosticRoutineStateUnion::Tag::
         kUnrecognizedArgument:
@@ -137,12 +152,13 @@ void DiagnosticRoutineObservation::OnRoutineStateChange(
       break;
     }
     case crosapi::TelemetryDiagnosticRoutineStateUnion::Tag::kFinished: {
+      legacy_finished_event = GetEventForLegacyFinishedRoutine(
+          state->state_union->get_finished().Clone(), info_.uuid,
+          info_.browser_context, info_.argument_tag_for_legacy_finished_events);
       event = GetEventForFinishedRoutine(
           std::move(state->state_union->get_finished()), info_.uuid,
-          info_.browser_context, info_.argument_tag_for_legacy_finished_events);
-      if (!event) {
-        return;
-      }
+          info_.browser_context);
+      break;
     }
   }
 
@@ -152,6 +168,11 @@ void DiagnosticRoutineObservation::OnRoutineStateChange(
   } else {
     extensions::EventRouter::Get(info_.browser_context)
         ->DispatchEventToExtension(info_.extension_id, std::move(event));
+    if (legacy_finished_event) {
+      extensions::EventRouter::Get(info_.browser_context)
+          ->DispatchEventToExtension(info_.extension_id,
+                                     std::move(legacy_finished_event));
+    }
   }
 
   if (state->state_union->is_finished() && on_routine_finished_) {

chrome/browser/chromeos/extensions/telemetry/api/routines/diagnostic_routine_observation_browsertest.cc

@@ -223,7 +223,7 @@ IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
 }
 
 IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
-                       CanObserveOnMemoryRoutineFinished) {
+                       CanObserveLegacyOnMemoryRoutineFinished) {
   SetLegacyFinishedEventRoutineObservation(
       crosapi::TelemetryDiagnosticRoutineArgument::Tag::kMemory);
   RegisterEventObserver(
@@ -294,7 +294,7 @@ IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
 // contain the routine detail.
 IN_PROC_BROWSER_TEST_F(
     TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
-    CanObserveOnVolumeButtonRoutineFinishedWithoutRoutineDetail) {
+    CanObserveLegacyOnVolumeButtonRoutineFinishedWithoutRoutineDetail) {
   SetLegacyFinishedEventRoutineObservation(
       crosapi::TelemetryDiagnosticRoutineArgument::Tag::kVolumeButton);
   RegisterEventObserver(
@@ -337,7 +337,7 @@ IN_PROC_BROWSER_TEST_F(
 // a routine detail.
 IN_PROC_BROWSER_TEST_F(
     TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
-    CanObserveOnVolumeButtonRoutineFinishedWithRoutineDetail) {
+    CanObserveLegacyOnVolumeButtonRoutineFinishedWithRoutineDetail) {
   SetLegacyFinishedEventRoutineObservation(
       crosapi::TelemetryDiagnosticRoutineArgument::Tag::kVolumeButton);
   RegisterEventObserver(
@@ -384,7 +384,7 @@ IN_PROC_BROWSER_TEST_F(
 }
 
 IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
-                       CanObserveOnFanRoutineFinished) {
+                       CanObserveLegacyOnFanRoutineFinished) {
   SetLegacyFinishedEventRoutineObservation(
       crosapi::TelemetryDiagnosticRoutineArgument::Tag::kFan);
   RegisterEventObserver(
@@ -435,4 +435,178 @@ IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
   EXPECT_EQ(info.uuid, uuid_);
 }
 
+IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
+                       CanObserveOnRoutineFinishedWithMemoryDetail) {
+  SetRoutineObservation();
+  RegisterEventObserver(
+      api::os_diagnostics::OnRoutineFinished::kEventName,
+      base::BindLambdaForTesting([this] {
+        auto memtester_result =
+            crosapi::TelemetryDiagnosticMemtesterResult::New();
+        memtester_result->passed_items = {
+            crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareDIV,
+            crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareMUL};
+        memtester_result->failed_items = {
+            crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareAND,
+            crosapi::TelemetryDiagnosticMemtesterTestItemEnum::kCompareSUB};
+
+        auto memory_detail =
+            crosapi::TelemetryDiagnosticMemoryRoutineDetail::New();
+        memory_detail->bytes_tested = 500;
+        memory_detail->result = std::move(memtester_result);
+
+        auto finished_detail =
+            crosapi::TelemetryDiagnosticRoutineDetail::NewMemory(
+                std::move(memory_detail));
+
+        auto finished_state = crosapi::TelemetryDiagnosticRoutineState::New();
+        finished_state->state_union =
+            crosapi::TelemetryDiagnosticRoutineStateUnion::NewFinished(
+                crosapi::TelemetryDiagnosticRoutineStateFinished::New(
+                    /*has_passed=*/true, std::move(finished_detail)));
+        finished_state->percentage = 100;
+
+        remote_->OnRoutineStateChange(std::move(finished_state));
+      }));
+
+  CreateExtensionAndRunServiceWorker(
+      base::StringPrintf(R"(
+    chrome.test.runTests([
+      async function canObserveOnRoutineFinishedWithMemoryDetail() {
+        chrome.os.diagnostics.onRoutineFinished.addListener((event) => {
+          chrome.test.assertEq(event, {
+            "has_passed": true,
+            "detail": {
+              "memory": {
+                "bytesTested": 500,
+                "result": {
+                  "failed_items": [
+                    "compare_and",
+                    "compare_sub"
+                  ],
+                  "passed_items": [
+                    "compare_div",
+                    "compare_mul"
+                  ]
+                },
+              }
+            },
+            "uuid":"%s"
+          });
+
+          chrome.test.succeed();
+        });
+      }
+    ]);
+  )",
+                         uuid_.AsLowercaseString().c_str()));
+
+  auto info = WaitForFinishedReport();
+  EXPECT_EQ(info.extension_id, extension_id());
+  EXPECT_EQ(info.uuid, uuid_);
+}
+
+// In older implementation of healthd, a finished volume button routine contains
+// a routine detail.
+IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
+                       CanObserveOnRoutineFinishedWithVolumeButtonDetail) {
+  SetRoutineObservation();
+  RegisterEventObserver(
+      api::os_diagnostics::OnRoutineFinished::kEventName,
+      base::BindLambdaForTesting([this] {
+        auto volume_button_detail =
+            crosapi::TelemetryDiagnosticVolumeButtonRoutineDetail::New();
+
+        auto finished_detail =
+            crosapi::TelemetryDiagnosticRoutineDetail::NewVolumeButton(
+                std::move(volume_button_detail));
+
+        auto finished_state = crosapi::TelemetryDiagnosticRoutineState::New();
+        finished_state->state_union =
+            crosapi::TelemetryDiagnosticRoutineStateUnion::NewFinished(
+                crosapi::TelemetryDiagnosticRoutineStateFinished::New(
+                    /*has_passed=*/true, std::move(finished_detail)));
+        finished_state->percentage = 100;
+
+        remote_->OnRoutineStateChange(std::move(finished_state));
+      }));
+
+  CreateExtensionAndRunServiceWorker(
+      base::StringPrintf(R"(
+    chrome.test.runTests([
+      async function canObserveOnRoutineFinishedWithVolumeButtonDetail() {
+        chrome.os.diagnostics.onRoutineFinished.addListener(
+          (event) => {
+            chrome.test.assertEq(event, {
+              "has_passed": true,
+              "uuid":"%s"
+            });
+
+            chrome.test.succeed();
+        });
+      }
+    ]);
+  )",
+                         uuid_.AsLowercaseString().c_str()));
+
+  auto info = WaitForFinishedReport();
+  EXPECT_EQ(info.extension_id, extension_id());
+  EXPECT_EQ(info.uuid, uuid_);
+}
+
+IN_PROC_BROWSER_TEST_F(TelemetryExtensionDiagnosticRoutineObserverBrowserTest,
+                       CanObserveOnRoutineFinishedWithFanDetail) {
+  SetRoutineObservation();
+  RegisterEventObserver(
+      api::os_diagnostics::OnRoutineFinished::kEventName,
+      base::BindLambdaForTesting([this] {
+        auto fan_detail = crosapi::TelemetryDiagnosticFanRoutineDetail::New();
+        fan_detail->passed_fan_ids = {0};
+        fan_detail->failed_fan_ids = {1};
+        fan_detail->fan_count_status =
+            crosapi::TelemetryDiagnosticHardwarePresenceStatus::kMatched;
+
+        auto finished_detail =
+            crosapi::TelemetryDiagnosticRoutineDetail::NewFan(
+                std::move(fan_detail));
+
+        auto finished_state = crosapi::TelemetryDiagnosticRoutineState::New();
+        finished_state->state_union =
+            crosapi::TelemetryDiagnosticRoutineStateUnion::NewFinished(
+                crosapi::TelemetryDiagnosticRoutineStateFinished::New(
+                    /*has_passed=*/true, std::move(finished_detail)));
+        finished_state->percentage = 100;
+
+        remote_->OnRoutineStateChange(std::move(finished_state));
+      }));
+
+  CreateExtensionAndRunServiceWorker(
+      base::StringPrintf(R"(
+    chrome.test.runTests([
+      async function canObserveOnRoutineFinishedWithFanDetail() {
+        chrome.os.diagnostics.onRoutineFinished.addListener((event) => {
+          chrome.test.assertEq(event, {
+            "detail": {
+              "fan": {
+                "passed_fan_ids": [0],
+                "failed_fan_ids": [1],
+                "fan_count_status": "matched"
+              }
+            },
+            "has_passed": true,
+            "uuid":"%s"
+          });
+
+          chrome.test.succeed();
+        });
+      }
+    ]);
+  )",
+                         uuid_.AsLowercaseString().c_str()));
+
+  auto info = WaitForFinishedReport();
+  EXPECT_EQ(info.extension_id, extension_id());
+  EXPECT_EQ(info.uuid, uuid_);
+}
+
 }  // namespace chromeos

chrome/common/chromeos/extensions/api/diagnostics.idl

@@ -281,7 +281,14 @@ namespace os.diagnostics {
     MemtesterTestItemEnum[] failed_items;
   };
 
-  dictionary MemoryRoutineFinishedInfo {
+  dictionary MemoryRoutineFinishedDetail {
+    // Number of bytes tested in the memory routine.
+    double? bytesTested;
+    // Contains the memtester test results.
+    MemtesterResult? result;
+  };
+
+  dictionary LegacyMemoryRoutineFinishedInfo {
     DOMString? uuid;
     boolean? has_passed;
     // Number of bytes tested in the memory routine.
@@ -303,7 +310,7 @@ namespace os.diagnostics {
     volume_down
   };
 
-  dictionary VolumeButtonRoutineFinishedInfo {
+  dictionary LegacyVolumeButtonRoutineFinishedInfo {
     DOMString? uuid;
     boolean? has_passed;
   };
@@ -325,7 +332,16 @@ namespace os.diagnostics {
     not_configured
   };
 
-  dictionary FanRoutineFinishedInfo {
+  dictionary FanRoutineFinishedDetail {
+    // The ids of fans that can be controlled.
+    long[]? passed_fan_ids;
+    // The ids of fans that cannot be controlled.
+    long[]? failed_fan_ids;
+    // Whether the number of fan probed is matched.
+    HardwarePresenceStatus? fan_count_status;
+  };
+
+  dictionary LegacyFanRoutineFinishedInfo {
     DOMString? uuid;
     boolean? has_passed;
     // The ids of fans that can be controlled.
@@ -339,6 +355,18 @@ namespace os.diagnostics {
   dictionary RunFanRoutineArguments {
   };
 
+  // This is a union type. Exactly one field should be set.
+  dictionary RoutineFinishedDetailUnion {
+    MemoryRoutineFinishedDetail? memory;
+    FanRoutineFinishedDetail? fan;
+  };
+
+  dictionary RoutineFinishedInfo {
+    DOMString? uuid;
+    boolean? has_passed;
+    RoutineFinishedDetailUnion? detail;
+  };
+
   dictionary CreateRoutineResponse {
     DOMString? uuid;
   };
@@ -524,19 +552,29 @@ namespace os.diagnostics {
     // about what the routine is waiting for (see prototype above).
     static void onRoutineWaiting(RoutineWaitingInfo waitingInfo);
 
-    // Informs the extension that a memory routine finished.
-    static void onMemoryRoutineFinished(MemoryRoutineFinishedInfo finishedInfo);
-
-    // Informs the extension that a volume button routine finished.
-    static void onVolumeButtonRoutineFinished(
-        VolumeButtonRoutineFinishedInfo finishedInfo);
-
-    // Informs the extension that a fan routine finished.
-    static void onFanRoutineFinished(FanRoutineFinishedInfo finishedInfo);
+    // Informs the extension that a routine finished.
+    static void onRoutineFinished(RoutineFinishedInfo finishedInfo);
 
     // Informs the extension that an exception occured. The error passed in
     // `ExceptionInfo` is non-recoverable.
     static void onRoutineException(ExceptionInfo exceptionInfo);
+
+    // ----------------- DEPRECATED DIAGNOSTICS API V2 -----------------
+    // TODO(b/331540565): Remove legacy APIs.
+
+    // Deprecated. Use `onRoutineFinished` instead.
+    // Informs the extension that a memory routine finished.
+    static void onMemoryRoutineFinished(
+        LegacyMemoryRoutineFinishedInfo finishedInfo);
+
+    // Deprecated. Use `onRoutineFinished` instead.
+    // Informs the extension that a volume button routine finished.
+    static void onVolumeButtonRoutineFinished(
+        LegacyVolumeButtonRoutineFinishedInfo finishedInfo);
+
+    // Deprecated. Use `onRoutineFinished` instead.
+    // Informs the extension that a fan routine finished.
+    static void onFanRoutineFinished(LegacyFanRoutineFinishedInfo finishedInfo);
   };
 
   // LINT.ThenChange(//docs/telemetry_extension/api_overview.md)

docs/telemetry_extension/api_overview.md

@@ -279,6 +279,13 @@ extension-event based interface in M119. The interface is described in
 | reason | RoutineWaitingReason | Reason why the routine waits |
 | message | string | Additional information, may be used to pass instruction or explanation |
 
+### RoutineFinishedInfo
+| Property Name | Type | Description |
+------------ | ------- | ----------- |
+| uuid | string | UUID of the routine that entered this state  |
+| has_passed | boolean | Whether the routine finished successfully |
+| detail | RoutineFinishedDetailUnion | Extra details about a finished routine |
+
 ### ExceptionInfo
 | Property Name | Type | Description |
 ------------ | ------- | ----------- |
@@ -286,13 +293,25 @@ extension-event based interface in M119. The interface is described in
 | reason | ExceptionReason | Reason why the routine threw an exception |
 | debugMessage | string | A human readable message for debugging. Don't rely on the content because it could change anytime |
 
+### RoutineFinishedDetailUnion
+| Property Name | Type | Description |
+------------ | ------- | ----------- |
+| memory | MemoryRoutineFinishedDetail | Extra detail for a finished memory routine  |
+| fan | FanRoutineFinishedDetail | Extra detail for a finished fan routine |
+
 ### MemtesterResult
 | Property Name | Type | Description |
 ------------ | ------- | ----------- |
 | passed_items | Array<MemtesterTestItemEnum\> | Passed test items |
 | failed_items | Array<MemtesterTestItemEnum\> | Failed test items |
 
-### MemoryRoutineFinishedInfo
+### MemoryRoutineFinishedDetail
+| Property Name | Type | Description |
+------------ | ------- | ----------- |
+| bytesTested | number | Number of bytes tested in the memory routine |
+| result | MemtesterResult | Contains the memtester test results |
+
+### LegacyMemoryRoutineFinishedInfo
 | Property Name | Type | Description |
 ------------ | ------- | ----------- |
 | uuid | string | UUID of the routine that entered this state  |
@@ -312,7 +331,14 @@ extension-event based interface in M119. The interface is described in
 | not_matched |
 | not_configured |
 
-### FanRoutineFinishedInfo
+### FanRoutineFinishedDetail
+| Property Name | Type | Description |
+------------ | ------- | ----------- |
+| passed_fan_ids | Array<number\> | The ids of fans that can be controlled |
+| failed_fan_ids | Array<number\> | The ids of fans that cannot be controlled |
+| fan_count_status | HardwarePresenceStatus | Whether the number of fan probed is matched |
+
+### LegacyFanRoutineFinishedInfo
 | Property Name | Type | Description |
 ------------ | ------- | ----------- |
 | uuid | string | UUID of the routine that entered this state  |
@@ -331,7 +357,7 @@ extension-event based interface in M119. The interface is described in
 | volume_up |
 | volume_down |
 
-### VolumeButtonRoutineFinishedInfo
+### LegacyVolumeButtonRoutineFinishedInfo
 | Property Name | Type | Description |
 ------------ | ------- | ----------- |
 | uuid | string | UUID of the routine that entered this state  |
@@ -384,9 +410,10 @@ extension-event based interface in M119. The interface is described in
 | onRoutineRunning | function(RoutineRunningInfo) | `os.diagnostics` | M119 | Informs the extension that a routine started running. This can happen in two situations: 1. `startRoutine` was called and the routine successfully started execution. 2. The routine exited the "waiting" state and returned to running |
 | onRoutineWaiting | function(RoutineWaitingInfo) | `os.diagnostics` | M119 | Informs the extension that a routine stopped execution and waits for an event, e.g. user interaction. `RoutineWaitingInfo` contains information about what the routine is waiting for |
 | onRoutineException | function(ExceptionInfo) | `os.diagnostics` | M119 | Informs the extension that an exception occurred. The error passed in `ExceptionInfo` is non-recoverable |
-| onMemoryRoutineFinished | function(MemoryRoutineFinishedInfo) | `os.diagnostics` | M119 | Informs the extension that a memory routine finished |
-| onFanRoutineFinished | function(FanRoutineFinishedInfo) | `os.diagnostics` | M121 | Informs the extension that a fan routine finished |
-| onVolumeButtonRoutineFinished | function(VolumeButtonRoutineFinishedInfo) | `os.diagnostics` | M121 | Informs the extension that a volume button routine finished |
+| onRoutineFinished | function(RoutineFinishedInfo) | `os.diagnostics` | M125 | Informs the extension that a routine finished |
+| onMemoryRoutineFinished | function(LegacyMemoryRoutineFinishedInfo) | `os.diagnostics` | M119 | (Deprecated, use `onRoutineFinished`) Informs the extension that a memory routine finished |
+| onFanRoutineFinished | function(LegacyFanRoutineFinishedInfo) | `os.diagnostics` | M121 | (Deprecated, use `onRoutineFinished`) Informs the extension that a fan routine finished |
+| onVolumeButtonRoutineFinished | function(LegacyVolumeButtonRoutineFinishedInfo) | `os.diagnostics` | M121 | (Deprecated, use `onRoutineFinished`) Informs the extension that a volume button routine finished |
 
 # Events
 

extensions/browser/extension_event_histogram_value.h

@@ -575,6 +575,7 @@ enum HistogramValue {
   PDF_VIEWER_PRIVATE_ON_SAVE = 553,
   ACCESSIBILITY_PRIVATE_ON_SELECT_TO_SPEAK_FOCUS_CHANGED = 554,
   TTS_ON_VOICES_CHANGED = 555,
+  OS_DIAGNOSTICS_ON_ROUTINE_FINISHED = 556,
   // Last entry: Add new entries above, then run:
   // tools/metrics/histograms/update_extension_histograms.py
   ENUM_BOUNDARY

tools/metrics/histograms/metadata/extensions/enums.xml

@@ -752,6 +752,7 @@ Called by update_extension_histograms.py.-->
   <int value="554"
       label="ACCESSIBILITY_PRIVATE_ON_SELECT_TO_SPEAK_FOCUS_CHANGED"/>
   <int value="555" label="TTS_ON_VOICES_CHANGED"/>
+  <int value="556" label="OS_DIAGNOSTICS_ON_ROUTINE_FINISHED"/>
 </enum>
 
 <enum name="ExtensionForceInstalledPreInstalledDeprecatedSite">