Convert NativeFileSystemFileWriter to new Mojo types

This CL converts NativeFileSystemFileWriterPtr to new
Mojo types.

It also updates CreateFileWriter from
native_file_system_file_handle.mojom.

Bug: 955171, 978694
Change-Id: Ie4bf329706f90ca5906a921f9befa960fe7779ae
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1771647Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Commit-Queue: Julie Kim <jkim@igalia.com>
Cr-Commit-Position: refs/heads/master@{#691495}

content/browser/native_file_system/native_file_system_file_handle_impl.cc

@@ -82,7 +82,7 @@ void NativeFileSystemFileHandleImpl::CreateFileWriter(
       base::BindOnce([](CreateFileWriterCallback callback) {
         std::move(callback).Run(native_file_system_error::FromStatus(
                                     NativeFileSystemStatus::kPermissionDenied),
-                                nullptr);
+                                mojo::NullRemote());
       }),
       std::move(callback));
 }
@@ -173,7 +173,7 @@ void NativeFileSystemFileHandleImpl::CreateSwapFile(
   if (GetWritePermissionStatus() != blink::mojom::PermissionStatus::GRANTED) {
     std::move(callback).Run(native_file_system_error::FromStatus(
                                 NativeFileSystemStatus::kPermissionDenied),
-                            nullptr);
+                            mojo::NullRemote());
     return;
   }
 
@@ -188,7 +188,7 @@ void NativeFileSystemFileHandleImpl::CreateSwapFile(
     std::move(callback).Run(native_file_system_error::FromStatus(
                                 NativeFileSystemStatus::kOperationFailed,
                                 "Failed to create swap file."),
-                            nullptr);
+                            mojo::NullRemote());
     return;
   }
 
@@ -251,7 +251,7 @@ void NativeFileSystemFileHandleImpl::DidCreateSwapFile(
                 << " path: " << swap_url.path();
     std::move(callback).Run(native_file_system_error::FromFileError(
                                 result, "Error creating swap file."),
-                            nullptr);
+                            mojo::NullRemote());
     return;
   }
 
