Convert filesystem::mojom::Directory to new Mojo types

This CL converts filesystem::mojom::DirectoryPtr to
new Mojo types using Remote or PendingRemote.

It also updates Unzip and UnzipWithFilter unzipper.mojom.

Bug: 955171
Change-Id: I1e9bb9beef460722e24cbcf353a715b8c84b6e72
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1819279Reviewed-by: Joshua Pawlicki <waffles@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#700542}

components/services/unzip/public/cpp/unzip.cc

@@ -20,7 +20,7 @@
 #include "components/services/filesystem/lock_table.h"
 #include "components/services/unzip/public/mojom/unzipper.mojom.h"
 #include "mojo/public/cpp/bindings/remote.h"
-#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/bindings/self_owned_receiver.h"
 
 namespace unzip {
 
@@ -111,10 +111,10 @@ void DoUnzipWithFilter(
     return;
   }
 
-  filesystem::mojom::DirectoryPtr directory_ptr;
-  mojo::MakeStrongBinding(
+  mojo::PendingRemote<filesystem::mojom::Directory> directory_remote;
+  mojo::MakeSelfOwnedReceiver(
       std::make_unique<filesystem::DirectoryImpl>(output_dir, nullptr, nullptr),
-      mojo::MakeRequest(&directory_ptr));
+      directory_remote.InitWithNewPipeAndPassReceiver());
 
   // |result_callback| is shared between the connection error handler and the
   // Unzip call using a refcounted UnzipParams object that owns
@@ -128,7 +128,7 @@ void DoUnzipWithFilter(
 
   if (filter_callback.is_null()) {
     unzip_params->unzipper()->Unzip(std::move(zip_file),
-                                    std::move(directory_ptr),
+                                    std::move(directory_remote),
                                     base::BindOnce(&UnzipDone, unzip_params));
     return;
   }
@@ -138,7 +138,7 @@ void DoUnzipWithFilter(
       mojo::MakeRequest(&unzip_filter_ptr), filter_callback));
 
   unzip_params->unzipper()->UnzipWithFilter(
-      std::move(zip_file), std::move(directory_ptr),
+      std::move(zip_file), std::move(directory_remote),
       std::move(unzip_filter_ptr), base::BindOnce(&UnzipDone, unzip_params));
 }
 

components/services/unzip/public/mojom/unzipper.mojom

