Migrate NativeFileSystemFileHandle and NativeFileSystemDirectoryHandle to use GC mojo wrappers.

No behavior change. This CL reduces potential risks of use-after-free bugs.

Bug: 1049056
Change-Id: I5264649499cc8e9507ca18aa2bf62ccfe0506b5e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2145527Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Minoru Chikamune <chikamune@chromium.org>
Cr-Commit-Position: refs/heads/master@{#758503}

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

@@ -37,10 +37,10 @@ NativeFileSystemDirectoryHandle::NativeFileSystemDirectoryHandle(
     ExecutionContext* context,
     const String& name,
     mojo::PendingRemote<mojom::blink::NativeFileSystemDirectoryHandle> mojo_ptr)
-    : NativeFileSystemHandle(context, name),
-      mojo_ptr_(std::move(mojo_ptr),
-                context->GetTaskRunner(TaskType::kMiscPlatformAPI)) {
-  DCHECK(mojo_ptr_);
+    : NativeFileSystemHandle(context, name), mojo_ptr_(context) {
+  mojo_ptr_.Bind(std::move(mojo_ptr),
+                 context->GetTaskRunner(TaskType::kMiscPlatformAPI));
+  DCHECK(mojo_ptr_.is_bound());
 }
 
 ScriptPromise NativeFileSystemDirectoryHandle::getFile(
@@ -79,7 +79,7 @@ ScriptPromise NativeFileSystemDirectoryHandle::getDirectory(
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise result = resolver->Promise();
 
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return result;
@@ -141,7 +141,7 @@ ScriptPromise NativeFileSystemDirectoryHandle::removeEntry(
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise result = resolver->Promise();
 
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return result;
@@ -164,7 +164,7 @@ ScriptPromise NativeFileSystemDirectoryHandle::resolve(
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise result = resolver->Promise();
 
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return result;
@@ -245,15 +245,20 @@ ScriptPromise NativeFileSystemDirectoryHandle::getSystemDirectory(
 mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken>
 NativeFileSystemDirectoryHandle::Transfer() {
   mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> result;
-  if (mojo_ptr_)
+  if (mojo_ptr_.is_bound())
     mojo_ptr_->Transfer(result.InitWithNewPipeAndPassReceiver());
   return result;
 }
 
+void NativeFileSystemDirectoryHandle::Trace(Visitor* visitor) {
+  visitor->Trace(mojo_ptr_);
+  NativeFileSystemHandle::Trace(visitor);
+}
+
 void NativeFileSystemDirectoryHandle::QueryPermissionImpl(
     bool writable,
     base::OnceCallback<void(mojom::blink::PermissionStatus)> callback) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     std::move(callback).Run(mojom::blink::PermissionStatus::DENIED);
     return;
   }
@@ -264,7 +269,7 @@ void NativeFileSystemDirectoryHandle::RequestPermissionImpl(
     bool writable,
     base::OnceCallback<void(mojom::blink::NativeFileSystemErrorPtr,
                             mojom::blink::PermissionStatus)> callback) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     std::move(callback).Run(
         mojom::blink::NativeFileSystemError::New(
             mojom::blink::NativeFileSystemStatus::kInvalidState,
@@ -280,7 +285,7 @@ void NativeFileSystemDirectoryHandle::IsSameEntryImpl(
     mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> other,
     base::OnceCallback<void(mojom::blink::NativeFileSystemErrorPtr, bool)>
         callback) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     std::move(callback).Run(
         mojom::blink::NativeFileSystemError::New(
             mojom::blink::NativeFileSystemStatus::kInvalidState,
@@ -302,8 +307,4 @@ void NativeFileSystemDirectoryHandle::IsSameEntryImpl(
           std::move(callback)));
 }
 
-void NativeFileSystemDirectoryHandle::ContextDestroyed() {
-  mojo_ptr_.reset();
-}
-
 }  // namespace blink

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

@@ -6,9 +6,9 @@
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_DIRECTORY_HANDLE_H_
 
 #include "mojo/public/cpp/bindings/pending_remote.h"
-#include "mojo/public/cpp/bindings/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/heap_mojo_remote.h"
 
 namespace blink {
 class FileSystemGetDirectoryOptions;
@@ -51,7 +51,7 @@ class NativeFileSystemDirectoryHandle final : public NativeFileSystemHandle {
     return mojo_ptr_.get();
   }
 
-  void ContextDestroyed() override;
+  void Trace(Visitor*) override;
 
  private:
   void QueryPermissionImpl(
@@ -68,7 +68,7 @@ class NativeFileSystemDirectoryHandle final : public NativeFileSystemHandle {
       base::OnceCallback<void(mojom::blink::NativeFileSystemErrorPtr, bool)>)
       override;
 
-  mojo::Remote<mojom::blink::NativeFileSystemDirectoryHandle> mojo_ptr_;
+  HeapMojoRemote<mojom::blink::NativeFileSystemDirectoryHandle> mojo_ptr_;
 };
 
 }  // namespace blink

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

@@ -27,10 +27,10 @@ NativeFileSystemFileHandle::NativeFileSystemFileHandle(
     ExecutionContext* context,
     const String& name,
     mojo::PendingRemote<mojom::blink::NativeFileSystemFileHandle> mojo_ptr)
-    : NativeFileSystemHandle(context, name),
-      mojo_ptr_(std::move(mojo_ptr),
-                context->GetTaskRunner(TaskType::kMiscPlatformAPI)) {
-  DCHECK(mojo_ptr_);
+    : NativeFileSystemHandle(context, name), mojo_ptr_(context) {
+  mojo_ptr_.Bind(std::move(mojo_ptr),
+                 context->GetTaskRunner(TaskType::kMiscPlatformAPI));
+  DCHECK(mojo_ptr_.is_bound());
 }
 
 ScriptPromise NativeFileSystemFileHandle::createWriter(
@@ -39,7 +39,7 @@ ScriptPromise NativeFileSystemFileHandle::createWriter(
   auto* resolver = MakeGarbageCollected<ScriptPromiseResolver>(script_state);
   ScriptPromise result = resolver->Promise();
 
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     resolver->Reject(MakeGarbageCollected<DOMException>(
         DOMExceptionCode::kInvalidStateError));
     return result;
@@ -71,7 +71,7 @@ ScriptPromise NativeFileSystemFileHandle::createWritable(
     ScriptState* script_state,
     const FileSystemCreateWriterOptions* options,
     ExceptionState& exception_state) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError, "");
     return ScriptPromise();
   }
@@ -105,7 +105,7 @@ ScriptPromise NativeFileSystemFileHandle::createWritable(
 ScriptPromise NativeFileSystemFileHandle::getFile(
     ScriptState* script_state,
     ExceptionState& exception_state) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     exception_state.ThrowDOMException(DOMExceptionCode::kInvalidStateError, "");
     return ScriptPromise();
   }
@@ -132,19 +132,20 @@ ScriptPromise NativeFileSystemFileHandle::getFile(
 mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken>
 NativeFileSystemFileHandle::Transfer() {
   mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> result;
-  if (mojo_ptr_)
+  if (mojo_ptr_.is_bound())
     mojo_ptr_->Transfer(result.InitWithNewPipeAndPassReceiver());
   return result;
 }
 
-void NativeFileSystemFileHandle::ContextDestroyed() {
-  mojo_ptr_.reset();
+void NativeFileSystemFileHandle::Trace(Visitor* visitor) {
+  visitor->Trace(mojo_ptr_);
+  NativeFileSystemHandle::Trace(visitor);
 }
 
 void NativeFileSystemFileHandle::QueryPermissionImpl(
     bool writable,
     base::OnceCallback<void(mojom::blink::PermissionStatus)> callback) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     std::move(callback).Run(mojom::blink::PermissionStatus::DENIED);
     return;
   }
@@ -155,7 +156,7 @@ void NativeFileSystemFileHandle::RequestPermissionImpl(
     bool writable,
     base::OnceCallback<void(mojom::blink::NativeFileSystemErrorPtr,
                             mojom::blink::PermissionStatus)> callback) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     std::move(callback).Run(
         mojom::blink::NativeFileSystemError::New(
             mojom::blink::NativeFileSystemStatus::kInvalidState,
@@ -171,7 +172,7 @@ void NativeFileSystemFileHandle::IsSameEntryImpl(
     mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> other,
     base::OnceCallback<void(mojom::blink::NativeFileSystemErrorPtr, bool)>
         callback) {
-  if (!mojo_ptr_) {
+  if (!mojo_ptr_.is_bound()) {
     std::move(callback).Run(
         mojom::blink::NativeFileSystemError::New(
             mojom::blink::NativeFileSystemStatus::kInvalidState,

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

@@ -6,9 +6,9 @@
 #define THIRD_PARTY_BLINK_RENDERER_MODULES_NATIVE_FILE_SYSTEM_NATIVE_FILE_SYSTEM_FILE_HANDLE_H_
 
 #include "mojo/public/cpp/bindings/pending_remote.h"
-#include "mojo/public/cpp/bindings/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/heap_mojo_remote.h"
 
 namespace blink {
 class FileSystemCreateWriterOptions;
@@ -34,12 +34,12 @@ class NativeFileSystemFileHandle final : public NativeFileSystemHandle {
   mojo::PendingRemote<mojom::blink::NativeFileSystemTransferToken> Transfer()
       override;
 
-  void ContextDestroyed() override;
-
   mojom::blink::NativeFileSystemFileHandle* MojoHandle() {
     return mojo_ptr_.get();
   }
 
+  void Trace(Visitor*) override;
+
  private:
   void QueryPermissionImpl(
       bool writable,
@@ -53,7 +53,7 @@ class NativeFileSystemFileHandle final : public NativeFileSystemHandle {
       base::OnceCallback<void(mojom::blink::NativeFileSystemErrorPtr, bool)>)
       override;
 
-  mojo::Remote<mojom::blink::NativeFileSystemFileHandle> mojo_ptr_;
+  HeapMojoRemote<mojom::blink::NativeFileSystemFileHandle> mojo_ptr_;
 };
 
 }  // namespace blink

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

@@ -22,7 +22,7 @@ using mojom::blink::NativeFileSystemErrorPtr;
 NativeFileSystemHandle::NativeFileSystemHandle(
     ExecutionContext* execution_context,
     const String& name)
-    : ExecutionContextLifecycleObserver(execution_context), name_(name) {}
+    : ExecutionContextClient(execution_context), name_(name) {}
 
 // static
 NativeFileSystemHandle* NativeFileSystemHandle::CreateFromMojoEntry(
@@ -114,7 +114,7 @@ ScriptPromise NativeFileSystemHandle::isSameEntry(
 
 void NativeFileSystemHandle::Trace(Visitor* visitor) {
   ScriptWrappable::Trace(visitor);
-  ExecutionContextLifecycleObserver::Trace(visitor);
+  ExecutionContextClient::Trace(visitor);
 }
 
 }  // namespace blink

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

@@ -22,7 +22,7 @@ class ExecutionContext;
 class FileSystemHandlePermissionDescriptor;
 
 class NativeFileSystemHandle : public ScriptWrappable,
-                               public ExecutionContextLifecycleObserver {
+                               public ExecutionContextClient {
   DEFINE_WRAPPERTYPEINFO();
   USING_GARBAGE_COLLECTED_MIXIN(NativeFileSystemHandle);