Bug 1809064 - Wait for close to be finished in the parent before resolving the promise for FileSystemSyncAccessHandle::BeginClose; r=dom-storage-reviewers,jari

The close in the parent is going to be asynchronous as well, so it won't be
guaranteed anymore that the unlocking happens synchronously immediatelly after
receiving the close message.

Depends on D166343

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

dom/fs/api/FileSystemSyncAccessHandle.cpp

@@ -24,6 +24,7 @@
 #include "mozilla/dom/WorkerCommon.h"
 #include "mozilla/dom/WorkerPrivate.h"
 #include "mozilla/dom/WorkerRef.h"
+#include "mozilla/dom/fs/IPCRejectReporter.h"
 #include "mozilla/dom/fs/TargetPtrHolder.h"
 #include "mozilla/dom/quota/QuotaCommon.h"
 #include "mozilla/dom/quota/ResultExtensions.h"
@@ -299,16 +300,31 @@ RefPtr<BoolPromise> FileSystemSyncAccessHandle::BeginClose() {
       ->Then(
           mWorkerRef->Private()->ControlEventTarget(), __func__,
           [self = RefPtr(this)](const ShutdownPromise::ResolveOrRejectValue&) {
-            if (self->mActor) {
-              self->mActor->SendClose();
+            if (self->mControlActor) {
+              RefPtr<BoolPromise::Private> promise =
+                  new BoolPromise::Private(__func__);
+
+              self->mControlActor->SendClose(
+                  [promise](void_t&&) { promise->Resolve(true, __func__); },
+                  [promise](const mozilla::ipc::ResponseRejectReason& aReason) {
+                    fs::IPCRejectReporter(aReason);
+
+                    promise->Reject(NS_ERROR_FAILURE, __func__);
+                  });
+
+              return RefPtr<BoolPromise>(promise);
             }
 
-            self->mWorkerRef = nullptr;
+            return BoolPromise::CreateAndResolve(true, __func__);
+          })
+      ->Then(mWorkerRef->Private()->ControlEventTarget(), __func__,
+             [self = RefPtr(this)](const BoolPromise::ResolveOrRejectValue&) {
+               self->mWorkerRef = nullptr;
 
-            self->mState = State::Closed;
+               self->mState = State::Closed;
 
-            self->mClosePromiseHolder.ResolveIfExists(true, __func__);
-          });
+               self->mClosePromiseHolder.ResolveIfExists(true, __func__);
+             });
 
   return OnClose();
 }

dom/fs/child/FileSystemRequestHandler.cpp

@@ -25,6 +25,7 @@
 #include "mozilla/dom/FileSystemWritableFileStreamChild.h"
 #include "mozilla/dom/IPCBlobUtils.h"
 #include "mozilla/dom/Promise.h"
+#include "mozilla/dom/fs/IPCRejectReporter.h"
 #include "mozilla/dom/quota/QuotaCommon.h"
 #include "mozilla/dom/quota/ResultExtensions.h"
 
@@ -270,30 +271,6 @@ mozilla::ipc::ResolveCallback<TResponse> SelectResolveCallback(
                 aPromise, TReturns(), std::forward<Args>(args)...));
 }
 
