ServiceConnection: expose the battery discharge routine

Expose a new diagnostic routine, battery discharge,
through cros_healthd's ServiceConnection. Extend the
RunRoutine remote command to be able to call this
new routine.

Bug: chromium:1063969
Change-Id: I5ae1ef114d3e20c878b3ec375b7f6bb78c86dc7d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2125571
Commit-Queue: Paul Moy <pmoy@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755013}

chrome/browser/chromeos/policy/remote_commands/device_command_run_routine_job.cc

@@ -425,6 +425,37 @@ void DeviceCommandRunRoutineJob::RunImpl(CallbackWithResult succeeded_callback,
                   std::move(failed_callback)));
       break;
     }
+    case chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::
+        kBatteryDischarge: {
+      constexpr char kLengthSecondsFieldName[] = "lengthSeconds";
+      constexpr char kMaximumDischargePercentAllowedFieldName[] =
+          "maximumDischargePercentAllowed";
+      base::Optional<int> length_seconds =
+          params_dict_.FindIntKey(kLengthSecondsFieldName);
+      base::Optional<int> maximum_discharge_percent_allowed =
+          params_dict_.FindIntKey(kMaximumDischargePercentAllowedFieldName);
+      // The battery discharge routine expects two integers >= 0.
+      if (!length_seconds.has_value() ||
+          !maximum_discharge_percent_allowed.has_value() ||
+          length_seconds.value() < 0 ||
+          maximum_discharge_percent_allowed.value() < 0) {
+        SYSLOG(ERROR) << "Invalid parameters for BatteryDischarge routine.";
+        base::ThreadTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(failed_callback),
+                                      std::make_unique<Payload>(
+                                          MakeInvalidParametersResponse())));
+        break;
+      }
+      chromeos::cros_healthd::ServiceConnection::GetInstance()
+          ->RunBatteryDischargeRoutine(
+              base::TimeDelta::FromSeconds(length_seconds.value()),
+              maximum_discharge_percent_allowed.value(),
+              base::BindOnce(
+                  &DeviceCommandRunRoutineJob::OnCrosHealthdResponseReceived,
+                  weak_ptr_factory_.GetWeakPtr(), std::move(succeeded_callback),
+                  std::move(failed_callback)));
+      break;
+    }
   }
 }
 

chrome/browser/chromeos/policy/remote_commands/device_command_run_routine_job_unittest.cc

@@ -71,6 +71,11 @@ constexpr char kFileSizeMbFieldName[] = "fileSizeMb";
 // routine
 constexpr char kMaxNumFieldName[] = "maxNum";
 
+// String constants identifying the parameter field for the battery discharge
+// routine.
+constexpr char kMaximumDischargePercentAllowedFieldName[] =
+    "maximumDischargePercentAllowed";
+
 // Dummy values to populate cros_healthd's RunRoutineResponse.
 constexpr uint32_t kId = 11;
 constexpr chromeos::cros_healthd::mojom::DiagnosticRoutineStatusEnum kStatus =
@@ -1559,4 +1564,146 @@ TEST_F(DeviceCommandRunRoutineJobTest, RunPrimeSearchRoutineInvalidMaxNum) {
   run_loop.Run();
 }
 
