ServiceConnection: Add battery charge routine

Expose a new routine through cros_healthd's ServiceConnection.

Bug: chromium:1094018
Change-Id: I25d2b2e1eb6652d127dcfb50925e75950423b6b3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2388432
Commit-Queue: Paul Moy <pmoy@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Cr-Commit-Position: refs/heads/master@{#804085}

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

@@ -456,6 +456,36 @@ void DeviceCommandRunRoutineJob::RunImpl(CallbackWithResult succeeded_callback,
                   std::move(failed_callback)));
       break;
     }
+    case chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryCharge: {
+      constexpr char kLengthSecondsFieldName[] = "lengthSeconds";
+      constexpr char kMinimumChargePercentRequiredFieldName[] =
+          "minimumChargePercentRequired";
+      base::Optional<int> length_seconds =
+          params_dict_.FindIntKey(kLengthSecondsFieldName);
+      base::Optional<int> minimum_charge_percent_required =
+          params_dict_.FindIntKey(kMinimumChargePercentRequiredFieldName);
+      // The battery charge routine expects two integers >= 0.
+      if (!length_seconds.has_value() ||
+          !minimum_charge_percent_required.has_value() ||
+          length_seconds.value() < 0 ||
+          minimum_charge_percent_required.value() < 0) {
+        SYSLOG(ERROR) << "Invalid parameters for BatteryCharge routine.";
+        base::ThreadTaskRunnerHandle::Get()->PostTask(
+            FROM_HERE, base::BindOnce(std::move(failed_callback),
+                                      std::make_unique<Payload>(
+                                          MakeInvalidParametersResponse())));
+        break;
+      }
+      chromeos::cros_healthd::ServiceConnection::GetInstance()
+          ->RunBatteryChargeRoutine(
+              base::TimeDelta::FromSeconds(length_seconds.value()),
+              minimum_charge_percent_required.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

@@ -76,6 +76,11 @@ constexpr char kMaxNumFieldName[] = "maxNum";
 constexpr char kMaximumDischargePercentAllowedFieldName[] =
     "maximumDischargePercentAllowed";
 
+// String constants identifying the parameter field for the battery charge
+// routine.
+constexpr char kMinimumChargePercentRequiredFieldName[] =
+    "minimumChargePercentRequired";
+
 // Dummy values to populate cros_healthd's RunRoutineResponse.
 constexpr uint32_t kId = 11;
 constexpr auto kStatus =
@@ -1179,4 +1184,94 @@ TEST_F(DeviceCommandRunRoutineJobTest,
       })));
 }
 
