From 7bdbae3ca72354bcf985c3ae71dba521011f5be6 Mon Sep 17 00:00:00 2001 From: Jan Varga Date: Wed, 7 Jun 2023 04:54:30 +0000 Subject: [PATCH] Bug 1825552 - Avoid the new/delete pattern; r=dom-storage-reviewers,jari Differential Revision: https://phabricator.services.mozilla.com/D179559 --- dom/fs/api/FileSystemWritableFileStream.cpp | 112 ++++++++++-------- dom/fs/api/FileSystemWritableFileStream.h | 7 +- .../child/FileSystemThreadSafeStreamOwner.cpp | 18 ++- .../fs/FileSystemThreadSafeStreamOwner.h | 15 ++- 4 files changed, 97 insertions(+), 55 deletions(-) diff --git a/dom/fs/api/FileSystemWritableFileStream.cpp b/dom/fs/api/FileSystemWritableFileStream.cpp index 39896b8a5ee5..f8b23c21ed1f 100644 --- a/dom/fs/api/FileSystemWritableFileStream.cpp +++ b/dom/fs/api/FileSystemWritableFileStream.cpp @@ -84,7 +84,7 @@ void WriteImpl(RefPtr aStream, RefPtr& aOutStreamOwner, const Maybe aPosition, const RefPtr& aPromise) { - aStream->NoteActiveCommand(); + auto command = aStream->CreateCommand(); InvokeAsync( aTaskQueue, __func__, @@ -130,10 +130,8 @@ void WriteImpl(RefPtr 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 aStream, } // namespace +class FileSystemWritableFileStream::Command { + public: + explicit Command(RefPtr aWritableFileStream) + : mWritableFileStream(std::move(aWritableFileStream)) { + MOZ_ASSERT(mWritableFileStream); + } + + NS_INLINE_DECL_REFCOUNTING(FileSystemWritableFileStream::Command) + + private: + ~Command() { mWritableFileStream->NoteFinishedCommand(); } + + RefPtr 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( - std::move(aStream))), + this, std::move(aStream))), mWorkerRef(), mMetadata(std::move(aMetadata)), mCloseHandler(MakeAndAddRef()), @@ -421,26 +434,25 @@ void FileSystemWritableFileStream::LastRelease() { } } +RefPtr +FileSystemWritableFileStream::CreateCommand() { + MOZ_ASSERT(!mCommandActive); + + mCommandActive = true; + + return MakeRefPtr(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& 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 FileSystemWritableFileStream::Finish() { diff --git a/dom/fs/api/FileSystemWritableFileStream.h b/dom/fs/api/FileSystemWritableFileStream.h index b434990c44f7..5324f1398b42 100644 --- a/dom/fs/api/FileSystemWritableFileStream.h +++ b/dom/fs/api/FileSystemWritableFileStream.h @@ -63,9 +63,10 @@ class FileSystemWritableFileStream final : public WritableStream { void ClearActor(); - void NoteActiveCommand(); + class Command; + RefPtr 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& aPromise); + void NoteFinishedCommand(); + [[nodiscard]] RefPtr Finish(); RefPtr mManager; diff --git a/dom/fs/child/FileSystemThreadSafeStreamOwner.cpp b/dom/fs/child/FileSystemThreadSafeStreamOwner.cpp index ec55d860705a..2b2d2913d903 100644 --- a/dom/fs/child/FileSystemThreadSafeStreamOwner.cpp +++ b/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& aStream, int64_t aEOF) { } // namespace FileSystemThreadSafeStreamOwner::FileSystemThreadSafeStreamOwner( + FileSystemWritableFileStream* aWritableFileStream, nsCOMPtr&& aStream) - : mStream(std::forward>(aStream)), - mClosed(false) {} + : +#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED + mWritableFileStream(aWritableFileStream), +#endif + mStream(std::forward>(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 FileSystemThreadSafeStreamOwner::OutputStream() { + MOZ_DIAGNOSTIC_ASSERT(mWritableFileStream->IsCommandActive()); + return mStream->OutputStream(); } diff --git a/dom/fs/include/fs/FileSystemThreadSafeStreamOwner.h b/dom/fs/include/fs/FileSystemThreadSafeStreamOwner.h index 0de0af39556a..3ef7639f363c 100644 --- a/dom/fs/include/fs/FileSystemThreadSafeStreamOwner.h +++ b/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&& 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 mStream; bool mClosed; }; -} // namespace mozilla::dom::fs +} // namespace fs +} // namespace mozilla::dom #endif // DOM_FS_FILESYSTEMTHREADSAFESTREAMOWNER_H_