@@ -16,13 +16,14 @@ interface UnzipFilter {
 interface Unzipper {
   // Unzip |zip_file| into |output_dir|.
   // Returns true on success, false otherwise.
-  Unzip(mojo_base.mojom.File zip_file, filesystem.mojom.Directory output_dir)
-      => (bool result);
+  Unzip(mojo_base.mojom.File zip_file,
+        pending_remote<filesystem.mojom.Directory> output_dir)
+       => (bool result);
 
   // Same as |unzip| but only includes the files for which |filter| returns
   // true. Note that this incurs one IPC for each file of the archive.
   UnzipWithFilter(
       mojo_base.mojom.File zip_file,
-      filesystem.mojom.Directory output_dir,
+      pending_remote<filesystem.mojom.Directory> output_dir,
       UnzipFilter filter) => (bool result);
 };

components/services/unzip/unzipper_impl.cc

@@ -12,6 +12,7 @@
 #include "base/strings/utf_string_conversions.h"
 #include "build/build_config.h"
 #include "components/services/filesystem/public/mojom/directory.mojom.h"
+#include "mojo/public/cpp/bindings/remote.h"
 #include "third_party/zlib/google/zip.h"
 #include "third_party/zlib/google/zip_reader.h"
 
@@ -43,26 +44,24 @@ std::string PathToMojoString(const base::FilePath& path) {
 }
 
 // Modifies output_dir to point to the final directory.
-bool CreateDirectory(filesystem::mojom::DirectoryPtr* output_dir,
+bool CreateDirectory(filesystem::mojom::Directory* output_dir,
                      const base::FilePath& path) {
   base::File::Error err = base::File::Error::FILE_OK;
-  return (*output_dir)
-             ->OpenDirectory(PathToMojoString(path), mojo::NullReceiver(),
-                             filesystem::mojom::kFlagOpenAlways, &err) &&
+  return output_dir->OpenDirectory(PathToMojoString(path), mojo::NullReceiver(),
+                                   filesystem::mojom::kFlagOpenAlways, &err) &&
          err == base::File::Error::FILE_OK;
 }
 
 std::unique_ptr<zip::WriterDelegate> MakeFileWriterDelegateNoParent(
-    filesystem::mojom::DirectoryPtr* output_dir,
+    filesystem::mojom::Directory* output_dir,
     const base::FilePath& path) {
   auto file = std::make_unique<base::File>();
   base::File::Error err;
-  if (!(*output_dir)
-           ->OpenFileHandle(PathToMojoString(path),
-                            filesystem::mojom::kFlagCreate |
-                                filesystem::mojom::kFlagWrite |
-                                filesystem::mojom::kFlagWriteAttributes,
-                            &err, file.get()) ||
+  if (!output_dir->OpenFileHandle(PathToMojoString(path),
+                                  filesystem::mojom::kFlagCreate |
+                                      filesystem::mojom::kFlagWrite |
+                                      filesystem::mojom::kFlagWriteAttributes,
+                                  &err, file.get()) ||
       err != base::File::Error::FILE_OK) {
     return std::make_unique<DudWriterDelegate>();
   }
@@ -70,20 +69,19 @@ std::unique_ptr<zip::WriterDelegate> MakeFileWriterDelegateNoParent(
 }
 
 std::unique_ptr<zip::WriterDelegate> MakeFileWriterDelegate(
-    filesystem::mojom::DirectoryPtr* output_dir,
+    filesystem::mojom::Directory* output_dir,
     const base::FilePath& path) {
   if (path == path.BaseName())
     return MakeFileWriterDelegateNoParent(output_dir, path);
-  filesystem::mojom::DirectoryPtr parent;
+  mojo::Remote<filesystem::mojom::Directory> parent;
   base::File::Error err;
-  if (!(*output_dir)
-           ->OpenDirectory(PathToMojoString(path.DirName()),
-                           mojo::MakeRequest(&parent),
-                           filesystem::mojom::kFlagOpenAlways, &err) ||
+  if (!output_dir->OpenDirectory(PathToMojoString(path.DirName()),
+                                 parent.BindNewPipeAndPassReceiver(),
+                                 filesystem::mojom::kFlagOpenAlways, &err) ||
       err != base::File::Error::FILE_OK) {
     return std::make_unique<DudWriterDelegate>();
   }
-  return MakeFileWriterDelegateNoParent(&parent, path.BaseName());
+  return MakeFileWriterDelegateNoParent(parent.get(), path.BaseName());
 }
 
 bool FilterNoFiles(const base::FilePath& unused) {
@@ -106,22 +104,28 @@ UnzipperImpl::UnzipperImpl(mojo::PendingReceiver<mojom::Unzipper> receiver)
 
 UnzipperImpl::~UnzipperImpl() = default;
 
-void UnzipperImpl::Unzip(base::File zip_file,
-                         filesystem::mojom::DirectoryPtr output_dir,
-                         UnzipCallback callback) {
+void UnzipperImpl::Unzip(
+    base::File zip_file,
+    mojo::PendingRemote<filesystem::mojom::Directory> output_dir_remote,
+    UnzipCallback callback) {
   DCHECK(zip_file.IsValid());
+  mojo::Remote<filesystem::mojom::Directory> output_dir(
+      std::move(output_dir_remote));
   std::move(callback).Run(zip::UnzipWithFilterAndWriters(
       zip_file.GetPlatformFile(),
-      base::BindRepeating(&MakeFileWriterDelegate, &output_dir),
-      base::BindRepeating(&CreateDirectory, &output_dir),
+      base::BindRepeating(&MakeFileWriterDelegate, output_dir.get()),
+      base::BindRepeating(&CreateDirectory, output_dir.get()),
       base::BindRepeating(&FilterNoFiles), /*log_skipped_files=*/false));
 }
 
-void UnzipperImpl::UnzipWithFilter(base::File zip_file,
-                                   filesystem::mojom::DirectoryPtr output_dir,
-                                   mojom::UnzipFilterPtr filter,
-                                   UnzipCallback callback) {
+void UnzipperImpl::UnzipWithFilter(
+    base::File zip_file,
+    mojo::PendingRemote<filesystem::mojom::Directory> output_dir_remote,
+    mojom::UnzipFilterPtr filter,
+    UnzipCallback callback) {
   DCHECK(zip_file.IsValid());
+  mojo::Remote<filesystem::mojom::Directory> output_dir(
+      std::move(output_dir_remote));
 
   // Note that we pass a pointer to |filter| below, as it is a repeating
   // callback and transferring its value would cause the callback to fail when
@@ -130,8 +134,8 @@ void UnzipperImpl::UnzipWithFilter(base::File zip_file,
   // the method returns.
   std::move(callback).Run(zip::UnzipWithFilterAndWriters(
       zip_file.GetPlatformFile(),
-      base::BindRepeating(&MakeFileWriterDelegate, &output_dir),
-      base::BindRepeating(&CreateDirectory, &output_dir),
+      base::BindRepeating(&MakeFileWriterDelegate, output_dir.get()),
+      base::BindRepeating(&CreateDirectory, output_dir.get()),
       base::BindRepeating(&FilterWithFilterPtr, &filter),
       /*log_skipped_files=*/false));
 }

components/services/unzip/unzipper_impl.h

@@ -11,6 +11,7 @@
 #include "base/macros.h"
 #include "components/services/unzip/public/mojom/unzipper.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "mojo/public/cpp/bindings/receiver.h"
 
 namespace unzip {
@@ -28,14 +29,16 @@ class UnzipperImpl : public mojom::Unzipper {
 
  private:
   // unzip::mojom::Unzipper:
-  void Unzip(base::File zip_file,
-             filesystem::mojom::DirectoryPtr output_dir,
-             UnzipCallback callback) override;
-
-  void UnzipWithFilter(base::File zip_file,
-                       filesystem::mojom::DirectoryPtr output_dir,
-                       mojom::UnzipFilterPtr filter,
-                       UnzipWithFilterCallback callback) override;
+  void Unzip(
+      base::File zip_file,
+      mojo::PendingRemote<filesystem::mojom::Directory> output_dir_remote,
+      UnzipCallback callback) override;
+
+  void UnzipWithFilter(
+      base::File zip_file,
+      mojo::PendingRemote<filesystem::mojom::Directory> output_dir_remote,
+      mojom::UnzipFilterPtr filter,
+      UnzipWithFilterCallback callback) override;
 
   mojo::Receiver<mojom::Unzipper> receiver_{this};