-// TODO: Find a better way to deal with these errors
-void IPCRejectReporter(mozilla::ipc::ResponseRejectReason aReason) {
-  switch (aReason) {
-    case mozilla::ipc::ResponseRejectReason::ActorDestroyed:
-      // This is ok
-      break;
-    case mozilla::ipc::ResponseRejectReason::HandlerRejected:
-      QM_TRY(OkIf(false), QM_VOID);
-      break;
-    case mozilla::ipc::ResponseRejectReason::ChannelClosed:
-      QM_TRY(OkIf(false), QM_VOID);
-      break;
-    case mozilla::ipc::ResponseRejectReason::ResolverDestroyed:
-      QM_TRY(OkIf(false), QM_VOID);
-      break;
-    case mozilla::ipc::ResponseRejectReason::SendError:
-      QM_TRY(OkIf(false), QM_VOID);
-      break;
-    default:
-      QM_TRY(OkIf(false), QM_VOID);
-      break;
-  }
-}
-
 void RejectCallback(
     RefPtr<Promise> aPromise,  // NOLINT(performance-unnecessary-value-param)
     mozilla::ipc::ResponseRejectReason aReason) {

dom/fs/parent/FileSystemAccessHandle.cpp

@@ -74,7 +74,7 @@ void FileSystemAccessHandle::Unregister() {
   --mRegCount;
 
   if (IsInactive() && IsOpen()) {
-    Close();
+    BeginClose();
   }
 }
 
@@ -93,7 +93,7 @@ void FileSystemAccessHandle::UnregisterActor(
   mActor = nullptr;
 
   if (IsInactive() && IsOpen()) {
-    Close();
+    BeginClose();
   }
 }
 
@@ -112,13 +112,13 @@ void FileSystemAccessHandle::UnregisterControlActor(
   mControlActor = nullptr;
 
   if (IsInactive() && IsOpen()) {
-    Close();
+    BeginClose();
   }
 }
 
 bool FileSystemAccessHandle::IsOpen() const { return !mClosed; }
 
-void FileSystemAccessHandle::Close() {
+RefPtr<BoolPromise> FileSystemAccessHandle::BeginClose() {
   MOZ_ASSERT(IsOpen());
 
   LOG(("Closing AccessHandle"));
@@ -133,16 +133,17 @@ void FileSystemAccessHandle::Close() {
     mDataManager->UnlockExclusive(mEntryId);
   }
 
-  InvokeAsync(mDataManager->MutableBackgroundTargetPtr(), __func__,
-              [self = RefPtr(this)]() {
-                if (self->mRegistered) {
-                  self->mDataManager->UnregisterAccessHandle(WrapNotNull(self));
-                }
+  return InvokeAsync(
+      mDataManager->MutableBackgroundTargetPtr(), __func__,
+      [self = RefPtr(this)]() {
+        if (self->mRegistered) {
+          self->mDataManager->UnregisterAccessHandle(WrapNotNull(self));
+        }
 
-                self->mDataManager = nullptr;
+        self->mDataManager = nullptr;
 
-                return BoolPromise::CreateAndResolve(true, __func__);
-              });
+        return BoolPromise::CreateAndResolve(true, __func__);
+      });
 }
 
 bool FileSystemAccessHandle::IsInactive() const {

dom/fs/parent/FileSystemAccessHandle.h

@@ -72,7 +72,7 @@ class FileSystemAccessHandle : public FileSystemStreamCallbacks {
 
   bool IsOpen() const;
 
-  void Close();
+  RefPtr<BoolPromise> BeginClose();
 
  private:
   FileSystemAccessHandle(RefPtr<fs::data::FileSystemDataManager> aDataManager,

dom/fs/parent/FileSystemAccessHandleControlParent.cpp

@@ -7,6 +7,7 @@
 #include "FileSystemAccessHandleControlParent.h"
 
 #include "mozilla/dom/FileSystemAccessHandle.h"
+#include "mozilla/ipc/IPCCore.h"
 
 namespace mozilla::dom {
 
@@ -18,7 +19,13 @@ FileSystemAccessHandleControlParent::~FileSystemAccessHandleControlParent() {
   MOZ_ASSERT(mActorDestroyed);
 }
 
-mozilla::ipc::IPCResult FileSystemAccessHandleControlParent::RecvFoo() {
+mozilla::ipc::IPCResult FileSystemAccessHandleControlParent::RecvClose(
+    CloseResolver&& aResolver) {
+  mAccessHandle->BeginClose()->Then(
+      GetCurrentSerialEventTarget(), __func__,
+      [resolver = std::move(aResolver)](
+          const BoolPromise::ResolveOrRejectValue&) { resolver(void_t()); });
+
   return IPC_OK();
 }
 

dom/fs/parent/FileSystemAccessHandleControlParent.h

@@ -23,7 +23,7 @@ class FileSystemAccessHandleControlParent
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FileSystemAccessHandleControlParent,
                                         override)
 
-  mozilla::ipc::IPCResult RecvFoo();
+  mozilla::ipc::IPCResult RecvClose(CloseResolver&& aResolver);
 
   void ActorDestroy(ActorDestroyReason aWhy) override;
 

dom/fs/parent/FileSystemAccessHandleParent.cpp

@@ -19,7 +19,7 @@ FileSystemAccessHandleParent::~FileSystemAccessHandleParent() {
 }
 
 mozilla::ipc::IPCResult FileSystemAccessHandleParent::RecvClose() {
-  mAccessHandle->Close();
+  mAccessHandle->BeginClose();
 
   return IPC_OK();
 }

dom/fs/shared/IPCRejectReporter.cpp

@@ -0,0 +1,36 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#include "mozilla/dom/quota/QuotaCommon.h"
+#include "mozilla/ipc/MessageChannel.h"
+
+namespace mozilla::dom::fs {
+
+// TODO: Find a better way to deal with these errors
+void IPCRejectReporter(mozilla::ipc::ResponseRejectReason aReason) {
+  switch (aReason) {
+    case mozilla::ipc::ResponseRejectReason::ActorDestroyed:
+      // This is ok
+      break;
+    case mozilla::ipc::ResponseRejectReason::HandlerRejected:
+      QM_TRY(OkIf(false), QM_VOID);
+      break;
+    case mozilla::ipc::ResponseRejectReason::ChannelClosed:
+      QM_TRY(OkIf(false), QM_VOID);
+      break;
+    case mozilla::ipc::ResponseRejectReason::ResolverDestroyed:
+      QM_TRY(OkIf(false), QM_VOID);
+      break;
+    case mozilla::ipc::ResponseRejectReason::SendError:
+      QM_TRY(OkIf(false), QM_VOID);
+      break;
+    default:
+      QM_TRY(OkIf(false), QM_VOID);
+      break;
+  }
+}
+
+}  // namespace mozilla::dom::fs

dom/fs/shared/IPCRejectReporter.h

@@ -0,0 +1,20 @@
+/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=8 sts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+namespace mozilla {
+
+namespace ipc {
+
+enum class ResponseRejectReason;
+
+}  // namespace ipc
+
+namespace dom::fs {
+
+void IPCRejectReporter(mozilla::ipc::ResponseRejectReason aReason);
+
+}  // namespace dom::fs
+}  // namespace mozilla

dom/fs/shared/PFileSystemAccessHandleControl.ipdl

@@ -2,15 +2,16 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+using struct mozilla::void_t from "mozilla/ipc/IPCCore.h";
+
 namespace mozilla {
 namespace dom {
 
 async protocol PFileSystemAccessHandleControl
 {
  parent:
-  // XXX A top level protocol must have at least one message. The foo message
-  // gets replaced with a real message in D166344.
-  async Foo();
+  async Close()
+      returns(void_t ok);
 };
 
 } // namespace dom

dom/fs/shared/moz.build

@@ -11,6 +11,7 @@ EXPORTS.mozilla.dom += [
 ]
 
 EXPORTS.mozilla.dom.fs += [
+    "IPCRejectReporter.h",
     "ManagedMozPromiseRequestHolder.h",
     "TargetPtrHolder.h",
 ]
@@ -18,6 +19,7 @@ EXPORTS.mozilla.dom.fs += [
 UNIFIED_SOURCES += [
     "FileSystemHelpers.cpp",
     "FileSystemLog.cpp",
+    "IPCRejectReporter.cpp",
 ]
 
 FINAL_LIBRARY = "xul"