Bug 1806490 - Make it possible to track all CreateFileSystemManagerChild requests in FileSystemManager; r=dom-storage-reviewers,jari

It can happen that BeginRequest is called multiple times when the actor doesn't
exist yet. Such situations can't be handled by a single MozPromiseRequestHolder.

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

dom/fs/api/FileSystemManager.cpp

@@ -12,6 +12,7 @@
 #include "mozilla/dom/FileSystemManagerChild.h"
 #include "mozilla/dom/Promise.h"
 #include "mozilla/dom/StorageManager.h"
+#include "mozilla/dom/fs/ManagedMozPromiseRequestHolder.h"
 #include "mozilla/dom/quota/QuotaCommon.h"
 #include "mozilla/dom/quota/ResultExtensions.h"
 
@@ -54,7 +55,20 @@ void FileSystemManager::Shutdown() {
 
   mBackgroundRequestHandler->Shutdown();
 
-  mCreateFileSystemManagerChildPromiseRequestHolder.DisconnectIfExists();
+  for (RefPtr<PromiseRequestHolder<BoolPromise>> holder :
+       mPromiseRequestHolders.ForwardRange()) {
+    holder->DisconnectIfExists();
+  }
+}
+
+void FileSystemManager::RegisterPromiseRequestHolder(
+    PromiseRequestHolder<BoolPromise>* aHolder) {
+  mPromiseRequestHolders.AppendElement(aHolder);
+}
+
+void FileSystemManager::UnregisterPromiseRequestHolder(
+    PromiseRequestHolder<BoolPromise>* aHolder) {
+  mPromiseRequestHolders.RemoveElement(aHolder);
 }
 
 void FileSystemManager::BeginRequest(
@@ -78,22 +92,23 @@ void FileSystemManager::BeginRequest(
   QM_TRY_INSPECT(const auto& principalInfo, mGlobal->GetStorageKey(), QM_VOID,
                  [&aFailure](nsresult rv) { aFailure(rv); });
 
-  mBackgroundRequestHandler->CreateFileSystemManagerChild(principalInfo)
-      ->Then(
-          GetCurrentSerialEventTarget(), __func__,
-          [self = RefPtr<FileSystemManager>(this),
-           success = std::move(aSuccess), failure = std::move(aFailure)](
-              const BoolPromise::ResolveOrRejectValue& aValue) {
-            self->mCreateFileSystemManagerChildPromiseRequestHolder.Complete();
+  auto holder = MakeRefPtr<PromiseRequestHolder<BoolPromise>>(this);
 
-            if (aValue.IsResolve()) {
-              success(self->mBackgroundRequestHandler
-                          ->FileSystemManagerChildStrongRef());
-            } else {
-              failure(aValue.RejectValue());
-            }
-          })
-      ->Track(mCreateFileSystemManagerChildPromiseRequestHolder);
+  mBackgroundRequestHandler->CreateFileSystemManagerChild(principalInfo)
+      ->Then(GetCurrentSerialEventTarget(), __func__,
+             [self = RefPtr<FileSystemManager>(this), holder,
+              success = std::move(aSuccess), failure = std::move(aFailure)](
+                 const BoolPromise::ResolveOrRejectValue& aValue) {
+               holder->Complete();
+
+               if (aValue.IsResolve()) {
+                 success(self->mBackgroundRequestHandler
+                             ->FileSystemManagerChildStrongRef());
+               } else {
+                 failure(aValue.RejectValue());
+               }
+             })
+      ->Track(*holder);
 }
 
 already_AddRefed<Promise> FileSystemManager::GetDirectory(ErrorResult& aError) {

dom/fs/api/FileSystemManager.h

@@ -16,6 +16,7 @@
 #include "nsCOMPtr.h"
 #include "nsCycleCollectionParticipant.h"
 #include "nsISupports.h"
+#include "nsTObserverArray.h"
 
 class nsIGlobalObject;
 
@@ -31,6 +32,8 @@ class StorageManager;
 
 namespace fs {
 class FileSystemRequestHandler;
+template <typename Manager, typename PromiseType>
+class ManagedMozPromiseRequestHolder;
 }  // namespace fs
 
 // `FileSystemManager` is supposed to be held by `StorageManager` and thus
@@ -47,6 +50,10 @@ class FileSystemRequestHandler;
 // the whole chain of managed actors would be destroyed as well.
 class FileSystemManager : public nsISupports {
  public:
+  template <typename PromiseType>
+  using PromiseRequestHolder =
+      fs::ManagedMozPromiseRequestHolder<FileSystemManager, PromiseType>;
+
   FileSystemManager(
       nsIGlobalObject* aGlobal, RefPtr<StorageManager> aStorageManager,
       RefPtr<FileSystemBackgroundRequestHandler> aBackgroundRequestHandler);
@@ -61,6 +68,11 @@ class FileSystemManager : public nsISupports {
 
   void Shutdown();
 
+  void RegisterPromiseRequestHolder(PromiseRequestHolder<BoolPromise>* aHolder);
+
+  void UnregisterPromiseRequestHolder(
+      PromiseRequestHolder<BoolPromise>* aHolder);
+
   void BeginRequest(
       std::function<void(const RefPtr<FileSystemManagerChild>&)>&& aSuccess,
       std::function<void(nsresult)>&& aFailure);
@@ -77,8 +89,7 @@ class FileSystemManager : public nsISupports {
   const RefPtr<FileSystemBackgroundRequestHandler> mBackgroundRequestHandler;
   const UniquePtr<fs::FileSystemRequestHandler> mRequestHandler;
 
-  MozPromiseRequestHolder<BoolPromise>
-      mCreateFileSystemManagerChildPromiseRequestHolder;
+  nsTObserverArray<PromiseRequestHolder<BoolPromise>*> mPromiseRequestHolders;
 
   FlippedOnce<false> mShutdown;
 };

dom/fs/shared/ManagedMozPromiseRequestHolder.h

@@ -0,0 +1,37 @@
+/* -*- 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/. */
+
+#ifndef DOM_FS_SHARED_MANAGEDMOZPROMISEREQUESTHOLDER_H_
+#define DOM_FS_SHARED_MANAGEDMOZPROMISEREQUESTHOLDER_H_
+
+#include "mozilla/MozPromise.h"
+#include "mozilla/RefPtr.h"
+#include "nsISupportsImpl.h"
+
+namespace mozilla::dom::fs {
+
+template <typename Manager, typename PromiseType>
+class ManagedMozPromiseRequestHolder final
+    : public MozPromiseRequestHolder<PromiseType> {
+ public:
+  explicit ManagedMozPromiseRequestHolder(Manager* aManager)
+      : mManager(aManager) {
+    mManager->RegisterPromiseRequestHolder(this);
+  }
+
+  NS_INLINE_DECL_REFCOUNTING(ManagedMozPromiseRequestHolder)
+
+ private:
+  ~ManagedMozPromiseRequestHolder() {
+    mManager->UnregisterPromiseRequestHolder(this);
+  }
+
+  RefPtr<Manager> mManager;
+};
+
+}  // namespace mozilla::dom::fs
+
+#endif  // DOM_FS_SHARED_MANAGEDMOZPROMISEREQUESTHOLDER_H_

dom/fs/shared/moz.build

@@ -11,6 +11,7 @@ EXPORTS.mozilla.dom += [
 ]
 
 EXPORTS.mozilla.dom.fs += [
+    "ManagedMozPromiseRequestHolder.h",
     "TargetPtrHolder.h",
 ]
 

dom/fs/test/common/test_basics.js

@@ -3,6 +3,16 @@
  * http://creativecommons.org/publicdomain/zero/1.0/
  */
 
+// This test must be first, since we need the actor not to be created already.
+exported_symbols.testGetDirectoryTwice = async function() {
+  const promise1 = navigator.storage.getDirectory();
+  const promise2 = navigator.storage.getDirectory();
+
+  await Promise.all([promise1, promise2]);
+
+  Assert.ok(true, "Should not have thrown");
+};
+
 exported_symbols.testGetDirectoryDoesNotThrow = async function() {
   await navigator.storage.getDirectory();