Convert NativeFileSystemTransferToken to new Mojo types

This CL converts NativeFileSystemTransferTokenRequest to
new Mojo types.

With this CL, NativeFileSystemManagerImpl forwards
mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
to NativeFileSystemTransferTokenImpl and
NativeFileSystemTransferTokenImpl owns
mojo::Receiver<blink::mojom::NativeFileSystemTransferToken>.

Bug: 955171, 978694
Change-Id: I8cba16dbae29478ca0909a05191412e7003d3f2d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1777442
Commit-Queue: Julie Kim <jkim@igalia.com>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Oksana Zhuravlova <oksamyt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#697981}

content/browser/native_file_system/native_file_system_directory_handle_impl.cc

@@ -22,8 +22,7 @@ using blink::mojom::NativeFileSystemEntry;
 using blink::mojom::NativeFileSystemEntryPtr;
 using blink::mojom::NativeFileSystemHandle;
 using blink::mojom::NativeFileSystemStatus;
-using blink::mojom::NativeFileSystemTransferTokenPtr;
-using blink::mojom::NativeFileSystemTransferTokenRequest;
+using blink::mojom::NativeFileSystemTransferToken;
 
 namespace content {
 
@@ -205,7 +204,7 @@ void NativeFileSystemDirectoryHandleImpl::RemoveEntry(
 }
 
 void NativeFileSystemDirectoryHandleImpl::Transfer(
-    NativeFileSystemTransferTokenRequest token) {
+    mojo::PendingReceiver<NativeFileSystemTransferToken> token) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   manager()->CreateTransferToken(*this, std::move(token));

content/browser/native_file_system/native_file_system_directory_handle_impl.h

@@ -10,6 +10,7 @@
 #include "base/memory/weak_ptr.h"
 #include "components/services/filesystem/public/mojom/types.mojom.h"
 #include "content/browser/native_file_system/native_file_system_handle_base.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "storage/browser/fileapi/file_system_url.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_directory_handle.mojom.h"
 
@@ -47,7 +48,8 @@ class NativeFileSystemDirectoryHandleImpl
                    bool recurse,
                    RemoveEntryCallback callback) override;
   void Transfer(
-      blink::mojom::NativeFileSystemTransferTokenRequest token) override;
+      mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken> token)
+      override;
 
  private:
   // State that is kept for the duration of a GetEntries/ReadDirectory call.

content/browser/native_file_system/native_file_system_file_handle_impl.cc

@@ -89,7 +89,7 @@ void NativeFileSystemFileHandleImpl::CreateFileWriter(
 }
 
 void NativeFileSystemFileHandleImpl::Transfer(
-    blink::mojom::NativeFileSystemTransferTokenRequest token) {
+    mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken> token) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   manager()->CreateTransferToken(*this, std::move(token));

content/browser/native_file_system/native_file_system_file_handle_impl.h

@@ -10,6 +10,7 @@
 #include "content/browser/native_file_system/native_file_system_handle_base.h"
 #include "content/browser/native_file_system/native_file_system_manager_impl.h"
 #include "content/common/content_export.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "storage/browser/fileapi/file_system_url.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_file_handle.mojom.h"
 
@@ -46,7 +47,8 @@ class CONTENT_EXPORT NativeFileSystemFileHandleImpl
   void CreateFileWriter(bool keep_existing_data,
                         CreateFileWriterCallback callback) override;
   void Transfer(
-      blink::mojom::NativeFileSystemTransferTokenRequest token) override;
+      mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken> token)
+      override;
 
   void set_max_swap_files_for_testing(int max) { max_swap_files_ = max; }
 

content/browser/native_file_system/native_file_system_manager_impl.cc