+TEST_F(DeviceCommandRunRoutineJobTest, RunBatteryDischargeRoutineSuccess) {
+  auto run_routine_response =
+      chromeos::cros_healthd::mojom::RunRoutineResponse::New(kId, kStatus);
+  chromeos::cros_healthd::FakeCrosHealthdClient::Get()
+      ->SetRunRoutineResponseForTesting(run_routine_response);
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kLengthSecondsFieldName, /*length_seconds=*/10);
+  params_dict.SetIntKey(kMaximumDischargePercentAllowedFieldName,
+                        /*maximum_discharge_percent_allowed=*/76);
+  std::unique_ptr<RemoteCommandJob> job =
+      std::make_unique<DeviceCommandRunRoutineJob>();
+  InitializeJob(
+      job.get(), kUniqueID, test_start_time_, base::TimeDelta::FromSeconds(30),
+      /*terminate_upon_input=*/false,
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryDischarge,
+      std::move(params_dict));
+  base::RunLoop run_loop;
+  bool success =
+      job->Run(base::Time::Now(), base::TimeTicks::Now(),
+               base::BindLambdaForTesting([&]() {
+                 EXPECT_EQ(job->status(), RemoteCommandJob::SUCCEEDED);
+                 std::unique_ptr<std::string> payload = job->GetResultPayload();
+                 EXPECT_TRUE(payload);
+                 EXPECT_EQ(CreateSuccessPayload(kId, kStatus), *payload);
+                 run_loop.Quit();
+               }));
+  EXPECT_TRUE(success);
+  run_loop.Run();
+}
+
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryDischargeRoutineMissingLengthSeconds) {
+  // Test that leaving out the lengthSeconds parameter causes the routine to
+  // fail.
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kMaximumDischargePercentAllowedFieldName,
+                        /*maximum_discharge_percent_allowed=*/76);
+  std::unique_ptr<RemoteCommandJob> job =
+      std::make_unique<DeviceCommandRunRoutineJob>();
+  InitializeJob(
+      job.get(), kUniqueID, test_start_time_, base::TimeDelta::FromSeconds(30),
+      /*terminate_upon_input=*/false,
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryDischarge,
+      std::move(params_dict));
+  base::RunLoop run_loop;
+  bool success =
+      job->Run(base::Time::Now(), base::TimeTicks::Now(),
+               base::BindLambdaForTesting([&]() {
+                 EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+                 std::unique_ptr<std::string> payload = job->GetResultPayload();
+                 EXPECT_TRUE(payload);
+                 EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+                 run_loop.Quit();
+               }));
+  EXPECT_TRUE(success);
+  run_loop.Run();
+}
+
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryDischargeRoutineMissingMaximumDischargePercentAllowed) {
+  // Test that leaving out the maximumDischargePercentAllowed parameter causes
+  // the routine to fail.
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kLengthSecondsFieldName, /*length_seconds=*/10);
+  std::unique_ptr<RemoteCommandJob> job =
+      std::make_unique<DeviceCommandRunRoutineJob>();
+  InitializeJob(
+      job.get(), kUniqueID, test_start_time_, base::TimeDelta::FromSeconds(30),
+      /*terminate_upon_input=*/false,
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryDischarge,
+      std::move(params_dict));
+  base::RunLoop run_loop;
+  bool success =
+      job->Run(base::Time::Now(), base::TimeTicks::Now(),
+               base::BindLambdaForTesting([&]() {
+                 EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+                 std::unique_ptr<std::string> payload = job->GetResultPayload();
+                 EXPECT_TRUE(payload);
+                 EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+                 run_loop.Quit();
+               }));
+  EXPECT_TRUE(success);
+  run_loop.Run();
+}
+
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryDischargeRoutineInvalidLengthSeconds) {
+  // Test that a negative lengthSeconds parameter causes the routine to fail.
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kLengthSecondsFieldName, /*length_seconds=*/-10);
+  params_dict.SetIntKey(kMaximumDischargePercentAllowedFieldName,
+                        /*maximum_discharge_percent_allowed=*/76);
+  std::unique_ptr<RemoteCommandJob> job =
+      std::make_unique<DeviceCommandRunRoutineJob>();
+  InitializeJob(
+      job.get(), kUniqueID, test_start_time_, base::TimeDelta::FromSeconds(30),
+      /*terminate_upon_input=*/false,
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryDischarge,
+      std::move(params_dict));
+  base::RunLoop run_loop;
+  bool success =
+      job->Run(base::Time::Now(), base::TimeTicks::Now(),
+               base::BindLambdaForTesting([&]() {
+                 EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+                 std::unique_ptr<std::string> payload = job->GetResultPayload();
+                 EXPECT_TRUE(payload);
+                 EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+                 run_loop.Quit();
+               }));
+  EXPECT_TRUE(success);
+  run_loop.Run();
+}
+
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryDischargeRoutineInvalidMaximumDischargePercentAllowed) {
+  // Test that a negative maximumDischargePercentAllowed parameter causes the
+  // routine to fail.
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kLengthSecondsFieldName, /*length_seconds=*/10);
+  params_dict.SetIntKey(kMaximumDischargePercentAllowedFieldName,
+                        /*maximum_discharge_percent_allowed=*/-76);
+  std::unique_ptr<RemoteCommandJob> job =
+      std::make_unique<DeviceCommandRunRoutineJob>();
+  InitializeJob(
+      job.get(), kUniqueID, test_start_time_, base::TimeDelta::FromSeconds(30),
+      /*terminate_upon_input=*/false,
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryDischarge,
+      std::move(params_dict));
+  base::RunLoop run_loop;
+  bool success =
+      job->Run(base::Time::Now(), base::TimeTicks::Now(),
+               base::BindLambdaForTesting([&]() {
+                 EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+                 std::unique_ptr<std::string> payload = job->GetResultPayload();
+                 EXPECT_TRUE(payload);
+                 EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+                 run_loop.Quit();
+               }));
+  EXPECT_TRUE(success);
+  run_loop.Run();
+}
+
 }  // namespace policy

