[FilesTrash] Create holder to manage lifetime of trash service

In prepraration for exposing a parseTrashInfoFiles method on
fileManagerPrivate the logic in RestoreIOTask needs to be extracted to
enable re-use of the trash info parsing. This simply extracts the
launching of the service and mojo lifetime management into a standalone
class.

Bug: b:238946031
Test: unit_tests --gtest_filter=*RestoreIOTask*
Change-Id: Ibf0471243a6ba9f58f96f77318469f18d3466e40
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3818689
Reviewed-by: Luciano Pacheco <lucmult@chromium.org>
Commit-Queue: Ben Reich <benreich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1032907}

chrome/browser/ash/file_manager/restore_io_task.cc

@@ -34,10 +34,6 @@ base::File::Error CreateNestedPath(
   return base::File::FILE_OK;
 }
 
-base::File GetReadOnlyFileFromPath(const base::FilePath& path) {
-  return base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
-}
-
 }  // namespace
 
 RestoreIOTask::RestoreIOTask(
@@ -86,11 +82,9 @@ void RestoreIOTask::Execute(IOTask::ProgressCallback progress_callback,
       GenerateEnabledTrashLocationsForProfile(profile_, base_path_);
   progress_.state = State::kInProgress;
 
-  auto trash_pending_remote = chromeos::trash_service::LaunchTrashService();
-  trash_service_ = mojo::Remote<chromeos::trash_service::mojom::TrashService>(
-      std::move(trash_pending_remote));
-  trash_service_.set_disconnect_handler(base::BindOnce(
-      &RestoreIOTask::Complete, weak_ptr_factory_.GetWeakPtr(), State::kError));
+  parser_ = std::make_unique<chromeos::trash_service::TrashInfoParser>(
+      base::BindOnce(&RestoreIOTask::Complete, weak_ptr_factory_.GetWeakPtr(),
+                     State::kError));
 
   ValidateTrashInfo(0);
 }
@@ -169,24 +163,8 @@ void RestoreIOTask::OnTrashedFileExists(
           ? progress_.sources[idx].url.path()
           : base_path_.Append(progress_.sources[idx].url.path());
 
-  base::ThreadPool::PostTaskAndReplyWithResult(
-      FROM_HERE, {base::MayBlock()},
-      base::BindOnce(&GetReadOnlyFileFromPath, trashinfo_path),
-      base::BindOnce(&RestoreIOTask::OnGotFile, weak_ptr_factory_.GetWeakPtr(),
-                     std::move(complete_callback), idx));
-}
-
-void RestoreIOTask::OnGotFile(
-    chromeos::trash_service::ParseTrashInfoCallback callback,
-    size_t idx,
-    base::File file) {
-  if (!file.IsValid()) {
-    LOG(ERROR) << "Supplied file is not valid: " << file.error_details();
-    progress_.sources[idx].error = file.error_details();
-    Complete(State::kError);
-    return;
-  }
-  trash_service_->ParseTrashInfoFile(std::move(file), std::move(callback));
+  parser_->ParseTrashInfoFile(std::move(trashinfo_path),
+                              std::move(complete_callback));
 }
 
 void RestoreIOTask::EnsureParentRestorePathExists(

chrome/browser/ash/file_manager/restore_io_task.h

@@ -11,9 +11,7 @@
 #include "base/memory/weak_ptr.h"
 #include "chrome/browser/ash/file_manager/io_task.h"
 #include "chrome/browser/ash/file_manager/trash_common_util.h"
-#include "chromeos/ash/components/trash_service/public/cpp/trash_service.h"
-#include "chromeos/ash/components/trash_service/public/mojom/trash_service.mojom.h"
-#include "mojo/public/cpp/bindings/remote.h"
+#include "chromeos/ash/components/trash_service/public/cpp/trash_info_parser.h"
 #include "storage/browser/file_system/file_system_context.h"
 #include "storage/browser/file_system/file_system_operation_runner.h"
 #include "storage/browser/file_system/file_system_url.h"
@@ -48,10 +46,6 @@ class RestoreIOTask : public IOTask {
   // Finalises the RestoreIOTask with the `state`.
   void Complete(State state);
 
-  void OnGotFile(chromeos::trash_service::ParseTrashInfoCallback callback,
-                 size_t idx,
-                 base::File file);
-
   // Ensure the metadata file conforms to the following:
   //   - Has a .trashinfo suffix
   //   - Resides in an enabled trash directory
@@ -122,7 +116,7 @@ class RestoreIOTask : public IOTask {
 
   // Holds the connection open to the `TrashService`. This is a sandboxed
   // process that performs parsing of the trashinfo files.
-  mojo::Remote<chromeos::trash_service::mojom::TrashService> trash_service_;
+  std::unique_ptr<chromeos::trash_service::TrashInfoParser> parser_ = nullptr;
 
   ProgressCallback progress_callback_;
   CompleteCallback complete_callback_;

chromeos/ash/components/trash_service/public/cpp/BUILD.gn

@@ -8,6 +8,8 @@ assert(is_chromeos_ash, "Non-ChromeOS builds cannot depend on //chromeos/ash")
 
 source_set("cpp") {
   sources = [
+    "trash_info_parser.cc",
+    "trash_info_parser.h",
     "trash_service.cc",
     "trash_service.h",
   ]

chromeos/ash/components/trash_service/public/cpp/trash_info_parser.cc

@@ -0,0 +1,57 @@
+// Copyright 2022 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/ash/components/trash_service/public/cpp/trash_info_parser.h"
+
+#include "base/files/file.h"
+#include "base/task/task_traits.h"
+#include "base/task/thread_pool.h"
+#include "base/time/time.h"
+
+namespace chromeos::trash_service {
+
+namespace {
+
+base::File GetReadOnlyFileFromPath(const base::FilePath& path) {
+  return base::File(path, base::File::FLAG_OPEN | base::File::FLAG_READ);
+}
+
+}  // namespace
+
+TrashInfoParser::TrashInfoParser(
+    base::OnceCallback<void()> disconnect_callback) {
+  auto trash_pending_remote = LaunchTrashService();
+  service_ = mojo::Remote<mojom::TrashService>(std::move(trash_pending_remote));
+  service_.set_disconnect_handler(std::move(disconnect_callback));
+}
+
+TrashInfoParser::~TrashInfoParser() = default;
+
+void TrashInfoParser::ParseTrashInfoFile(const base::FilePath& path,
+                                         ParseTrashInfoCallback callback) {
+  if (!service_) {
+    LOG(ERROR) << "Trash service is not connected";
+    std::move(callback).Run(base::File::FILE_ERROR_FAILED, base::FilePath(),
+                            base::Time());
+    return;
+  }
+  base::ThreadPool::PostTaskAndReplyWithResult(
+      FROM_HERE, {base::MayBlock()},
+      base::BindOnce(&GetReadOnlyFileFromPath, std::move(path)),
+      base::BindOnce(&TrashInfoParser::OnGotFile,
+                     weak_ptr_factory_.GetWeakPtr(), std::move(callback)));
+}
+
+void TrashInfoParser::OnGotFile(ParseTrashInfoCallback callback,
+                                base::File file) {
+  if (!file.IsValid()) {
+    LOG(ERROR) << "Trash info file is not valid " << file.error_details();
+    std::move(callback).Run(file.error_details(), base::FilePath(),
+                            base::Time());
+    return;
+  }
+  service_->ParseTrashInfoFile(std::move(file), std::move(callback));
+}
+
+}  // namespace chromeos::trash_service

chromeos/ash/components/trash_service/public/cpp/trash_info_parser.h

@@ -0,0 +1,39 @@
+// Copyright 2022 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_ASH_COMPONENTS_TRASH_SERVICE_PUBLIC_CPP_TRASH_INFO_PARSER_H_
+#define CHROMEOS_ASH_COMPONENTS_TRASH_SERVICE_PUBLIC_CPP_TRASH_INFO_PARSER_H_
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/memory/weak_ptr.h"
+#include "chromeos/ash/components/trash_service/public/cpp/trash_service.h"
+#include "chromeos/ash/components/trash_service/public/mojom/trash_service.mojom.h"
+#include "mojo/public/cpp/bindings/remote.h"
+
+namespace chromeos::trash_service {
+
+// A class that manages the lifetime and mojo connection of the Trash service.
+class TrashInfoParser {
+ public:
+  explicit TrashInfoParser(base::OnceCallback<void()> disconnect_callback);
+  ~TrashInfoParser();
+
+  TrashInfoParser(const TrashInfoParser&) = delete;
+  TrashInfoParser& operator=(const TrashInfoParser&) = delete;
+
+  void ParseTrashInfoFile(const base::FilePath& path,
+                          ParseTrashInfoCallback callback);
+
+ private:
+  void OnGotFile(ParseTrashInfoCallback callback, base::File file);
+
+  // A connection to the underlying TrashService.
+  mojo::Remote<mojom::TrashService> service_;
+  base::WeakPtrFactory<TrashInfoParser> weak_ptr_factory_{this};
+};
+
+}  // namespace chromeos::trash_service
+
+#endif  // CHROMEOS_ASH_COMPONENTS_TRASH_SERVICE_PUBLIC_CPP_TRASH_INFO_PARSER_H_