@@ -351,27 +351,31 @@ NativeFileSystemManagerImpl::CreateFileWriter(
 
 void NativeFileSystemManagerImpl::CreateTransferToken(
     const NativeFileSystemFileHandleImpl& file,
-    blink::mojom::NativeFileSystemTransferTokenRequest request) {
+    mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
+        receiver) {
   return CreateTransferTokenImpl(file.url(), file.handle_state(),
-                                 /*is_directory=*/false, std::move(request));
+                                 /*is_directory=*/false, std::move(receiver));
 }
 
 void NativeFileSystemManagerImpl::CreateTransferToken(
     const NativeFileSystemDirectoryHandleImpl& directory,
-    blink::mojom::NativeFileSystemTransferTokenRequest request) {
+    mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
+        receiver) {
   return CreateTransferTokenImpl(directory.url(), directory.handle_state(),
-                                 /*is_directory=*/true, std::move(request));
+                                 /*is_directory=*/true, std::move(receiver));
 }
 
 void NativeFileSystemManagerImpl::ResolveTransferToken(
-    blink::mojom::NativeFileSystemTransferTokenPtr token,
+    mojo::PendingRemote<blink::mojom::NativeFileSystemTransferToken> token,
     ResolvedTokenCallback callback) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
-  auto* raw_token = token.get();
+  mojo::Remote<blink::mojom::NativeFileSystemTransferToken> token_remote(
+      std::move(token));
+  auto* raw_token = token_remote.get();
   raw_token->GetInternalID(mojo::WrapCallbackWithDefaultInvokeIfNotRun(
       base::BindOnce(&NativeFileSystemManagerImpl::DoResolveTransferToken,
-                     weak_factory_.GetWeakPtr(), std::move(token),
+                     weak_factory_.GetWeakPtr(), std::move(token_remote),
                      std::move(callback)),
       base::UnguessableToken()));
 }
@@ -554,25 +558,20 @@ void NativeFileSystemManagerImpl::CreateTransferTokenImpl(
     const storage::FileSystemURL& url,
     const SharedHandleState& handle_state,
     bool is_directory,
-    blink::mojom::NativeFileSystemTransferTokenRequest request) {
+    mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
+        receiver) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
   auto token_impl = std::make_unique<NativeFileSystemTransferTokenImpl>(
       url, handle_state,
       is_directory ? NativeFileSystemTransferTokenImpl::HandleType::kDirectory
-                   : NativeFileSystemTransferTokenImpl::HandleType::kFile);
+                   : NativeFileSystemTransferTokenImpl::HandleType::kFile,
+      this, std::move(receiver));
   auto token = token_impl->token();
-  blink::mojom::NativeFileSystemTransferTokenPtr result;
-  auto emplace_result = transfer_tokens_.emplace(
-      std::piecewise_construct, std::forward_as_tuple(token),
-      std::forward_as_tuple(std::move(token_impl), std::move(request)));
-  DCHECK(emplace_result.second);
-  emplace_result.first->second.set_connection_error_handler(base::BindOnce(
-      &NativeFileSystemManagerImpl::TransferTokenConnectionErrorHandler,
-      base::Unretained(this), token));
+  transfer_tokens_.emplace(token, std::move(token_impl));
 }
 
-void NativeFileSystemManagerImpl::TransferTokenConnectionErrorHandler(
+void NativeFileSystemManagerImpl::RemoveToken(
     const base::UnguessableToken& token) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
 
@@ -581,7 +580,7 @@ void NativeFileSystemManagerImpl::TransferTokenConnectionErrorHandler(
 }
 
 void NativeFileSystemManagerImpl::DoResolveTransferToken(
-    blink::mojom::NativeFileSystemTransferTokenPtr,
+    mojo::Remote<blink::mojom::NativeFileSystemTransferToken>,
     ResolvedTokenCallback callback,
     const base::UnguessableToken& token) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
@@ -590,8 +589,7 @@ void NativeFileSystemManagerImpl::DoResolveTransferToken(
   if (it == transfer_tokens_.end()) {
     std::move(callback).Run(nullptr);
   } else {
-    std::move(callback).Run(
-        static_cast<NativeFileSystemTransferTokenImpl*>(it->second.impl()));
+    std::move(callback).Run(it->second.get());
   }
 }
 

content/browser/native_file_system/native_file_system_manager_impl.h

@@ -13,8 +13,11 @@
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/native_file_system_entry_factory.h"
 #include "content/public/browser/native_file_system_permission_context.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/pending_remote.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "mojo/public/cpp/bindings/receiver_set.h"
-#include "mojo/public/cpp/bindings/strong_binding.h"
+#include "mojo/public/cpp/bindings/remote.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"
@@ -136,10 +139,12 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
   // Create a transfer token for a specific file or directory.
   void CreateTransferToken(
       const NativeFileSystemFileHandleImpl& file,
-      blink::mojom::NativeFileSystemTransferTokenRequest request);
+      mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
+          receiver);
   void CreateTransferToken(
       const NativeFileSystemDirectoryHandleImpl& directory,
-      blink::mojom::NativeFileSystemTransferTokenRequest request);
+      mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
+          receiver);
 
   // Given a mojom transfer token, looks up the token in our internal list of
   // valid tokens. Calls the callback with the found token, or nullptr if no