+// Test that the battery charge routine can be run.
+TEST_F(DeviceCommandRunRoutineJobTest, RunBatteryChargeRoutineSuccess) {
+  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, kPositiveInt);
+  params_dict.SetIntKey(kMinimumChargePercentRequiredFieldName, kPositiveInt);
+  EXPECT_TRUE(RunJob(
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryCharge,
+      std::move(params_dict),
+      base::BindLambdaForTesting([](RemoteCommandJob* job) {
+        EXPECT_EQ(job->status(), RemoteCommandJob::SUCCEEDED);
+        std::unique_ptr<std::string> payload = job->GetResultPayload();
+        ASSERT_TRUE(payload);
+        EXPECT_EQ(CreateSuccessPayload(kId, kStatus), *payload);
+      })));
+}
+
+// Test that leaving out the lengthSeconds parameter causes the battery charge
+// routine to fail.
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryChargeRoutineMissingLengthSeconds) {
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kMinimumChargePercentRequiredFieldName, kPositiveInt);
+  EXPECT_TRUE(RunJob(
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryCharge,
+      std::move(params_dict),
+      base::BindLambdaForTesting([](RemoteCommandJob* job) {
+        EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+        std::unique_ptr<std::string> payload = job->GetResultPayload();
+        EXPECT_TRUE(payload);
+        EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+      })));
+}
+
+// Test that leaving out the minimumChargePercentRequired parameter causes the
+// battery charge routine to fail.
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryChargeRoutineMissingMinimumChargePercentRequired) {
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kLengthSecondsFieldName, kPositiveInt);
+  EXPECT_TRUE(RunJob(
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryCharge,
+      std::move(params_dict),
+      base::BindLambdaForTesting([](RemoteCommandJob* job) {
+        EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+        std::unique_ptr<std::string> payload = job->GetResultPayload();
+        EXPECT_TRUE(payload);
+        EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+      })));
+}
+
+// Test that a negative lengthSeconds parameter causes the battery charge
+// routine to fail.
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryChargeRoutineInvalidLengthSeconds) {
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kLengthSecondsFieldName, kNegativeInt);
+  params_dict.SetIntKey(kMinimumChargePercentRequiredFieldName, kPositiveInt);
+  EXPECT_TRUE(RunJob(
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryCharge,
+      std::move(params_dict),
+      base::BindLambdaForTesting([](RemoteCommandJob* job) {
+        EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+        std::unique_ptr<std::string> payload = job->GetResultPayload();
+        EXPECT_TRUE(payload);
+        EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+      })));
+}
+
+// Test that a negative minimumChargePercentRequired parameter causes the
+// battery charge routine to fail.
+TEST_F(DeviceCommandRunRoutineJobTest,
+       RunBatteryChargeRoutineInvalidMinimumChargePercentRequired) {
+  base::Value params_dict(base::Value::Type::DICTIONARY);
+  params_dict.SetIntKey(kLengthSecondsFieldName, kPositiveInt);
+  params_dict.SetIntKey(kMinimumChargePercentRequiredFieldName, kNegativeInt);
+  EXPECT_TRUE(RunJob(
+      chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kBatteryCharge,
+      std::move(params_dict),
+      base::BindLambdaForTesting([](RemoteCommandJob* job) {
+        EXPECT_EQ(job->status(), RemoteCommandJob::FAILED);
+        std::unique_ptr<std::string> payload = job->GetResultPayload();
+        EXPECT_TRUE(payload);
+        EXPECT_EQ(CreateInvalidParametersFailurePayload(), *payload);
+      })));
+}
+
 }  // namespace policy

chromeos/dbus/cros_healthd/fake_cros_healthd_service.cc

@@ -189,6 +189,16 @@ void FakeCrosHealthdService::RunBatteryDischargeRoutine(
       callback_delay_);
 }
 
+void FakeCrosHealthdService::RunBatteryChargeRoutine(
+    uint32_t length_seconds,
+    uint32_t minimum_charge_percent_required,
+    RunBatteryChargeRoutineCallback callback) {
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE,
+      base::BindOnce(std::move(callback), run_routine_response_.Clone()),
+      callback_delay_);
+}
+
 void FakeCrosHealthdService::AddBluetoothObserver(
     mojom::CrosHealthdBluetoothObserverPtr observer) {
   bluetooth_observers_.Add(observer.PassInterface());

chromeos/dbus/cros_healthd/fake_cros_healthd_service.h

@@ -93,6 +93,10 @@ class FakeCrosHealthdService final
       uint32_t length_seconds,
       uint32_t maximum_discharge_percent_allowed,
       RunBatteryDischargeRoutineCallback callback) override;
+  void RunBatteryChargeRoutine(
+      uint32_t length_seconds,
+      uint32_t minimum_charge_percent_required,
+      RunBatteryChargeRoutineCallback callback) override;
 
   // CrosHealthdEventService overrides:
   void AddBluetoothObserver(

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

@@ -96,6 +96,11 @@ class ServiceConnectionImpl : public ServiceConnection {
       uint32_t maximum_discharge_percent_allowed,
       mojom::CrosHealthdDiagnosticsService::RunBatteryDischargeRoutineCallback
           callback) override;
+  void RunBatteryChargeRoutine(
+      base::TimeDelta exec_duration,
+      uint32_t minimum_charge_percent_required,
+      mojom::CrosHealthdDiagnosticsService::RunBatteryChargeRoutineCallback
+          callback) override;
   void AddBluetoothObserver(
       mojo::PendingRemote<mojom::CrosHealthdBluetoothObserver> pending_observer)
       override;
@@ -325,6 +330,18 @@ void ServiceConnectionImpl::RunBatteryDischargeRoutine(
       std::move(callback));
 }
 
