Bug 1803062 - Change FileSystemSyncAccessHandle::Close to be async; r=dom-storage-reviewers,jesup

This patch adds all remaining infrastructure for handling async close calls
without actually closing the stream on the IO task queue.

Differential Revision: https://phabricator.services.mozilla.com/D163842

dom/fs/api/FileSystemManager.cpp

@@ -43,7 +43,13 @@ void FileSystemManager::Shutdown() {
   mShutdown.Flip();
 
   if (mBackgroundRequestHandler->FileSystemManagerChildStrongRef()) {
-    mBackgroundRequestHandler->FileSystemManagerChildStrongRef()->CloseAll();
+    // FileSystemAccessHandles prevent shutdown until they are full closed, so
+    // at this point, we should see no open FileSystemAccessHandles.
+    MOZ_ASSERT(mBackgroundRequestHandler->FileSystemManagerChildStrongRef()
+                   ->AllSyncAccessHandlesClosed());
+
+    mBackgroundRequestHandler->FileSystemManagerChildStrongRef()
+        ->CloseAllWritableFileStreams();
   }
 
   mBackgroundRequestHandler->Shutdown();

dom/fs/api/FileSystemSyncAccessHandle.cpp

@@ -110,7 +110,7 @@ FileSystemSyncAccessHandle::FileSystemSyncAccessHandle(
 
 FileSystemSyncAccessHandle::~FileSystemSyncAccessHandle() {
   MOZ_ASSERT(!mActor);
-  MOZ_ASSERT(mState == State::Closed);
+  MOZ_ASSERT(IsClosed());
 }
 
 // static
@@ -137,7 +137,9 @@ FileSystemSyncAccessHandle::Create(
   RefPtr<StrongWorkerRef> workerRef = StrongWorkerRef::Create(
       workerPrivate, "FileSystemSyncAccessHandle", [result]() {
         if (result->IsOpen()) {
-          result->CloseInternal();
+          // We don't need to use the result, we just need to begin the closing
+          // process.
+          Unused << result->BeginClose();
         }
       });
   QM_TRY(MOZ_TO_RESULT(workerRef));
@@ -165,7 +167,9 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(FileSystemSyncAccessHandle)
   // Don't unlink mManager!
   NS_IMPL_CYCLE_COLLECTION_UNLINK_PRESERVED_WRAPPER
   if (tmp->IsOpen()) {
-    tmp->CloseInternal();
+    // We don't need to use the result, we just need to begin the closing
+    // process.
+    Unused << tmp->BeginClose();
   }
 NS_IMPL_CYCLE_COLLECTION_UNLINK_END
 NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(FileSystemSyncAccessHandle)
@@ -200,7 +204,13 @@ bool FileSystemSyncAccessHandle::IsOpen() const {
   return mState == State::Open;
 }
 
-void FileSystemSyncAccessHandle::CloseInternal() {
+bool FileSystemSyncAccessHandle::IsClosed() const {
+  MOZ_ASSERT(mState != State::Initial);
+
+  return mState == State::Closed;
+}
+
+RefPtr<BoolPromise> FileSystemSyncAccessHandle::BeginClose() {
   MOZ_ASSERT(IsOpen());
 
   LOG(("%p: Closing", mStream.get()));
@@ -215,6 +225,8 @@ void FileSystemSyncAccessHandle::CloseInternal() {
   }
 
   mWorkerRef = nullptr;
+
+  return BoolPromise::CreateAndResolve(true, __func__);
 }
 
 // WebIDL Boilerplate
@@ -295,9 +307,31 @@ void FileSystemSyncAccessHandle::Flush(ErrorResult& aError) {
 }
 
 void FileSystemSyncAccessHandle::Close() {
-  if (IsOpen()) {
-    CloseInternal();
+  if (!IsOpen()) {
+    return;
   }
+
+  // Normally mWorkerRef can be used directly for stopping the sync loop, but
+  // the async close is special because mWorkerRef is cleared as part of the
+  // operation. That's why we need to use this extra strong ref to the
+  // `StrongWorkerRef`.
+  RefPtr<StrongWorkerRef> workerRef = mWorkerRef;
+
+  AutoSyncLoopHolder syncLoop(workerRef->Private(), Killing);
+
+  nsCOMPtr<nsISerialEventTarget> syncLoopTarget =
+      syncLoop.GetSerialEventTarget();
+  MOZ_ASSERT(syncLoopTarget);
+
+  InvokeAsync(syncLoopTarget, __func__, [self = RefPtr(this)]() {
+    return self->BeginClose();
+  })->Then(syncLoopTarget, __func__, [&workerRef, &syncLoopTarget]() {
+    workerRef->Private()->AssertIsOnWorkerThread();
+
+    workerRef->Private()->StopSyncLoop(syncLoopTarget, NS_OK);
+  });
+
+  MOZ_ALWAYS_SUCCEEDS(syncLoop.Run());
 }
 
 uint64_t FileSystemSyncAccessHandle::ReadOrWrite(

dom/fs/api/FileSystemSyncAccessHandle.h

@@ -8,6 +8,7 @@
 #define DOM_FS_FILESYSTEMSYNCACCESSHANDLE_H_
 
 #include "mozilla/dom/PFileSystemManager.h"
+#include "mozilla/dom/quota/ForwardDecls.h"
 #include "nsCOMPtr.h"
 #include "nsISupports.h"
 #include "nsWrapperCache.h"
@@ -47,7 +48,9 @@ class FileSystemSyncAccessHandle final : public nsISupports,
 
   bool IsOpen() const;
 
-  void CloseInternal();
+  bool IsClosed() const;
+
+  [[nodiscard]] RefPtr<BoolPromise> BeginClose();
 
   // WebIDL Boilerplate
   nsIGlobalObject* GetParentObject() const;

dom/fs/child/FileSystemManagerChild.cpp

@@ -13,20 +13,22 @@
 
 namespace mozilla::dom {
 
-void FileSystemManagerChild::CloseAll() {
-  // NOTE: getFile() creates blobs that read the data from the child;
-  // we'll need to abort any reads and resolve this call only when all
-  // blobs are closed.
-
+#ifdef DEBUG
+bool FileSystemManagerChild::AllSyncAccessHandlesClosed() const {
   for (const auto& item : ManagedPFileSystemAccessHandleChild()) {
     auto* child = static_cast<FileSystemAccessHandleChild*>(item);
     auto* handle = child->MutableAccessHandlePtr();
 
-    if (handle->IsOpen()) {
-      handle->CloseInternal();
+    if (!handle->IsClosed()) {
+      return false;
     }
   }
 
+  return true;
+}
+#endif
+
+void FileSystemManagerChild::CloseAllWritableFileStreams() {
   for (const auto& item : ManagedPFileSystemWritableFileStreamChild()) {
     auto* child = static_cast<FileSystemWritableFileStreamChild*>(item);
 
@@ -54,9 +56,29 @@ FileSystemManagerChild::AllocPFileSystemWritableFileStreamChild() {
 
 ::mozilla::ipc::IPCResult FileSystemManagerChild::RecvCloseAll(
     CloseAllResolver&& aResolver) {
-  CloseAll();
+  nsTArray<RefPtr<BoolPromise>> promises;
+
+  // NOTE: getFile() creates blobs that read the data from the child;
+  // we'll need to abort any reads and resolve this call only when all
+  // blobs are closed.
+
+  for (const auto& item : ManagedPFileSystemAccessHandleChild()) {
+    auto* child = static_cast<FileSystemAccessHandleChild*>(item);
+    auto* handle = child->MutableAccessHandlePtr();
+
+    if (handle->IsOpen()) {
+      promises.AppendElement(handle->BeginClose());
+    }
+  }
+
+  CloseAllWritableFileStreams();
+
+  BoolPromise::AllSettled(GetCurrentSerialEventTarget(), promises)
+      ->Then(GetCurrentSerialEventTarget(), __func__,
+             [resolver = std::move(aResolver)](
+                 const BoolPromise::AllSettledPromiseType::ResolveOrRejectValue&
+                     aValues) { resolver(NS_OK); });
 
-  aResolver(NS_OK);
   return IPC_OK();
 }
 

dom/fs/child/FileSystemManagerChild.h

@@ -16,7 +16,11 @@ class FileSystemManagerChild : public PFileSystemManagerChild {
  public:
   NS_INLINE_DECL_REFCOUNTING_WITH_DESTROY(FileSystemManagerChild, Destroy())
 
-  virtual void CloseAll();
+#ifdef DEBUG
+  virtual bool AllSyncAccessHandlesClosed() const;
+#endif
+
+  virtual void CloseAllWritableFileStreams();
 
   virtual void Shutdown();