@@ -147,7 +152,7 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
   using ResolvedTokenCallback =
       base::OnceCallback<void(NativeFileSystemTransferTokenImpl*)>;
   void ResolveTransferToken(
-      blink::mojom::NativeFileSystemTransferTokenPtr token,
+      mojo::PendingRemote<blink::mojom::NativeFileSystemTransferToken> token,
       ResolvedTokenCallback callback);
 
   storage::FileSystemContext* context() {
@@ -170,6 +175,10 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
     permission_context_ = permission_context;
   }
 
+  // Remove |token| from |transfer_tokens_|. It is an error to try to remove a
+  // token that doesn't exist.
+  void RemoveToken(const base::UnguessableToken& token);
+
  private:
   friend class NativeFileSystemFileHandleImpl;
 
@@ -205,11 +214,12 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
       const storage::FileSystemURL& url,
       const SharedHandleState& handle_state,
       bool is_directory,
-      blink::mojom::NativeFileSystemTransferTokenRequest request);
-  void TransferTokenConnectionErrorHandler(const base::UnguessableToken& token);
-  void DoResolveTransferToken(blink::mojom::NativeFileSystemTransferTokenPtr,
-                              ResolvedTokenCallback callback,
-                              const base::UnguessableToken& token);
+      mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
+          receiver);
+  void DoResolveTransferToken(
+      mojo::Remote<blink::mojom::NativeFileSystemTransferToken>,
+      ResolvedTokenCallback callback,
+      const base::UnguessableToken& token);
 
   // Creates a FileSystemURL which corresponds to a FilePath and Origin.
   struct FileSystemURLAndFSHandle {
@@ -258,14 +268,11 @@ class CONTENT_EXPORT NativeFileSystemManagerImpl
 
   bool off_the_record_;
 
-  // Transfer token bindings are stored in what is effectively a
-  // StrongBindingMap. The Binding instances own the implementation, and tokens
-  // are removed from this map when the mojo connection is closed.
-  using TransferTokenBinding =
-      mojo::Binding<blink::mojom::NativeFileSystemTransferToken,
-                    mojo::UniquePtrImplRefTraits<
-                        blink::mojom::NativeFileSystemTransferToken>>;
-  std::map<base::UnguessableToken, TransferTokenBinding> transfer_tokens_;
+  // NativeFileSystemTransferTokenImpl owns a Transfer token receiver and is
+  // removed from this map when the mojo connection is closed.
+  std::map<base::UnguessableToken,
+           std::unique_ptr<NativeFileSystemTransferTokenImpl>>
+      transfer_tokens_;
 
   base::WeakPtrFactory<NativeFileSystemManagerImpl> weak_factory_{this};
   DISALLOW_COPY_AND_ASSIGN(NativeFileSystemManagerImpl);

content/browser/native_file_system/native_file_system_transfer_token_impl.cc

@@ -9,19 +9,34 @@ namespace content {
 NativeFileSystemTransferTokenImpl::NativeFileSystemTransferTokenImpl(
     const storage::FileSystemURL& url,
     const SharedHandleState& handle_state,
-    HandleType type)
+    HandleType type,
+    NativeFileSystemManagerImpl* manager,
+    mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken> receiver)
     : token_(base::UnguessableToken::Create()),
       url_(url),
       handle_state_(handle_state),
-      type_(type) {
+      type_(type),
+      manager_(manager),
+      receiver_(this, std::move(receiver)) {
+  DCHECK(manager_);
   DCHECK_EQ(url_.mount_type() == storage::kFileSystemTypeIsolated,
             handle_state_.file_system.is_valid())
       << url_.mount_type();
+  receiver_.set_disconnect_handler(
+      base::BindOnce(&NativeFileSystemTransferTokenImpl::OnMojoDisconnect,
+                     base::Unretained(this)));
 }
 
+NativeFileSystemTransferTokenImpl::~NativeFileSystemTransferTokenImpl() =
+    default;
+
 void NativeFileSystemTransferTokenImpl::GetInternalID(
     GetInternalIDCallback callback) {
   std::move(callback).Run(token_);
 }
 
+void NativeFileSystemTransferTokenImpl::OnMojoDisconnect() {
+  manager_->RemoveToken(token_);
+}
+
 }  // namespace content

content/browser/native_file_system/native_file_system_transfer_token_impl.h

@@ -6,6 +6,8 @@
 #define CONTENT_BROWSER_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_TRANSFER_TOKEN_IMPL_H_
 
 #include "content/browser/native_file_system/native_file_system_manager_impl.h"
+#include "mojo/public/cpp/bindings/pending_receiver.h"
+#include "mojo/public/cpp/bindings/receiver.h"
 #include "storage/browser/fileapi/file_system_url.h"
 #include "storage/browser/fileapi/isolated_context.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_transfer_token.mojom.h"
@@ -24,9 +26,14 @@ class NativeFileSystemTransferTokenImpl
 
   enum class HandleType { kFile, kDirectory };
 
-  NativeFileSystemTransferTokenImpl(const storage::FileSystemURL& url,
-                                    const SharedHandleState& handle_state,
-                                    HandleType type);
+  NativeFileSystemTransferTokenImpl(
+      const storage::FileSystemURL& url,
+      const SharedHandleState& handle_state,
+      HandleType type,
+      NativeFileSystemManagerImpl* manager,
+      mojo::PendingReceiver<blink::mojom::NativeFileSystemTransferToken>
+          receiver);
+  ~NativeFileSystemTransferTokenImpl() override;
 
   const base::UnguessableToken& token() const { return token_; }
   const storage::FileSystemURL& url() const { return url_; }
@@ -36,10 +43,15 @@ class NativeFileSystemTransferTokenImpl
   void GetInternalID(GetInternalIDCallback callback) override;
 
  private:
+  void OnMojoDisconnect();
+
   const base::UnguessableToken token_;
   const storage::FileSystemURL url_;
   const SharedHandleState handle_state_;
   const HandleType type_;
+  // Raw pointer since NativeFileSystemManagerImpl owns |this|.
+  NativeFileSystemManagerImpl* const manager_;
+  mojo::Receiver<blink::mojom::NativeFileSystemTransferToken> receiver_;
 
   DISALLOW_COPY_AND_ASSIGN(NativeFileSystemTransferTokenImpl);
 };

