ServiceConnection: Add memory routine

Expose a new routine through cros_healthd's ServiceConnection.
This routine runs memtester on all of the device's available
memory (except for 200MB reserved for the OS) to check that
the device's memory is working correctly. The routine is modeled
after crosh's memory_test. Long-term, crosh's memory_test will
be migrated to this new implementation. The migration is blocked
on cros_healthd becoming available on all boards, which should
be done in a couple of weeks. This is part of an effort to
consolidate all existing diagnostic routines into cros_healthd.

Bug: chromium:1113897
Change-Id: I3bb25c1f04c1394e541af9c3c9ef66b8ae6ecd7b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2412956Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Maksim Ivanov <emaxx@chromium.org>
Commit-Queue: Paul Moy <pmoy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808500}

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

@@ -486,6 +486,14 @@ void DeviceCommandRunRoutineJob::RunImpl(CallbackWithResult succeeded_callback,
                   std::move(failed_callback)));
       break;
     }
+    case chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kMemory: {
+      chromeos::cros_healthd::ServiceConnection::GetInstance()
+          ->RunMemoryRoutine(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

@@ -1274,4 +1274,23 @@ TEST_F(DeviceCommandRunRoutineJobTest,
       })));
 }
 
+// Note that the memory routine has no parameters, so we only need to test that
+// it can be run successfully.
+TEST_F(DeviceCommandRunRoutineJobTest, RunMemoryRoutineSuccess) {
+  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);
+  EXPECT_TRUE(
+      RunJob(chromeos::cros_healthd::mojom::DiagnosticRoutineEnum::kMemory,
+             std::move(params_dict),
+             base::BindLambdaForTesting([](RemoteCommandJob* job) {
+               EXPECT_EQ(job->status(), RemoteCommandJob::SUCCEEDED);
+               std::unique_ptr<std::string> payload = job->GetResultPayload();
+               EXPECT_TRUE(payload);
+               EXPECT_EQ(CreateSuccessPayload(kId, kStatus), *payload);
+             })));
+}
+
 }  // namespace policy

chromeos/dbus/cros_healthd/fake_cros_healthd_service.cc

@@ -199,6 +199,14 @@ void FakeCrosHealthdService::RunBatteryChargeRoutine(
       callback_delay_);
 }
 
+void FakeCrosHealthdService::RunMemoryRoutine(
+    RunMemoryRoutineCallback 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

@@ -97,6 +97,7 @@ class FakeCrosHealthdService final
       uint32_t length_seconds,
       uint32_t minimum_charge_percent_required,
       RunBatteryChargeRoutineCallback callback) override;
+  void RunMemoryRoutine(RunMemoryRoutineCallback callback) override;
 
   // CrosHealthdEventService overrides:
   void AddBluetoothObserver(

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

@@ -101,6 +101,9 @@ class ServiceConnectionImpl : public ServiceConnection {
       uint32_t minimum_charge_percent_required,
       mojom::CrosHealthdDiagnosticsService::RunBatteryChargeRoutineCallback
           callback) override;
+  void RunMemoryRoutine(
+      mojom::CrosHealthdDiagnosticsService::RunMemoryRoutineCallback callback)
+      override;
   void AddBluetoothObserver(
       mojo::PendingRemote<mojom::CrosHealthdBluetoothObserver> pending_observer)
       override;
@@ -343,6 +346,13 @@ void ServiceConnectionImpl::RunBatteryChargeRoutine(
       std::move(callback));
 }
 
+void ServiceConnectionImpl::RunMemoryRoutine(
+    mojom::CrosHealthdDiagnosticsService::RunMemoryRoutineCallback callback) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  BindCrosHealthdDiagnosticsServiceIfNeeded();
+  cros_healthd_diagnostics_service_->RunMemoryRoutine(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

@@ -172,6 +172,13 @@ class ServiceConnection {
       mojom::CrosHealthdDiagnosticsService::RunBatteryChargeRoutineCallback
           callback) = 0;
 
+  // Requests that cros_healthd runs the memory routine. See
+  // src/chromeos/service/cros_healthd/public/mojom/cros_healthd.mojom for
+  // details.
+  virtual void RunMemoryRoutine(
+      mojom::CrosHealthdDiagnosticsService::RunMemoryRoutineCallback
+          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

@@ -502,6 +502,19 @@ TEST_F(CrosHealthdServiceConnectionTest, RunBatteryChargeRoutine) {
   run_loop.Run();
 }
 
+// Test that we can run the memory routine.
+TEST_F(CrosHealthdServiceConnectionTest, RunMemoryRoutine) {
+  auto response = MakeRunRoutineResponse();
+  FakeCrosHealthdClient::Get()->SetRunRoutineResponseForTesting(response);
+  base::RunLoop run_loop;
+  ServiceConnection::GetInstance()->RunMemoryRoutine(
+      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

@@ -297,6 +297,15 @@ interface CrosHealthdDiagnosticsService {
   RunBatteryChargeRoutine(uint32 length_seconds,
                           uint32 minimum_charge_percent_required)
       => (RunRoutineResponse response);
+
+  // Requests that the Memory routine is created and started on the platform.
+  // This routine checks that the device's memory is working correctly. This
+  // routine is only available if GetAvailableRoutines returned kMemory.
+  //
+  // The response:
+  // * |response| - contains a unique identifier and status for the created
+  //                routine.
+  RunMemoryRoutine() => (RunRoutineResponse response);
 };
 
 // Event interface exposed by the cros_healthd daemon.

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

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