@@ -288,7 +288,7 @@ void NativeFileSystemFileHandleImpl::DidCopySwapFile(
                 << " path: " << swap_url.path();
     std::move(callback).Run(native_file_system_error::FromFileError(
                                 result, "Error copying to swap file."),
-                            nullptr);
+                            mojo::NullRemote());
     return;
   }
   std::move(callback).Run(

content/browser/native_file_system/native_file_system_file_handle_impl_unittest.cc

@@ -121,7 +121,8 @@ TEST_F(NativeFileSystemFileHandleImplTest, CreateFileWriterOverLimitNotOK) {
           test_src_origin_.GetURL(), storage::kFileSystemTypeTest,
           base::FilePath::FromUTF8Unsafe("test.crswap"));
 
-  std::vector<blink::mojom::NativeFileSystemFileWriterPtr> writers;
+  std::vector<mojo::PendingRemote<blink::mojom::NativeFileSystemFileWriter>>
+      writers;
   for (int i = 0; i < max_files; i++) {
     FileSystemURL swap_url;
     if (i == 0) {
@@ -138,11 +139,12 @@ TEST_F(NativeFileSystemFileHandleImplTest, CreateFileWriterOverLimitNotOK) {
         /*keepExistingData=*/false,
         base::BindLambdaForTesting(
             [&](blink::mojom::NativeFileSystemErrorPtr result,
-                blink::mojom::NativeFileSystemFileWriterPtr writer_ptr) {
+                mojo::PendingRemote<blink::mojom::NativeFileSystemFileWriter>
+                    writer_remote) {
               EXPECT_EQ(blink::mojom::NativeFileSystemStatus::kOk,
                         result->status);
               EXPECT_EQ("", ReadFile(swap_url));
-              writers.push_back(std::move(writer_ptr));
+              writers.push_back(std::move(writer_remote));
               loop.Quit();
             }));
     loop.Run();
@@ -153,7 +155,8 @@ TEST_F(NativeFileSystemFileHandleImplTest, CreateFileWriterOverLimitNotOK) {
       /*keepExistingData=*/false,
       base::BindLambdaForTesting(
           [&](blink::mojom::NativeFileSystemErrorPtr result,
-              blink::mojom::NativeFileSystemFileWriterPtr writer_ptr) {
+              mojo::PendingRemote<blink::mojom::NativeFileSystemFileWriter>
+                  writer_remote) {
             EXPECT_EQ(blink::mojom::NativeFileSystemStatus::kOperationFailed,
                       result->status);
             loop.Quit();

content/browser/native_file_system/native_file_system_manager_impl.cc

@@ -309,7 +309,7 @@ NativeFileSystemManagerImpl::CreateDirectoryHandle(
   return result;
 }
 
-blink::mojom::NativeFileSystemFileWriterPtr
+mojo::PendingRemote<blink::mojom::NativeFileSystemFileWriter>
 NativeFileSystemManagerImpl::CreateFileWriter(
     const BindingContext& binding_context,
     const storage::FileSystemURL& url,
@@ -317,11 +317,10 @@ NativeFileSystemManagerImpl::CreateFileWriter(
     const SharedHandleState& handle_state) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  blink::mojom::NativeFileSystemFileWriterPtr result;
-  writer_bindings_.AddBinding(
-      std::make_unique<NativeFileSystemFileWriterImpl>(
-          this, binding_context, url, swap_url, handle_state),
-      mojo::MakeRequest(&result));
+  mojo::PendingRemote<blink::mojom::NativeFileSystemFileWriter> result;
+  writer_receivers_.Add(std::make_unique<NativeFileSystemFileWriterImpl>(
+                            this, binding_context, url, swap_url, handle_state),
+                        result.InitWithNewPipeAndPassReceiver());
 
   return result;
 }

content/browser/native_file_system/native_file_system_manager_impl.h

@@ -16,6 +16,7 @@
 #include "mojo/public/cpp/bindings/receiver_set.h"
 #include "mojo/public/cpp/bindings/strong_binding.h"
 #include "mojo/public/cpp/bindings/strong_binding_set.h"
+#include "mojo/public/cpp/bindings/unique_receiver_set.h"
 #include "storage/browser/fileapi/file_system_url.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_file_writer.mojom.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_manager.mojom.h"
@@ -135,11 +136,11 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
 
   // Creates a new NativeFileSystemFileWriterImpl for a given target and
   // swap file URLs. Assumes the passed in URLs are valid and represent files.
-  blink::mojom::NativeFileSystemFileWriterPtr CreateFileWriter(
-      const BindingContext& binding_context,
-      const storage::FileSystemURL& url,
-      const storage::FileSystemURL& swap_url,
-      const SharedHandleState& handle_state);
+  mojo::PendingRemote<blink::mojom::NativeFileSystemFileWriter>
+  CreateFileWriter(const BindingContext& binding_context,
+                   const storage::FileSystemURL& url,
+                   const storage::FileSystemURL& swap_url,
+                   const SharedHandleState& handle_state);
 
   // Create a transfer token for a specific file or directory.
   void CreateTransferToken(
@@ -238,14 +239,14 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
   mojo::ReceiverSet<blink::mojom::NativeFileSystemManager, BindingContext>
       receivers_;
 
-  // All the bindings for file and directory handles that have references to
-  // them.
+  // All the bindings and receivers for file and directory handles that have
+  // references to them.
   mojo::StrongBindingSet<blink::mojom::NativeFileSystemFileHandle>
       file_bindings_;
   mojo::StrongBindingSet<blink::mojom::NativeFileSystemDirectoryHandle>
       directory_bindings_;
-  mojo::StrongBindingSet<blink::mojom::NativeFileSystemFileWriter>
-      writer_bindings_;
+  mojo::UniqueReceiverSet<blink::mojom::NativeFileSystemFileWriter>
+      writer_receivers_;
 
   bool off_the_record_;
 

content/browser/native_file_system/native_file_system_manager_impl_unittest.cc

@@ -206,18 +206,18 @@ TEST_F(NativeFileSystemManagerImplTest,
             AsyncFileTestHelper::CreateFile(file_system_context_.get(),
                                             test_swap_url));
 
-  auto writer_ptr =
+  mojo::Remote<blink::mojom::NativeFileSystemFileWriter> writer_remote(
       manager_->CreateFileWriter(kBindingContext, test_file_url, test_swap_url,
                                  NativeFileSystemManagerImpl::SharedHandleState(
-                                     allow_grant_, allow_grant_, {}));
+                                     allow_grant_, allow_grant_, {})));
 
-  ASSERT_TRUE(writer_ptr.is_bound());
+  ASSERT_TRUE(writer_remote.is_bound());
   ASSERT_TRUE(
       AsyncFileTestHelper::FileExists(file_system_context_.get(), test_swap_url,
                                       AsyncFileTestHelper::kDontCheckSize));
 
   // Severs the mojo pipe, causing the writer to be destroyed.
-  writer_ptr.reset();
+  writer_remote.reset();
   base::RunLoop().RunUntilIdle();
 
   ASSERT_FALSE(
@@ -239,19 +239,19 @@ TEST_F(NativeFileSystemManagerImplTest,
             AsyncFileTestHelper::CreateFileWithData(file_system_context_.get(),
                                                     test_swap_url, "foo", 3));
 
-  auto writer_ptr =
+  mojo::Remote<blink::mojom::NativeFileSystemFileWriter> writer_remote(
       manager_->CreateFileWriter(kBindingContext, test_file_url, test_swap_url,
                                  NativeFileSystemManagerImpl::SharedHandleState(
-                                     allow_grant_, allow_grant_, {}));
+                                     allow_grant_, allow_grant_, {})));
 
-  ASSERT_TRUE(writer_ptr.is_bound());
+  ASSERT_TRUE(writer_remote.is_bound());
   ASSERT_FALSE(
       AsyncFileTestHelper::FileExists(file_system_context_.get(), test_file_url,
                                       AsyncFileTestHelper::kDontCheckSize));
-  writer_ptr->Close(base::DoNothing());
+  writer_remote->Close(base::DoNothing());
 
   // Severs the mojo pipe, causing the writer to be destroyed.
-  writer_ptr.reset();
+  writer_remote.reset();
   base::RunLoop().RunUntilIdle();
 
   ASSERT_FALSE(

third_party/blink/public/mojom/native_file_system/native_file_system_file_handle.mojom

@@ -26,7 +26,7 @@ interface NativeFileSystemFileHandle {
 
   // Returns a FileWriter object. The FileWriter provides write operations on a file.
   CreateFileWriter(bool keep_existing_data) => (
-      NativeFileSystemError result, NativeFileSystemFileWriter? writer);
+      NativeFileSystemError result, pending_remote<NativeFileSystemFileWriter>? writer);
 
   // Create a TransferToken for this directory. This token can be used to pass
   // a reference to this directory to other methods, for example to copy or move

third_party/blink/renderer/modules/native_file_system/native_file_system_file_handle.cc

@@ -38,7 +38,8 @@ ScriptPromise NativeFileSystemFileHandle::createWriter(
       WTF::Bind(
           [](ScriptPromiseResolver* resolver,
              mojom::blink::NativeFileSystemErrorPtr result,
-             mojom::blink::NativeFileSystemFileWriterPtr writer) {
+             mojo::PendingRemote<mojom::blink::NativeFileSystemFileWriter>
+                 writer) {
             ExecutionContext* context = resolver->GetExecutionContext();
             if (!context)
               return;
@@ -48,7 +49,7 @@ ScriptPromise NativeFileSystemFileHandle::createWriter(
             }
             resolver->Resolve(MakeGarbageCollected<NativeFileSystemWriter>(
                 RevocableInterfacePtr<mojom::blink::NativeFileSystemFileWriter>(
-                    writer.PassInterface(), context->GetInterfaceInvalidator(),
+                    std::move(writer), context->GetInterfaceInvalidator(),
                     context->GetTaskRunner(TaskType::kMiscPlatformAPI))));
           },
           WrapPersistent(resolver)));