chromeos/dbus/cros_healthd/fake_cros_healthd_service.cc

@@ -115,6 +115,13 @@ void FakeCrosHealthdService::RunPrimeSearchRoutine(
   std::move(callback).Run(run_routine_response_.Clone());
 }
 
+void FakeCrosHealthdService::RunBatteryDischargeRoutine(
+    uint32_t length_seconds,
+    uint32_t maximum_discharge_percent_allowed,
+    RunBatteryDischargeRoutineCallback callback) {
+  std::move(callback).Run(run_routine_response_.Clone());
+}
+
 void FakeCrosHealthdService::ProbeTelemetryInfo(
     const std::vector<mojom::ProbeCategoryEnum>& categories,
     ProbeTelemetryInfoCallback callback) {

chromeos/dbus/cros_healthd/fake_cros_healthd_service.h

@@ -73,6 +73,10 @@ class FakeCrosHealthdService final
   void RunPrimeSearchRoutine(uint32_t length_seconds,
                              uint64_t max_num,
                              RunPrimeSearchRoutineCallback callback) override;
+  void RunBatteryDischargeRoutine(
+      uint32_t length_seconds,
+      uint32_t maximum_discharge_percent_allowed,
+      RunBatteryDischargeRoutineCallback callback) override;
 
   // CrosHealthdProbeService overrides:
   void ProbeTelemetryInfo(

chromeos/services/cros_healthd/public/cpp/service_connection.cc

@@ -89,6 +89,11 @@ class ServiceConnectionImpl : public ServiceConnection {
       uint64_t max_num,
       mojom::CrosHealthdDiagnosticsService::RunPrimeSearchRoutineCallback
           callback) override;
+  void RunBatteryDischargeRoutine(
+      base::TimeDelta exec_duration,
+      uint32_t maximum_discharge_percent_allowed,
+      mojom::CrosHealthdDiagnosticsService::RunBatteryDischargeRoutineCallback
+          callback) override;
   void ProbeTelemetryInfo(
       const std::vector<mojom::ProbeCategoryEnum>& categories_to_test,
       mojom::CrosHealthdProbeService::ProbeTelemetryInfoCallback callback)
@@ -267,6 +272,18 @@ void ServiceConnectionImpl::RunPrimeSearchRoutine(
       exec_duration.InSeconds(), max_num, std::move(callback));
 }
 
+void ServiceConnectionImpl::RunBatteryDischargeRoutine(
+    base::TimeDelta exec_duration,
+    uint32_t maximum_discharge_percent_allowed,
+    mojom::CrosHealthdDiagnosticsService::RunBatteryDischargeRoutineCallback
+        callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  BindCrosHealthdDiagnosticsServiceIfNeeded();
+  cros_healthd_diagnostics_service_->RunBatteryDischargeRoutine(
+      exec_duration.InSeconds(), maximum_discharge_percent_allowed,
+      std::move(callback));
+}
+
 void ServiceConnectionImpl::ProbeTelemetryInfo(
     const std::vector<mojom::ProbeCategoryEnum>& categories_to_test,
     mojom::CrosHealthdProbeService::ProbeTelemetryInfoCallback callback) {

chromeos/services/cros_healthd/public/cpp/service_connection.h

@@ -140,6 +140,15 @@ class ServiceConnection {
       mojom::CrosHealthdDiagnosticsService::RunPrimeSearchRoutineCallback
           callback) = 0;
 
+  // Requests that cros_healthd runs the battery discharge routine. See
+  // src/chromeos/service/cros_healthd/public/mojom/cros_healthd.mojom for
+  // details.
+  virtual void RunBatteryDischargeRoutine(
+      base::TimeDelta exec_duration,
+      uint32_t maximum_discharge_percent_allowed,
+      mojom::CrosHealthdDiagnosticsService::RunBatteryDischargeRoutineCallback
+          callback) = 0;
+
   // Gather pieces of information about the platform. See
   // src/chromeos/service/cros_healthd/public/mojom/cros_healthd.mojom for
   // details.

chromeos/services/cros_healthd/public/cpp/service_connection_unittest.cc

@@ -407,6 +407,21 @@ TEST_F(CrosHealthdServiceConnectionTest, RunPrimeSearchRoutine) {
   run_loop.Run();
 }
 
+TEST_F(CrosHealthdServiceConnectionTest, RunBatteryDischargeRoutine) {
+  // Test that we can run the battery discharge routine.
+  auto response = MakeRunRoutineResponse();
+  FakeCrosHealthdClient::Get()->SetRunRoutineResponseForTesting(response);
+  base::RunLoop run_loop;
+  ServiceConnection::GetInstance()->RunBatteryDischargeRoutine(
+      /*exec_duration=*/base::TimeDelta::FromSeconds(12),
+      /*maximum_discharge_percent_allowed=*/99,
+      base::BindLambdaForTesting([&](mojom::RunRoutineResponsePtr response) {
+        EXPECT_EQ(response, MakeRunRoutineResponse());
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+}
+
 TEST_F(CrosHealthdServiceConnectionTest, ProbeTelemetryInfo) {
   // Test that we can send a request without categories.
   auto empty_info = mojom::TelemetryInfo::New();

chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom

@@ -22,7 +22,8 @@ interface CrosHealthdServiceFactory {
   GetProbeService(CrosHealthdProbeService& service);
 };
 
-// Diagnostics interface exposed by the cros_healthd daemon.
+// Diagnostics interface exposed by the cros_healthd daemon. Consumed in Chrome
+// by remote commands, which are made available via the cloud EMM API.
 interface CrosHealthdDiagnosticsService {
   // Returns an array of all diagnostic routines that the platform supports.
   GetAvailableRoutines()
@@ -245,6 +246,24 @@ interface CrosHealthdDiagnosticsService {
   //                routine.
   RunPrimeSearchRoutine(uint32 length_seconds, uint64 max_num)
       => (RunRoutineResponse response);
+
+  // Requests that the BatteryDischarge routine is created and started on the
+  // platform. This routine checks the battery's discharge rate over a period of
+  // time. This routine is only available if GetAvailableRoutines returned
+  // kBatteryDischarge.
+  //
+  // The request:
+  // * |length_seconds| - length of time to run the routine for.
+  // * |maximum_discharge_percent_allowed| - the routine will fail if the
+  //                                         battery discharges by more than
+  //                                         this percentage.
+  //
+  // The response:
+  // * |response| - contains a unique identifier and status for the created
+  //                routine.
+  RunBatteryDischargeRoutine(uint32 length_seconds,
+                             uint32 maximum_discharge_percent_allowed)
+      => (RunRoutineResponse response);
 };
 
 // Probe interface exposed by the cros_healthd daemon.

chromeos/services/cros_healthd/public/mojom/cros_healthd_diagnostics.mojom

@@ -28,6 +28,7 @@ enum DiagnosticRoutineEnum {
   kNvmeSelfTest = 9,
   kDiskRead = 10,
   kPrimeSearch = 11,
+  kBatteryDischarge = 12,
 };
 
 // Enumeration of the possible DiskRead routine's command type