Bug 1825552 - Avoid the new/delete pattern; r=dom-storage-reviewers,jari, a=dmeehan

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

dom/fs/api/FileSystemWritableFileStream.cpp

@@ -84,7 +84,7 @@ void WriteImpl(RefPtr<FileSystemWritableFileStream> aStream,
                RefPtr<fs::FileSystemThreadSafeStreamOwner>& aOutStreamOwner,
                const Maybe<uint64_t> aPosition,
                const RefPtr<Promise>& aPromise) {
-  aStream->NoteActiveCommand();
+  auto command = aStream->CreateCommand();
 
   InvokeAsync(
       aTaskQueue, __func__,
@@ -130,10 +130,8 @@ void WriteImpl(RefPtr<FileSystemWritableFileStream> aStream,
       })
       ->Then(
           GetCurrentSerialEventTarget(), __func__,
-          [aStream,
+          [command,
            aPromise](const Int64Promise::ResolveOrRejectValue& aValue) {
-            aStream->NoteFinishedCommand();
-
             if (aValue.IsResolve()) {
               aPromise->MaybeResolve(aValue.ResolveValue());
               return;
@@ -153,6 +151,21 @@ void WriteImpl(RefPtr<FileSystemWritableFileStream> aStream,
 
 }  // namespace
 
+class FileSystemWritableFileStream::Command {
+ public:
+  explicit Command(RefPtr<FileSystemWritableFileStream> aWritableFileStream)
+      : mWritableFileStream(std::move(aWritableFileStream)) {
+    MOZ_ASSERT(mWritableFileStream);
+  }
+
+  NS_INLINE_DECL_REFCOUNTING(FileSystemWritableFileStream::Command)
+
+ private:
+  ~Command() { mWritableFileStream->NoteFinishedCommand(); }
+
+  RefPtr<FileSystemWritableFileStream> mWritableFileStream;
+};
+
 class FileSystemWritableFileStream::CloseHandler {
   enum struct State : uint8_t { Initial = 0, Open, Closing, Closed };
 
@@ -246,7 +259,7 @@ FileSystemWritableFileStream::FileSystemWritableFileStream(
       mActor(std::move(aActor)),
       mTaskQueue(aTaskQueue),
       mStreamOwner(MakeAndAddRef<fs::FileSystemThreadSafeStreamOwner>(
-          std::move(aStream))),
+          this, std::move(aStream))),
       mWorkerRef(),
       mMetadata(std::move(aMetadata)),
       mCloseHandler(MakeAndAddRef<CloseHandler>()),
@@ -421,26 +434,25 @@ void FileSystemWritableFileStream::LastRelease() {
   }
 }
 
+RefPtr<FileSystemWritableFileStream::Command>
+FileSystemWritableFileStream::CreateCommand() {
+  MOZ_ASSERT(!mCommandActive);
+
+  mCommandActive = true;
+
+  return MakeRefPtr<Command>(this);
+}
+
+bool FileSystemWritableFileStream::IsCommandActive() const {
+  return mCommandActive;
+}
+
 void FileSystemWritableFileStream::ClearActor() {
   MOZ_ASSERT(mActor);
 
   mActor = nullptr;
 }
 
-void FileSystemWritableFileStream::NoteActiveCommand() {
-  MOZ_ASSERT(!mCommandActive);
-
-  mCommandActive = true;
-}
-
-void FileSystemWritableFileStream::NoteFinishedCommand() {
-  MOZ_ASSERT(mCommandActive);
-
-  mCommandActive = false;
-
-  mFinishPromiseHolder.ResolveIfExists(true, __func__);
-}
-
 bool FileSystemWritableFileStream::IsOpen() const {
   return mCloseHandler->IsOpen();
 }
@@ -803,7 +815,7 @@ void FileSystemWritableFileStream::Seek(uint64_t aPosition,
 
   LOG_VERBOSE(("%p: Seeking to %" PRIu64, mStreamOwner.get(), aPosition));
 
-  NoteActiveCommand();
+  auto command = CreateCommand();
 
   InvokeAsync(mTaskQueue, __func__,
               [aPosition, streamOwner = mStreamOwner]() mutable {
@@ -812,30 +824,28 @@ void FileSystemWritableFileStream::Seek(uint64_t aPosition,
 
                 return BoolPromise::CreateAndResolve(true, __func__);
               })
-      ->Then(GetCurrentSerialEventTarget(), __func__,
-             [self = RefPtr(this),
-              aPromise](const BoolPromise::ResolveOrRejectValue& aValue) {
-               self->NoteFinishedCommand();
-
-               if (aValue.IsReject()) {
-                 auto rv = aValue.RejectValue();
-                 if (IsFileNotFoundError(rv)) {
-                   aPromise->MaybeRejectWithNotFoundError("File not found");
-                   return;
-                 }
-                 aPromise->MaybeReject(rv);
-                 return;
-               }
-               MOZ_ASSERT(aValue.IsResolve());
-               aPromise->MaybeResolveWithUndefined();
-             });
+      ->Then(
+          GetCurrentSerialEventTarget(), __func__,
+          [command, aPromise](const BoolPromise::ResolveOrRejectValue& aValue) {
+            if (aValue.IsReject()) {
+              auto rv = aValue.RejectValue();
+              if (IsFileNotFoundError(rv)) {
+                aPromise->MaybeRejectWithNotFoundError("File not found");
+                return;
+              }
+              aPromise->MaybeReject(rv);
+              return;
+            }
+            MOZ_ASSERT(aValue.IsResolve());
+            aPromise->MaybeResolveWithUndefined();
+          });
 }
 
 void FileSystemWritableFileStream::Truncate(uint64_t aSize,
                                             const RefPtr<Promise>& aPromise) {
   MOZ_ASSERT(IsOpen());
 
-  NoteActiveCommand();
+  auto command = CreateCommand();
 
   InvokeAsync(mTaskQueue, __func__,
               [aSize, streamOwner = mStreamOwner]() mutable {
@@ -844,18 +854,24 @@ void FileSystemWritableFileStream::Truncate(uint64_t aSize,
 
                 return BoolPromise::CreateAndResolve(true, __func__);
               })
-      ->Then(GetCurrentSerialEventTarget(), __func__,
-             [self = RefPtr(this),
-              aPromise](const BoolPromise::ResolveOrRejectValue& aValue) {
-               self->NoteFinishedCommand();
+      ->Then(
+          GetCurrentSerialEventTarget(), __func__,
+          [command, aPromise](const BoolPromise::ResolveOrRejectValue& aValue) {
+            if (aValue.IsReject()) {
+              aPromise->MaybeReject(aValue.RejectValue());
+              return;
+            }
 
-               if (aValue.IsReject()) {
-                 aPromise->MaybeReject(aValue.RejectValue());
-                 return;
-               }
+            aPromise->MaybeResolveWithUndefined();
+          });
+}
 
-               aPromise->MaybeResolveWithUndefined();
-             });
+void FileSystemWritableFileStream::NoteFinishedCommand() {
+  MOZ_ASSERT(mCommandActive);
+
+  mCommandActive = false;
+
+  mFinishPromiseHolder.ResolveIfExists(true, __func__);
 }
 
 RefPtr<BoolPromise> FileSystemWritableFileStream::Finish() {

dom/fs/api/FileSystemWritableFileStream.h

@@ -63,9 +63,10 @@ class FileSystemWritableFileStream final : public WritableStream {
 
   void ClearActor();
 
-  void NoteActiveCommand();
+  class Command;
+  RefPtr<Command> CreateCommand();
 
-  void NoteFinishedCommand();
+  bool IsCommandActive() const;
 
   bool IsOpen() const;
 
@@ -113,6 +114,8 @@ class FileSystemWritableFileStream final : public WritableStream {
 
   void Truncate(uint64_t aSize, const RefPtr<Promise>& aPromise);
 
+  void NoteFinishedCommand();
+
   [[nodiscard]] RefPtr<BoolPromise> Finish();
 
   RefPtr<FileSystemManager> mManager;

dom/fs/child/FileSystemThreadSafeStreamOwner.cpp

@@ -8,6 +8,7 @@
 
 #include "mozilla/CheckedInt.h"
 #include "mozilla/dom/FileSystemLog.h"
+#include "mozilla/dom/FileSystemWritableFileStream.h"
 #include "mozilla/dom/quota/QuotaCommon.h"
 #include "mozilla/dom/quota/ResultExtensions.h"
 #include "nsIOutputStream.h"
@@ -36,11 +37,20 @@ nsresult TruncFile(nsCOMPtr<nsIRandomAccessStream>& aStream, int64_t aEOF) {
 }  // namespace
 
 FileSystemThreadSafeStreamOwner::FileSystemThreadSafeStreamOwner(
+    FileSystemWritableFileStream* aWritableFileStream,
     nsCOMPtr<nsIRandomAccessStream>&& aStream)
-    : mStream(std::forward<nsCOMPtr<nsIRandomAccessStream>>(aStream)),
-      mClosed(false) {}
+    :
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+      mWritableFileStream(aWritableFileStream),
+#endif
+      mStream(std::forward<nsCOMPtr<nsIRandomAccessStream>>(aStream)),
+      mClosed(false) {
+  MOZ_ASSERT(mWritableFileStream);
+}
 
 nsresult FileSystemThreadSafeStreamOwner::Truncate(uint64_t aSize) {
+  MOZ_DIAGNOSTIC_ASSERT(mWritableFileStream->IsCommandActive());
+
   if (mClosed) {  // Multiple closes can end up in a queue
     return NS_ERROR_DOM_INVALID_STATE_ERR;
   }
@@ -62,6 +72,8 @@ nsresult FileSystemThreadSafeStreamOwner::Truncate(uint64_t aSize) {
 }
 
 nsresult FileSystemThreadSafeStreamOwner::Seek(uint64_t aPosition) {
+  MOZ_DIAGNOSTIC_ASSERT(mWritableFileStream->IsCommandActive());
+
   if (mClosed) {  // Multiple closes can end up in a queue
     return NS_ERROR_DOM_INVALID_STATE_ERR;
   }
@@ -84,6 +96,8 @@ void FileSystemThreadSafeStreamOwner::Close() {
 }
 
 nsCOMPtr<nsIOutputStream> FileSystemThreadSafeStreamOwner::OutputStream() {
+  MOZ_DIAGNOSTIC_ASSERT(mWritableFileStream->IsCommandActive());
+
   return mStream->OutputStream();
 }
 

dom/fs/include/fs/FileSystemThreadSafeStreamOwner.h

@@ -12,11 +12,16 @@
 class nsIOutputStream;
 class nsIRandomAccessStream;
 
-namespace mozilla::dom::fs {
+namespace mozilla::dom {
+
+class FileSystemWritableFileStream;
+
+namespace fs {
 
 class FileSystemThreadSafeStreamOwner {
  public:
-  explicit FileSystemThreadSafeStreamOwner(
+  FileSystemThreadSafeStreamOwner(
+      FileSystemWritableFileStream* aWritableFileStream,
       nsCOMPtr<nsIRandomAccessStream>&& aStream);
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(FileSystemThreadSafeStreamOwner)
@@ -33,11 +38,15 @@ class FileSystemThreadSafeStreamOwner {
   virtual ~FileSystemThreadSafeStreamOwner() = default;
 
  private:
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+  FileSystemWritableFileStream* MOZ_NON_OWNING_REF mWritableFileStream;
+#endif
   nsCOMPtr<nsIRandomAccessStream> mStream;
 
   bool mClosed;
 };
 
-}  // namespace mozilla::dom::fs
+}  // namespace fs
+}  // namespace mozilla::dom
 
 #endif  // DOM_FS_FILESYSTEMTHREADSAFESTREAMOWNER_H_