Introduce DiagnosticsUi.RoutineCount histogram

This change introduces a count histogram for the number of routines a
user runs during one session in the Diagnostics App. The count starts at
zero each time the app is opened, and it is emitted when the app is
closed.


Bug: 1128204
Change-Id: I5dc7eec4f33006a9694c6c694d5a83a06d59a3a9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2570345
Commit-Queue: Bailey Berro <baileyberro@chromium.org>
Reviewed-by: Caitlin Fischer <caitlinfischer@google.com>
Reviewed-by: Zentaro Kavanagh <zentaro@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840326}

chromeos/components/diagnostics_ui/backend/BUILD.gn

@@ -12,6 +12,8 @@ static_library("backend") {
     "cros_healthd_helpers.h",
     "diagnostics_manager.cc",
     "diagnostics_manager.h",
+    "histogram_util.cc",
+    "histogram_util.h",
     "power_manager_client_conversions.cc",
     "power_manager_client_conversions.h",
     "system_data_provider.cc",

chromeos/components/diagnostics_ui/backend/histogram_util.cc

+// 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.
+
+#include "chromeos/components/diagnostics_ui/backend/histogram_util.h"
+
+#include "base/metrics/histogram_functions.h"
+
+namespace chromeos {
+namespace diagnostics {
+namespace metrics {
+
+void EmitRoutineRunCount(uint16_t routine_count) {
+  base::UmaHistogramCounts100("ChromeOS.DiagnosticsUi.RoutineCount",
+                              routine_count);
+}
+
+}  // namespace metrics
+}  // namespace diagnostics
+}  // namespace chromeos

chromeos/components/diagnostics_ui/backend/histogram_util.h

+// 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.
+
+#ifndef CHROMEOS_COMPONENTS_DIAGNOSTICS_UI_BACKEND_HISTOGRAM_UTIL_H_
+#define CHROMEOS_COMPONENTS_DIAGNOSTICS_UI_BACKEND_HISTOGRAM_UTIL_H_
+
+#include <cstdint>
+
+namespace chromeos {
+namespace diagnostics {
+namespace metrics {
+
+void EmitRoutineRunCount(uint16_t routine_count);
+
+}  // namespace metrics
+}  // namespace diagnostics
+}  // namespace chromeos
+
+#endif  // CHROMEOS_COMPONENTS_DIAGNOSTICS_UI_BACKEND_HISTOGRAM_UTIL_H_

chromeos/components/diagnostics_ui/backend/system_routine_controller.cc

@@ -15,6 +15,7 @@
 #include "base/timer/timer.h"
 #include "base/values.h"
 #include "chromeos/components/diagnostics_ui/backend/cros_healthd_helpers.h"
+#include "chromeos/components/diagnostics_ui/backend/histogram_util.h"
 #include "chromeos/services/cros_healthd/public/cpp/service_connection.h"
 
 namespace chromeos {
@@ -199,17 +200,19 @@ SystemRoutineController::SystemRoutineController() {
 }
 
 SystemRoutineController::~SystemRoutineController() {
-  if (!inflight_routine_runner_) {
-    return;
+  if (inflight_routine_runner_) {
+    // Since SystemRoutineController is torn down at the same time as the
+    // frontend, there's no guarantee that the disconnect handler will be
+    // called. If there's a routine inflight, cancel it but do not pass a
+    // callback.
+    BindCrosHealthdDiagnosticsServiceIfNeccessary();
+    diagnostics_service_->GetRoutineUpdate(
+        inflight_routine_id_, healthd::DiagnosticRoutineCommandEnum::kCancel,
+        /*should_include_output=*/false, base::DoNothing());
   }
 
-  // Since SystemRoutineController is torn down at the same time as the
-  // frontend, there's no guarantee that the disconnect handler will be called.
-  // If there's a routine inflight, cancel it but do not pass a callback.
-  BindCrosHealthdDiagnosticsServiceIfNeccessary();
-  diagnostics_service_->GetRoutineUpdate(
-      inflight_routine_id_, healthd::DiagnosticRoutineCommandEnum::kCancel,
-      /*should_include_output=*/false, base::DoNothing());
+  // Emit the total number of routines run.
+  metrics::EmitRoutineRunCount(routine_count_);
 }
 
 void SystemRoutineController::RunRoutine(
@@ -225,6 +228,8 @@ void SystemRoutineController::RunRoutine(
     return;
   }
 
+  ++routine_count_;
+
   inflight_routine_runner_ =
       mojo::Remote<mojom::RoutineRunner>(std::move(runner));
   inflight_routine_runner_.set_disconnect_handler(base::BindOnce(

chromeos/components/diagnostics_ui/backend/system_routine_controller.h

@@ -119,6 +119,10 @@ class SystemRoutineController : public mojom::SystemRoutineController {
   // routine.
   int32_t inflight_routine_id_ = kInvalidRoutineId;
 
+  // Records the number of routines that a user attempts to run during one
+  // session in the app. Emitted when the app is closed.
+  uint16_t routine_count_ = 0;
+
   mojo::Remote<mojom::RoutineRunner> inflight_routine_runner_;
   std::unique_ptr<base::OneShotTimer> inflight_routine_timer_;
 

chromeos/components/diagnostics_ui/backend/system_routine_controller_unittest.cc

@@ -11,6 +11,7 @@
 #include "base/json/json_writer.h"
 #include "base/run_loop.h"
 #include "base/test/bind.h"
+#include "base/test/metrics/histogram_tester.h"
 #include "base/test/task_environment.h"
 #include "chromeos/dbus/cros_healthd/fake_cros_healthd_client.h"
 #include "chromeos/dbus/cros_healthd/fake_cros_healthd_service.h"
@@ -580,5 +581,52 @@ TEST_F(SystemRoutineControllerTest, CancelRoutineDtor) {
             update_params->command);
 }
 
+TEST_F(SystemRoutineControllerTest, RunRoutineCount0) {
+  base::HistogramTester histogram_tester;
+
+  system_routine_controller_.reset();
+
+  histogram_tester.ExpectBucketCount("ChromeOS.DiagnosticsUi.RoutineCount", 0,
+                                     1);
+}
+
+TEST_F(SystemRoutineControllerTest, RunRoutineCount1) {
+  // Run a routine.
+  SetRunRoutineResponse(/*id=*/1,
+                        healthd::DiagnosticRoutineStatusEnum::kRunning);
+
+  FakeRoutineRunner routine_runner;
+  system_routine_controller_->RunRoutine(
+      mojom::RoutineType::kCpuStress,
+      routine_runner.receiver.BindNewPipeAndPassRemote());
+  base::RunLoop().RunUntilIdle();
+
+  // Assert that the first routine is not complete.
+  EXPECT_TRUE(routine_runner.result.is_null());
+
+  // Update the status on cros_healthd.
+  SetNonInteractiveRoutineUpdateResponse(
+      /*percent_complete=*/100, healthd::DiagnosticRoutineStatusEnum::kPassed,
+      mojo::ScopedHandle());
+
+  // Before the update interval, the routine status is not processed.
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(59));
+  EXPECT_TRUE(routine_runner.result.is_null());
+
+  // After the update interval, the update is fetched and processed.
+  task_environment_.FastForwardBy(base::TimeDelta::FromSeconds(1));
+  EXPECT_FALSE(routine_runner.result.is_null());
+  VerifyRoutineResult(*routine_runner.result, mojom::RoutineType::kCpuStress,
+                      mojom::StandardRoutineResult::kTestPassed);
+
+  // Destroy the SystemRoutineController and check the emitted result.
+  base::HistogramTester histogram_tester;
+
+  system_routine_controller_.reset();
+
+  histogram_tester.ExpectBucketCount("ChromeOS.DiagnosticsUi.RoutineCount", 1,
+                                     1);
+}
+
 }  // namespace diagnostics
 }  // namespace chromeos

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

@@ -314,6 +314,17 @@ incoming reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="ChromeOS.DiagnosticsUi.RoutineCount" units="routines"
+    expires_after="2021-07-15">
+  <owner>baileyberro@chromium.org</owner>
+  <owner>cros-peripherals@google.com</owner>
+  <summary>
+    Records the number of routines run while the Chrome OS Diagnostics App was
+    open. Begins at zero each time the app is open and recorded each time the
+    app is closed.
+  </summary>
+</histogram>
+
 <histogram name="ChromeOS.FamilyLinkUser.LogSegment"
     enum="FamilyLinkUserLogSegment" expires_after="2021-10-12">
   <owner>tobyhuang@chromium.org</owner>