third_party/blink/public/mojom/native_file_system/native_file_system_directory_handle.mojom

@@ -65,5 +65,5 @@ interface NativeFileSystemDirectoryHandle {
   // 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
   // the directory, or when transferring the handle over postMessage.
-  Transfer(NativeFileSystemTransferToken& token);
+  Transfer(pending_receiver<NativeFileSystemTransferToken> token);
 };

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

@@ -31,5 +31,5 @@ interface NativeFileSystemFileHandle {
   // 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
   // the file, or when transferring the handle over postMessage.
-  Transfer(NativeFileSystemTransferToken& token);
+  Transfer(pending_receiver<NativeFileSystemTransferToken> token);
 };

third_party/blink/public/mojom/native_file_system/native_file_system_manager.mojom

@@ -49,6 +49,6 @@ interface NativeFileSystemManager {
        array<NativeFileSystemEntry> entries);
 
   // TODO(mek): Add this method when it is needed for transferability of handles.
-  // GetHandleFromToken(NativeFileSystemTransferToken token) =>
+  // GetHandleFromToken(pending_remote<NativeFileSystemTransferToken> token) =>
   //     (NativeFileSystemError result, NativeFileSystemHandle? entry_handle);
 };

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

@@ -191,10 +191,10 @@ ScriptPromise NativeFileSystemDirectoryHandle::getSystemDirectory(
   return result;
 }
 
