Migrate engine_file_requests.mojom to the new Mojo types

Convert the implementation and all users of the
chrome_cleaner::mojom::EngineFileRequests interface.

Bug: 955171
Change-Id: Ia3c314bb521e11a60c3562b976f4a70870977a1a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1852248Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Commit-Queue: Henrique Ferreiro <hferreiro@igalia.com>
Cr-Commit-Position: refs/heads/master@{#707956}

chrome/chrome_cleaner/engines/broker/engine_client.cc

@@ -290,10 +290,10 @@ void EngineClient::InitializeAsync(InitializeCallback result_callback) {
   if (interface_metadata_observer_)
     interface_metadata_observer_->ObserveCall(CURRENT_FILE_AND_METHOD);
 
-  // Create a binding to the EngineFileRequests interface that will receive file
-  // reading requests from Initialize.
-  mojom::EngineFileRequestsAssociatedPtrInfo file_requests_info;
-  sandbox_file_requests_->Bind(&file_requests_info);
+  // Create a receiver to the EngineFileRequests interface that will receive
+  // file reading requests from Initialize.
+  mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests;
+  sandbox_file_requests_->Bind(&file_requests);
 
   // Expose the logging directory for writing debug logs.
   base::FilePath logging_path;
@@ -303,7 +303,7 @@ void EngineClient::InitializeAsync(InitializeCallback result_callback) {
 #endif
 
   (*engine_commands_)
-      ->Initialize(std::move(file_requests_info), logging_path,
+      ->Initialize(std::move(file_requests), logging_path,
                    CallbackWithErrorHandling(std::move(result_callback)));
 }
 
@@ -345,15 +345,14 @@ void EngineClient::StartScanAsync(
     EngineClient::FoundUwSCallback found_callback,
     EngineClient::DoneCallback done_callback,
     StartScanCallback result_callback) {
-  // Create bindings to receive the requests sent from the sandboxed
-  // code.
-  mojom::EngineFileRequestsAssociatedPtrInfo file_requests_info;
-  sandbox_file_requests_->Bind(&file_requests_info);
+  // Create receiver to receive the requests sent from the sandboxed code.
+  mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests;
+  sandbox_file_requests_->Bind(&file_requests);
 
   mojom::EngineRequestsAssociatedPtrInfo engine_requests_info;
   sandbox_requests_->Bind(&engine_requests_info);
 
-  // Create a binding to the EngineScanResults interface that will receive
+  // Create a receiver to the EngineScanResults interface that will receive
   // results and pass them on to |found_callback| and |done_callback|.
   mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results;
 
@@ -369,8 +368,8 @@ void EngineClient::StartScanAsync(
   // |done_callback|) with further results.
   (*engine_commands_)
       ->StartScan(enabled_uws, enabled_locations, include_details,
-                  std::move(file_requests_info),
-                  std::move(engine_requests_info), std::move(scan_results),
+                  std::move(file_requests), std::move(engine_requests_info),
+                  std::move(scan_results),
                   CallbackWithErrorHandling(std::move(result_callback)));
 }
 
@@ -408,10 +407,9 @@ uint32_t EngineClient::StartCleanup(const std::vector<UwSId>& enabled_uws,
 void EngineClient::StartCleanupAsync(const std::vector<UwSId>& enabled_uws,
                                      EngineClient::DoneCallback done_callback,
                                      StartCleanupCallback result_callback) {
-  // Create bindings to receive the requests sent from the sandboxed
-  // code.
-  mojom::EngineFileRequestsAssociatedPtrInfo file_requests_info;
-  sandbox_file_requests_->Bind(&file_requests_info);
+  // Create receiver to receive the requests sent from the sandboxed code.
+  mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests;
+  sandbox_file_requests_->Bind(&file_requests);
 
   mojom::EngineRequestsAssociatedPtrInfo engine_requests_info;
   sandbox_requests_->Bind(&engine_requests_info);
@@ -419,8 +417,8 @@ void EngineClient::StartCleanupAsync(const std::vector<UwSId>& enabled_uws,
   mojom::CleanerEngineRequestsAssociatedPtrInfo cleaner_engine_requests_info;
   sandbox_cleaner_requests_->Bind(&cleaner_engine_requests_info);
 
-  // Create a binding to the EngineCleanupResults interface that will
-  // receive results and pass them on to |done_callback|.
+  // Create a receiver to the EngineCleanupResults interface that will receive
+  // results and pass them on to |done_callback|.
   mojo::PendingAssociatedRemote<mojom::EngineCleanupResults> cleanup_results;
   cleanup_results_impl_->BindToCallbacks(&cleanup_results,
                                          std::move(done_callback));
@@ -429,7 +427,7 @@ void EngineClient::StartCleanupAsync(const std::vector<UwSId>& enabled_uws,
     interface_metadata_observer_->ObserveCall(CURRENT_FILE_AND_METHOD);
 
   (*engine_commands_)
-      ->StartCleanup(enabled_uws, std::move(file_requests_info),
+      ->StartCleanup(enabled_uws, std::move(file_requests),
                      std::move(engine_requests_info),
                      std::move(cleaner_engine_requests_info),
                      std::move(cleanup_results),

chrome/chrome_cleaner/engines/broker/engine_file_requests_impl.cc

@@ -34,16 +34,14 @@ EngineFileRequestsImpl::EngineFileRequestsImpl(
     scoped_refptr<MojoTaskRunner> mojo_task_runner,
     InterfaceMetadataObserver* metadata_observer)
     : mojo_task_runner_(mojo_task_runner),
-      metadata_observer_(metadata_observer),
-      binding_(this) {}
+      metadata_observer_(metadata_observer) {}
 
 void EngineFileRequestsImpl::Bind(
-    mojom::EngineFileRequestsAssociatedPtrInfo* ptr_info) {
-  if (binding_.is_bound())
-    binding_.Unbind();
+    mojo::PendingAssociatedRemote<mojom::EngineFileRequests>* remote) {
+  receiver_.reset();
 
-  binding_.Bind(mojo::MakeRequest(ptr_info));
-  // There's no need to call set_connection_error_handler on this since it's an
+  receiver_.Bind(remote->InitWithNewEndpointAndPassReceiver());
+  // There's no need to call set_disconnect_handler on this since it's an
   // associated interface. Any errors will be handled on the main EngineCommands
   // interface.
 }

chrome/chrome_cleaner/engines/broker/engine_file_requests_impl.h

@@ -6,9 +6,10 @@
 #define CHROME_CHROME_CLEANER_ENGINES_BROKER_ENGINE_FILE_REQUESTS_IMPL_H_
 
 #include "chrome/chrome_cleaner/engines/broker/interface_metadata_observer.h"
-#include "chrome/chrome_cleaner/mojom/engine_file_requests.mojom.h"
 #include "chrome/chrome_cleaner/ipc/mojo_task_runner.h"
-#include "mojo/public/cpp/bindings/associated_binding.h"
+#include "chrome/chrome_cleaner/mojom/engine_file_requests.mojom.h"
+#include "mojo/public/cpp/bindings/associated_receiver.h"
+#include "mojo/public/cpp/bindings/pending_associated_remote.h"
 
 namespace chrome_cleaner {
 
@@ -19,7 +20,7 @@ class EngineFileRequestsImpl : public mojom::EngineFileRequests {
       InterfaceMetadataObserver* metadata_observer = nullptr);
   ~EngineFileRequestsImpl() override;
 
-  void Bind(mojom::EngineFileRequestsAssociatedPtrInfo* ptr_info);
+  void Bind(mojo::PendingAssociatedRemote<mojom::EngineFileRequests>* remote);
 
   // mojom::EngineFileRequests
   void SandboxFindFirstFile(
@@ -48,7 +49,7 @@ class EngineFileRequestsImpl : public mojom::EngineFileRequests {
 
   scoped_refptr<MojoTaskRunner> mojo_task_runner_;
   InterfaceMetadataObserver* metadata_observer_ = nullptr;
-  mojo::AssociatedBinding<mojom::EngineFileRequests> binding_;
+  mojo::AssociatedReceiver<mojom::EngineFileRequests> receiver_{this};
 };
 
 }  // namespace chrome_cleaner

chrome/chrome_cleaner/engines/target/engine_commands_impl.cc

@@ -20,8 +20,6 @@ namespace chrome_cleaner {
 
 using mojom::CleanerEngineRequestsAssociatedPtr;
 using mojom::CleanerEngineRequestsAssociatedPtrInfo;
-using mojom::EngineFileRequestsAssociatedPtr;
-using mojom::EngineFileRequestsAssociatedPtrInfo;
 using mojom::EngineRequestsAssociatedPtr;
 using mojom::EngineRequestsAssociatedPtrInfo;
 
@@ -62,17 +60,15 @@ EngineCommandsImpl::EngineCommandsImpl(
 EngineCommandsImpl::~EngineCommandsImpl() = default;
 
 void EngineCommandsImpl::Initialize(
-    mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
+    mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
     const base::FilePath& log_directory_path,
     InitializeCallback callback) {
   ScopedCrashStageRecorder crash_stage(__func__);
 
   // Create proxies to pass requests to the broker process over Mojo.
-  mojom::EngineFileRequestsAssociatedPtr file_requests_ptr;
-  file_requests_ptr.Bind(std::move(file_requests));
   scoped_refptr<EngineFileRequestsProxy> file_requests_proxy =
-      base::MakeRefCounted<EngineFileRequestsProxy>(
-          std::move(file_requests_ptr), task_runner_);
+      base::MakeRefCounted<EngineFileRequestsProxy>(std::move(file_requests),
+                                                    task_runner_);
 
   // This object is not retained because it outlives the callback: it's
   // destroyed on this sequence, once the main thread returns, which should only
@@ -89,18 +85,16 @@ void EngineCommandsImpl::StartScan(
     const std::vector<UwSId>& enabled_uws,
     const std::vector<UwS::TraceLocation>& enabled_trace_locations,
     bool include_details,
-    mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
+    mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
     mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
     mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results,
     StartScanCallback callback) {
   ScopedCrashStageRecorder crash_stage(__func__);
 
   // Create proxies to pass requests to the broker process over Mojo.
-  EngineFileRequestsAssociatedPtr file_requests_ptr;
-  file_requests_ptr.Bind(std::move(file_requests));
   scoped_refptr<EngineFileRequestsProxy> file_requests_proxy =
-      base::MakeRefCounted<EngineFileRequestsProxy>(
-          std::move(file_requests_ptr), task_runner_);
+      base::MakeRefCounted<EngineFileRequestsProxy>(std::move(file_requests),
+                                                    task_runner_);
 
   EngineRequestsAssociatedPtr engine_requests_ptr;
   engine_requests_ptr.Bind(std::move(sandboxed_engine_requests));
@@ -122,7 +116,7 @@ void EngineCommandsImpl::StartScan(
 
 void EngineCommandsImpl::StartCleanup(
     const std::vector<UwSId>& enabled_uws,
-    mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
+    mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
     mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
     mojom::CleanerEngineRequestsAssociatedPtrInfo
         sandboxed_cleaner_engine_requests,
@@ -131,11 +125,9 @@ void EngineCommandsImpl::StartCleanup(
   ScopedCrashStageRecorder crash_stage(__func__);
 
   // Create proxies to pass requests to the broker process over Mojo.
-  EngineFileRequestsAssociatedPtr file_requests_ptr;
-  file_requests_ptr.Bind(std::move(file_requests));
   scoped_refptr<EngineFileRequestsProxy> file_requests_proxy =
-      base::MakeRefCounted<EngineFileRequestsProxy>(
-          std::move(file_requests_ptr), task_runner_);
+      base::MakeRefCounted<EngineFileRequestsProxy>(std::move(file_requests),
+                                                    task_runner_);
 
   EngineRequestsAssociatedPtr engine_requests_ptr;
   engine_requests_ptr.Bind(std::move(sandboxed_engine_requests));

chrome/chrome_cleaner/engines/target/engine_commands_impl.h

@@ -32,20 +32,21 @@ class EngineCommandsImpl : public mojom::EngineCommands {
 
   // mojom::EngineCommands
 
-  void Initialize(mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
-                  const base::FilePath& log_directory,
-                  InitializeCallback callback) override;
+  void Initialize(
+      mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
+      const base::FilePath& log_directory,
+      InitializeCallback callback) override;
   void StartScan(
       const std::vector<UwSId>& enabled_uws,
       const std::vector<UwS::TraceLocation>& enabled_trace_locations,
       bool include_details,
-      mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
+      mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
       mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
       mojo::PendingAssociatedRemote<mojom::EngineScanResults> scan_results,
       StartScanCallback callback) override;
   void StartCleanup(
       const std::vector<UwSId>& enabled_uws,
-      mojom::EngineFileRequestsAssociatedPtrInfo file_requests,
+      mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
       mojom::EngineRequestsAssociatedPtrInfo sandboxed_engine_requests,
       mojom::CleanerEngineRequestsAssociatedPtrInfo
           sandboxed_cleaner_engine_requests,

chrome/chrome_cleaner/engines/target/engine_file_requests_proxy.cc

@@ -64,10 +64,9 @@ void SaveOpenReadOnlyFileCallback(base::win::ScopedHandle* result_holder,
 }  // namespace
 
 EngineFileRequestsProxy::EngineFileRequestsProxy(
-    mojom::EngineFileRequestsAssociatedPtr file_requests_ptr,
+    mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner)
-    : file_requests_ptr_(std::move(file_requests_ptr)),
-      task_runner_(task_runner) {}
+    : file_requests_(std::move(file_requests)), task_runner_(task_runner) {}
 
 EngineFileRequestsProxy::~EngineFileRequestsProxy() = default;
 
@@ -144,19 +143,19 @@ base::win::ScopedHandle EngineFileRequestsProxy::OpenReadOnlyFile(
   return handle;
 }
 
-void EngineFileRequestsProxy::UnbindRequestsPtr() {
-  file_requests_ptr_.reset();
+void EngineFileRequestsProxy::UnbindRequestsRemote() {
+  file_requests_.reset();
 }
 
 MojoCallStatus EngineFileRequestsProxy::SandboxFindFirstFile(
     const base::FilePath& path,
     mojom::EngineFileRequests::SandboxFindFirstFileCallback result_callback) {
-  if (!file_requests_ptr_.is_bound()) {
+  if (!file_requests_.is_bound()) {
     LOG(ERROR) << "SandboxFindFirstFile called without bound pointer";
     return MojoCallStatus::Failure(SandboxErrorCode::INTERNAL_ERROR);
   }
 
-  file_requests_ptr_->SandboxFindFirstFile(path, std::move(result_callback));
+  file_requests_->SandboxFindFirstFile(path, std::move(result_callback));
 
   return MojoCallStatus::Success();
 }
@@ -164,15 +163,15 @@ MojoCallStatus EngineFileRequestsProxy::SandboxFindFirstFile(
 MojoCallStatus EngineFileRequestsProxy::SandboxFindNextFile(
     FindFileHandle handle,
     mojom::EngineFileRequests::SandboxFindNextFileCallback result_callback) {
-  if (!file_requests_ptr_.is_bound()) {
+  if (!file_requests_.is_bound()) {
     LOG(ERROR) << "SandboxFindNextFile called without bound pointer";
     return MojoCallStatus::Failure(SandboxErrorCode::INTERNAL_ERROR);
   }
 
   auto find_handle = mojom::FindHandle::New();
   find_handle->find_handle = handle;
-  file_requests_ptr_->SandboxFindNextFile(std::move(find_handle),
-                                          std::move(result_callback));
+  file_requests_->SandboxFindNextFile(std::move(find_handle),
+                                      std::move(result_callback));
 
   return MojoCallStatus::Success();
 }
@@ -180,15 +179,15 @@ MojoCallStatus EngineFileRequestsProxy::SandboxFindNextFile(
 MojoCallStatus EngineFileRequestsProxy::SandboxFindClose(
     FindFileHandle handle,
     mojom::EngineFileRequests::SandboxFindCloseCallback result_callback) {
-  if (!file_requests_ptr_.is_bound()) {
+  if (!file_requests_.is_bound()) {
     LOG(ERROR) << "SandboxFindClose called without bound pointer";
     return MojoCallStatus::Failure(SandboxErrorCode::INTERNAL_ERROR);
   }
 
   auto find_handle = mojom::FindHandle::New();
   find_handle->find_handle = handle;
-  file_requests_ptr_->SandboxFindClose(std::move(find_handle),
-                                       std::move(result_callback));
+  file_requests_->SandboxFindClose(std::move(find_handle),
+                                   std::move(result_callback));
 
   return MojoCallStatus::Success();
 }
@@ -198,13 +197,13 @@ MojoCallStatus EngineFileRequestsProxy::SandboxOpenReadOnlyFile(
     uint32_t dwFlagsAndAttributes,
     mojom::EngineFileRequests::SandboxOpenReadOnlyFileCallback
         result_callback) {
-  if (!file_requests_ptr_.is_bound()) {
+  if (!file_requests_.is_bound()) {
     LOG(ERROR) << "SandboxOpenReadOnlyFile called without bound pointer";
     return MojoCallStatus::Failure(SandboxErrorCode::INTERNAL_ERROR);
   }
 
-  file_requests_ptr_->SandboxOpenReadOnlyFile(path, dwFlagsAndAttributes,
-                                              std::move(result_callback));
+  file_requests_->SandboxOpenReadOnlyFile(path, dwFlagsAndAttributes,
+                                          std::move(result_callback));
 
   return MojoCallStatus::Success();
 }

chrome/chrome_cleaner/engines/target/engine_file_requests_proxy.h

@@ -14,6 +14,8 @@
 #include "base/win/scoped_handle.h"
 #include "chrome/chrome_cleaner/engines/target/sandbox_request_helper.h"
 #include "chrome/chrome_cleaner/mojom/engine_file_requests.mojom.h"
+#include "mojo/public/cpp/bindings/associated_remote.h"
+#include "mojo/public/cpp/bindings/pending_associated_remote.h"
 
 namespace chrome_cleaner {
 
@@ -24,7 +26,7 @@ class EngineFileRequestsProxy
     : public base::RefCountedThreadSafe<EngineFileRequestsProxy> {
  public:
   EngineFileRequestsProxy(
-      mojom::EngineFileRequestsAssociatedPtr file_requests_ptr,
+      mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner);
 
   // Implements synchronous callbacks to be called on arbitrary threads from the
@@ -43,7 +45,7 @@ class EngineFileRequestsProxy
     return task_runner_;
   }
 
-  void UnbindRequestsPtr();
+  void UnbindRequestsRemote();
 
  protected:
   virtual ~EngineFileRequestsProxy();
@@ -70,7 +72,7 @@ class EngineFileRequestsProxy
 
   // A EngineFileRequests that will send the requests over the Mojo
   // connection.
-  mojom::EngineFileRequestsAssociatedPtr file_requests_ptr_;
+  mojo::AssociatedRemote<mojom::EngineFileRequests> file_requests_;
 
   // A task runner for the IPC thread.
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;

chrome/chrome_cleaner/engines/target/sandboxed_test_helpers.cc

@@ -82,7 +82,7 @@ class SandboxChildProcess::FakeEngineDelegate : public EngineDelegate {
       privileged_scan_calls_->UnbindRequestsPtr();
     }
     if (privileged_file_calls_) {
-      privileged_file_calls_->UnbindRequestsPtr();
+      privileged_file_calls_->UnbindRequestsRemote();
     }
   }
 

chrome/chrome_cleaner/engines/target/sandboxed_test_helpers.h

@@ -85,8 +85,8 @@ class MaybeSandboxedParentProcess : public BaseClass {
     engine_commands_->Bind(
         mojo::PendingRemote<mojom::EngineCommands>(std::move(mojo_pipe), 0));
 
-    mojom::EngineFileRequestsAssociatedPtrInfo file_requests_info;
-    file_requests_impl_->Bind(&file_requests_info);
+    mojo::PendingAssociatedRemote<mojom::EngineFileRequests> file_requests;
+    file_requests_impl_->Bind(&file_requests);
 
     // Bind to empty callbacks as we don't care about the result.
     mojom::EngineRequestsAssociatedPtrInfo scanner_info;
@@ -113,7 +113,7 @@ class MaybeSandboxedParentProcess : public BaseClass {
     auto operation_started = base::BindOnce([](uint32_t unused_result_code) {});
     if (requests_to_setup_ == CallbacksToSetup::kFileRequests) {
       (*engine_commands_)
-          ->Initialize(std::move(file_requests_info), base::FilePath(),
+          ->Initialize(std::move(file_requests), base::FilePath(),
                        std::move(operation_started));
 
     } else if (requests_to_setup_ ==
@@ -121,14 +121,14 @@ class MaybeSandboxedParentProcess : public BaseClass {
       (*engine_commands_)
           ->StartScan(/*enabled_uws=*/std::vector<UwSId>{},
                       /*enabled_locations=*/std::vector<UwS::TraceLocation>{},
-                      /*include_details=*/false, std::move(file_requests_info),
+                      /*include_details=*/false, std::move(file_requests),
                       std::move(scanner_info), std::move(scanner_results),
                       std::move(operation_started));
 
     } else if (requests_to_setup_ == CallbacksToSetup::kCleanupRequests) {
       (*engine_commands_)
           ->StartCleanup(/*enabled_uws=*/std::vector<UwSId>(),
-                         std::move(file_requests_info), std::move(scanner_info),
+                         std::move(file_requests), std::move(scanner_info),
                          std::move(cleaner_info), std::move(cleanup_results),
                          std::move(operation_started));
     }

chrome/chrome_cleaner/mojom/engine_sandbox.mojom

@@ -35,7 +35,7 @@ interface EngineCleanupResults {
 // target process.
 interface EngineCommands {
   // Runs the engine's initialization routine.
-  Initialize(associated EngineFileRequests file_requests,
+  Initialize(pending_associated_remote<EngineFileRequests> file_requests,
              FilePath log_directory) => (uint32 result_code);
 
   // Starts scanning the user's system.
@@ -56,7 +56,7 @@ interface EngineCommands {
     array<uint32> enabled_uws,
     array<TraceLocation> enabled_trace_locations,
     bool include_details,
-    associated EngineFileRequests file_requests,
+    pending_associated_remote<EngineFileRequests> file_requests,
     associated EngineRequests sandboxed_engine_requests,
     pending_associated_remote<EngineScanResults> results) =>
       (uint32 result_code);
@@ -70,7 +70,7 @@ interface EngineCommands {
   // interface.
   StartCleanup(
     array<uint32> enabled_uws,
-    associated EngineFileRequests file_requests,
+    pending_associated_remote<EngineFileRequests> file_requests,
     associated EngineRequests sandboxed_engine_requests,
     associated CleanerEngineRequests sandboxed_cleaner_engine_requests,
     pending_associated_remote<EngineCleanupResults> results) =>