+void ServiceConnectionImpl::RunBatteryChargeRoutine(
+    base::TimeDelta exec_duration,
+    uint32_t minimum_charge_percent_required,
+    mojom::CrosHealthdDiagnosticsService::RunBatteryChargeRoutineCallback
+        callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  BindCrosHealthdDiagnosticsServiceIfNeeded();
+  cros_healthd_diagnostics_service_->RunBatteryChargeRoutine(
+      exec_duration.InSeconds(), minimum_charge_percent_required,
+      std::move(callback));
+}
+
 void ServiceConnectionImpl::AddBluetoothObserver(
     mojo::PendingRemote<mojom::CrosHealthdBluetoothObserver> pending_observer) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

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

@@ -163,6 +163,15 @@ class ServiceConnection {
       mojom::CrosHealthdDiagnosticsService::RunBatteryDischargeRoutineCallback
           callback) = 0;
 
+  // Requests that cros_healthd runs the battery charge routine. See
+  // src/chromeos/service/cros_healthd/public/mojom/cros_healthd.mojom for
+  // details.
+  virtual void RunBatteryChargeRoutine(
+      base::TimeDelta exec_duration,
+      uint32_t minimum_charge_percent_required,
+      mojom::CrosHealthdDiagnosticsService::RunBatteryChargeRoutineCallback
+          callback) = 0;
+
   // Subscribes to cros_healthd's Bluetooth-related events. See
   // src/chromeos/services/cros_healthd/public/mojom/cros_healthd.mojom for
   // details.

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

@@ -487,6 +487,21 @@ TEST_F(CrosHealthdServiceConnectionTest, RunBatteryDischargeRoutine) {
   run_loop.Run();
 }
 
+// Test that we can run the battery charge routine.
+TEST_F(CrosHealthdServiceConnectionTest, RunBatteryChargeRoutine) {
+  auto response = MakeRunRoutineResponse();
+  FakeCrosHealthdClient::Get()->SetRunRoutineResponseForTesting(response);
+  base::RunLoop run_loop;
+  ServiceConnection::GetInstance()->RunBatteryChargeRoutine(
+      /*exec_duration=*/base::TimeDelta::FromSeconds(30),
+      /*minimum_charge_percent_required=*/10,
+      base::BindLambdaForTesting([&](mojom::RunRoutineResponsePtr response) {
+        EXPECT_EQ(response, MakeRunRoutineResponse());
+        run_loop.Quit();
+      }));
+  run_loop.Run();
+}
+
 // Test that we can add a Bluetooth observer.
 TEST_F(CrosHealthdServiceConnectionTest, AddBluetoothObserver) {
   MockCrosHealthdBluetoothObserver observer;

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

@@ -280,6 +280,23 @@ interface CrosHealthdDiagnosticsService {
   RunBatteryDischargeRoutine(uint32 length_seconds,
                              uint32 maximum_discharge_percent_allowed)
       => (RunRoutineResponse response);
+
+  // Requests that the BatteryCharge routine is created and started on the
+  // platform. This routine checks the battery's charge rate over a period of
+  // time. This routine is only available if GetAvailableRoutines returned
+  // kBatteryCharge.
+  //
+  // The request:
+  // * |length_seconds| - length of time to run the routine for.
+  // * |minimum_charge_percent_required| - the routine will fail if the battery
+  //                                       charges by less than this percentage.
+  //
+  // The response:
+  // * |response| - contains a unique identifier and status for the created
+  //                routine.
+  RunBatteryChargeRoutine(uint32 length_seconds,
+                          uint32 minimum_charge_percent_required)
+      => (RunRoutineResponse response);
 };
 
 // Event interface exposed by the cros_healthd daemon.

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

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