-mojom::blink::NativeFileSystemTransferTokenPtr
+mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken>
 NativeFileSystemDirectoryHandle::Transfer() {
-  mojom::blink::NativeFileSystemTransferTokenPtr result;
-  mojo_ptr_->Transfer(mojo::MakeRequest(&result));
+  mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> result;
+  mojo_ptr_->Transfer(result.InitWithNewPipeAndPassReceiver());
   return result;
 }
 

third_party/blink/renderer/modules/native_file_system/native_file_system_directory_handle.h

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_DIRECTORY_HANDLE_H_
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_DIRECTORY_HANDLE_H_
 
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_directory_handle.mojom-blink.h"
 #include "third_party/blink/renderer/modules/native_file_system/native_file_system_handle.h"
 #include "third_party/blink/renderer/platform/mojo/revocable_interface_ptr.h"
@@ -39,7 +40,8 @@ class NativeFileSystemDirectoryHandle final : public NativeFileSystemHandle {
   static ScriptPromise getSystemDirectory(ScriptState*,
                                           const GetSystemDirectoryOptions*);
 
-  mojom::blink::NativeFileSystemTransferTokenPtr Transfer() override;
+  mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> Transfer()
+      override;
 
   mojom::blink::NativeFileSystemDirectoryHandle* MojoHandle() {
     return mojo_ptr_.get();

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

@@ -76,10 +76,10 @@ ScriptPromise NativeFileSystemFileHandle::getFile(ScriptState* script_state) {
   return result;
 }
 
-mojom::blink::NativeFileSystemTransferTokenPtr
+mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken>
 NativeFileSystemFileHandle::Transfer() {
-  mojom::blink::NativeFileSystemTransferTokenPtr result;
-  mojo_ptr_->Transfer(mojo::MakeRequest(&result));
+  mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> result;
+  mojo_ptr_->Transfer(result.InitWithNewPipeAndPassReceiver());
   return result;
 }
 

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

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_FILE_HANDLE_H_
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_FILE_HANDLE_H_
 
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_file_handle.mojom-blink.h"
 #include "third_party/blink/renderer/modules/native_file_system/native_file_system_handle.h"
 #include "third_party/blink/renderer/platform/mojo/revocable_interface_ptr.h"
@@ -26,7 +27,8 @@ class NativeFileSystemFileHandle final : public NativeFileSystemHandle {
                              const FileSystemCreateWriterOptions* options);
   ScriptPromise getFile(ScriptState*);
 
-  mojom::blink::NativeFileSystemTransferTokenPtr Transfer() override;
+  mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> Transfer()
+      override;
 
   mojom::blink::NativeFileSystemFileHandle* MojoHandle() {
     return mojo_ptr_.get();

third_party/blink/renderer/modules/native_file_system/native_file_system_handle.h

@@ -5,6 +5,7 @@
 #ifndef THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_HANDLE_H_
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_HANDLE_H_
 
+#include "mojo/public/cpp/bindings/pending_remote.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_directory_handle.mojom-blink-forward.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_error.mojom-blink-forward.h"
 #include "third_party/blink/public/mojom/native_file_system/native_file_system_transfer_token.mojom-blink-forward.h"
@@ -37,7 +38,8 @@ class NativeFileSystemHandle : public ScriptWrappable {
   ScriptPromise requestPermission(ScriptState*,
                                   const FileSystemHandlePermissionDescriptor*);
 
-  virtual mojom::blink::NativeFileSystemTransferTokenPtr Transfer() = 0;
+  virtual mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken>
+  Transfer() = 0;
 
  private:
   virtual void QueryPermissionImpl(