From 18c7f261c4e70063eaa2faff20e587bf8a5aa514 Mon Sep 17 00:00:00 2001 From: Simon Giesecke Date: Tue, 21 Apr 2020 14:32:51 +0000 Subject: [PATCH] Bug 1623278 - Use SafeRefPtr for IDBTransaction. r=dom-workers-and-storage-reviewers,ttung,janv Differential Revision: https://phabricator.services.mozilla.com/D67287 --- dom/indexedDB/ActorsChild.cpp | 285 +++++++++++++++---------------- dom/indexedDB/ActorsChild.h | 33 ++-- dom/indexedDB/IDBCursor.cpp | 21 ++- dom/indexedDB/IDBDatabase.cpp | 44 ++--- dom/indexedDB/IDBDatabase.h | 4 +- dom/indexedDB/IDBIndex.cpp | 89 +++++----- dom/indexedDB/IDBObjectStore.cpp | 53 +++--- dom/indexedDB/IDBObjectStore.h | 29 +++- dom/indexedDB/IDBRequest.cpp | 22 ++- dom/indexedDB/IDBRequest.h | 47 +++-- dom/indexedDB/IDBTransaction.cpp | 36 ++-- dom/indexedDB/IDBTransaction.h | 14 +- dom/indexedDB/ProfilerHelpers.h | 8 +- js/src/new-regexp/RegExpAPI.cpp | 3 +- 14 files changed, 375 insertions(+), 313 deletions(-) diff --git a/dom/indexedDB/ActorsChild.cpp b/dom/indexedDB/ActorsChild.cpp index 26b883d0acec..3b28f16a4746 100644 --- a/dom/indexedDB/ActorsChild.cpp +++ b/dom/indexedDB/ActorsChild.cpp @@ -91,8 +91,7 @@ const uint32_t kFileCopyBufferSize = 32768; ******************************************************************************/ ThreadLocal::ThreadLocal(const nsID& aBackgroundChildLoggingId) - : mLoggingInfo(aBackgroundChildLoggingId, 1, -1, 1), - mCurrentTransaction(nullptr) { + : mLoggingInfo(aBackgroundChildLoggingId, 1, -1, 1) { MOZ_COUNT_CTOR(mozilla::dom::indexedDB::ThreadLocal); // NSID_LENGTH counts the null terminator, SetLength() does not. @@ -149,8 +148,8 @@ void MaybeCollectGarbageOnIPCMessage() { class MOZ_STACK_CLASS AutoSetCurrentTransaction final { typedef mozilla::ipc::BackgroundChildImpl BackgroundChildImpl; - IDBTransaction* const mTransaction; - IDBTransaction* mPreviousTransaction; + Maybe const mTransaction; + Maybe mPreviousTransaction; ThreadLocal* mThreadLocal; public: @@ -160,9 +159,9 @@ class MOZ_STACK_CLASS AutoSetCurrentTransaction final { delete; AutoSetCurrentTransaction& operator=(AutoSetCurrentTransaction&&) = delete; - explicit AutoSetCurrentTransaction(IDBTransaction* aTransaction) + explicit AutoSetCurrentTransaction(Maybe aTransaction) : mTransaction(aTransaction), - mPreviousTransaction(nullptr), + mPreviousTransaction(), mThreadLocal(nullptr) { if (aTransaction) { BackgroundChildImpl::ThreadLocal* threadLocal = @@ -174,7 +173,7 @@ class MOZ_STACK_CLASS AutoSetCurrentTransaction final { MOZ_ASSERT(mThreadLocal); // Save the current value. - mPreviousTransaction = mThreadLocal->GetCurrentTransaction(); + mPreviousTransaction = mThreadLocal->MaybeCurrentTransactionRef(); // Set the new value. mThreadLocal->SetCurrentTransaction(aTransaction); @@ -184,20 +183,20 @@ class MOZ_STACK_CLASS AutoSetCurrentTransaction final { ~AutoSetCurrentTransaction() { MOZ_ASSERT_IF(mThreadLocal, mTransaction); MOZ_ASSERT_IF(mThreadLocal, - mThreadLocal->GetCurrentTransaction() == mTransaction); + ReferenceEquals(mThreadLocal->MaybeCurrentTransactionRef(), + mTransaction)); if (mThreadLocal) { // Reset old value. mThreadLocal->SetCurrentTransaction(mPreviousTransaction); } } - - IDBTransaction* Transaction() const { return mTransaction; } }; class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { - IDBRequest* const mRequest; - AutoSetCurrentTransaction mAutoTransaction; + const RefPtr mRequest; + const AutoSetCurrentTransaction mAutoTransaction; + const SafeRefPtr mTransaction; union { IDBDatabase* mDatabase; @@ -224,10 +223,11 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { } mResultType; public: - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, IDBDatabase* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeDatabase) { MOZ_ASSERT(aRequest); MOZ_ASSERT(aResult); @@ -235,30 +235,33 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { mResult.mDatabase = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, IDBCursor* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeCursor) { MOZ_ASSERT(aRequest); mResult.mCursor = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, IDBMutableFile* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeMutableFile) { MOZ_ASSERT(aRequest); mResult.mMutableFile = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, StructuredCloneReadInfoChild* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeStructuredClone) { MOZ_ASSERT(aRequest); MOZ_ASSERT(aResult); @@ -266,10 +269,11 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { mResult.mStructuredClone = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, nsTArray* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeStructuredCloneArray) { MOZ_ASSERT(aRequest); MOZ_ASSERT(aResult); @@ -277,10 +281,11 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { mResult.mStructuredCloneArray = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, const Key* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeKey) { MOZ_ASSERT(aRequest); MOZ_ASSERT(aResult); @@ -288,10 +293,11 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { mResult.mKey = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, const nsTArray* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeKeyArray) { MOZ_ASSERT(aRequest); MOZ_ASSERT(aResult); @@ -299,10 +305,11 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { mResult.mKeyArray = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, const JS::Value* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeJSVal) { MOZ_ASSERT(aRequest); MOZ_ASSERT(!aResult->isGCThing()); @@ -310,20 +317,17 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { mResult.mJSVal = aResult; } - ResultHelper(IDBRequest* aRequest, IDBTransaction* aTransaction, + ResultHelper(IDBRequest* aRequest, SafeRefPtr aTransaction, const JS::Handle* aResult) : mRequest(aRequest), - mAutoTransaction(aTransaction), + mAutoTransaction(aTransaction ? SomeRef(*aTransaction) : Nothing()), + mTransaction(std::move(aTransaction)), mResultType(ResultTypeJSValHandle) { MOZ_ASSERT(aRequest); mResult.mJSValHandle = aResult; } - IDBRequest* Request() const { return mRequest; } - - IDBTransaction* Transaction() const { return mAutoTransaction.Transaction(); } - virtual nsresult GetResult(JSContext* aCx, JS::MutableHandle aResult) override { MOZ_ASSERT(aCx); @@ -367,6 +371,8 @@ class MOZ_STACK_CLASS ResultHelper final : public IDBRequest::ResultCallback { MOZ_CRASH("Should never get here!"); } + void DispatchSuccessEvent(RefPtr aEvent = nullptr); + private: template std::enable_if_t || @@ -650,9 +656,10 @@ StructuredCloneReadInfoChild DeserializeStructuredCloneReadInfo( // TODO: Remove duplication between DispatchErrorEvent and DispatchSucessEvent. -void DispatchErrorEvent(IDBRequest* aRequest, nsresult aErrorCode, - IDBTransaction* aTransaction = nullptr, - Event* aEvent = nullptr) { +void DispatchErrorEvent( + const RefPtr& aRequest, nsresult aErrorCode, + const SafeRefPtr& aTransaction = nullptr, + RefPtr aEvent = nullptr) { MOZ_ASSERT(aRequest); aRequest->AssertIsOnOwningThread(); MOZ_ASSERT(NS_FAILED(aErrorCode)); @@ -661,33 +668,31 @@ void DispatchErrorEvent(IDBRequest* aRequest, nsresult aErrorCode, AUTO_PROFILER_LABEL("IndexedDB:DispatchErrorEvent", DOM); const RefPtr request = aRequest; - const RefPtr transaction = aTransaction; request->SetError(aErrorCode); - RefPtr errorEvent; if (!aEvent) { // Make an error event and fire it at the target. - errorEvent = CreateGenericEvent(request, nsDependentString(kErrorEventType), - eDoesBubble, eCancelable); - MOZ_ASSERT(errorEvent); - - aEvent = errorEvent; + aEvent = CreateGenericEvent(request, nsDependentString(kErrorEventType), + eDoesBubble, eCancelable); } + MOZ_ASSERT(aEvent); + // XXX This is redundant if we are called from + // ResultHelper::DispatchSuccessEvent. Maybe asct; if (aTransaction) { - asct.emplace(aTransaction); + asct.emplace(SomeRef(*aTransaction)); } - if (transaction && transaction->IsInactive()) { - transaction->TransitionToActive(); + if (aTransaction && aTransaction->IsInactive()) { + aTransaction->TransitionToActive(); } - if (transaction) { + if (aTransaction) { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "Firing %s event with error 0x%x", "%s (0x%x)", - transaction->LoggingSerialNumber(), request->LoggingSerialNumber(), + aTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(aEvent, kErrorEventType), aErrorCode); } else { IDB_LOG_MARK_CHILD_REQUEST("Firing %s event with error 0x%x", "%s (0x%x)", @@ -703,11 +708,12 @@ void DispatchErrorEvent(IDBRequest* aRequest, nsresult aErrorCode, return; } - MOZ_ASSERT(!transaction || transaction->IsActive() || - transaction->IsAborted() || transaction->WasExplicitlyCommitted()); + MOZ_ASSERT(!aTransaction || aTransaction->IsActive() || + aTransaction->IsAborted() || + aTransaction->WasExplicitlyCommitted()); - if (transaction && transaction->IsActive()) { - transaction->TransitionToInactive(); + if (aTransaction && aTransaction->IsActive()) { + aTransaction->TransitionToInactive(); // Do not abort the transaction here if this request is failed due to the // abortion of its transaction to ensure that the correct error cause of @@ -718,65 +724,53 @@ void DispatchErrorEvent(IDBRequest* aRequest, nsresult aErrorCode, MOZ_ASSERT(internalEvent); if (internalEvent->mFlags.mExceptionWasRaised) { - transaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR); + aTransaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR); } else if (doDefault) { - transaction->Abort(request); + aTransaction->Abort(request); } } } } -void DispatchSuccessEvent(ResultHelper* aResultHelper, - Event* aEvent = nullptr) { - MOZ_ASSERT(aResultHelper); - +void ResultHelper::DispatchSuccessEvent(RefPtr aEvent) { AUTO_PROFILER_LABEL("IndexedDB:DispatchSuccessEvent", DOM); - const RefPtr request = aResultHelper->Request(); - MOZ_ASSERT(request); - request->AssertIsOnOwningThread(); + MOZ_ASSERT(mRequest); + mRequest->AssertIsOnOwningThread(); - const RefPtr transaction = aResultHelper->Transaction(); - - if (transaction && transaction->IsAborted()) { - DispatchErrorEvent(request, transaction->AbortCode(), transaction); + if (mTransaction && mTransaction->IsAborted()) { + DispatchErrorEvent(mRequest, mTransaction->AbortCode(), mTransaction); return; } - RefPtr successEvent; if (!aEvent) { - successEvent = - CreateGenericEvent(request, nsDependentString(kSuccessEventType), - eDoesNotBubble, eNotCancelable); - MOZ_ASSERT(successEvent); - - aEvent = successEvent; + aEvent = CreateGenericEvent(mRequest, nsDependentString(kSuccessEventType), + eDoesNotBubble, eNotCancelable); } - - request->SetResultCallback(aResultHelper); - MOZ_ASSERT(aEvent); - if (transaction && transaction->IsInactive()) { - transaction->TransitionToActive(); + mRequest->SetResultCallback(this); + + if (mTransaction && mTransaction->IsInactive()) { + mTransaction->TransitionToActive(); } - if (transaction) { + if (mTransaction) { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( - "Firing %s event", "%s", transaction->LoggingSerialNumber(), - request->LoggingSerialNumber(), + "Firing %s event", "%s", mTransaction->LoggingSerialNumber(), + mRequest->LoggingSerialNumber(), IDB_LOG_STRINGIFY(aEvent, kSuccessEventType)); } else { IDB_LOG_MARK_CHILD_REQUEST("Firing %s event", "%s", - request->LoggingSerialNumber(), + mRequest->LoggingSerialNumber(), IDB_LOG_STRINGIFY(aEvent, kSuccessEventType)); } - MOZ_ASSERT_IF(transaction && !transaction->WasExplicitlyCommitted(), - transaction->IsActive() && !transaction->IsAborted()); + MOZ_ASSERT_IF(mTransaction && !mTransaction->WasExplicitlyCommitted(), + mTransaction->IsActive() && !mTransaction->IsAborted()); IgnoredErrorResult rv; - request->DispatchEvent(*aEvent, rv); + mRequest->DispatchEvent(*aEvent, rv); if (NS_WARN_IF(rv.Failed())) { return; } @@ -784,14 +778,14 @@ void DispatchSuccessEvent(ResultHelper* aResultHelper, WidgetEvent* const internalEvent = aEvent->WidgetEventPtr(); MOZ_ASSERT(internalEvent); - if (transaction && transaction->IsActive()) { - transaction->TransitionToInactive(); + if (mTransaction && mTransaction->IsActive()) { + mTransaction->TransitionToInactive(); if (internalEvent->mFlags.mExceptionWasRaised) { - transaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR); + mTransaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR); } else { // To handle upgrade transaction. - transaction->CommitIfNotStarted(); + mTransaction->CommitIfNotStarted(); } } } @@ -1611,7 +1605,7 @@ bool BackgroundFactoryRequestChild::HandleResponse( } else { ResultHelper helper(mRequest, nullptr, database); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); } databaseActor->ReleaseDOMObject(); @@ -1631,7 +1625,7 @@ bool BackgroundFactoryRequestChild::HandleResponse( aResponse.previousVersion()); MOZ_ASSERT(successEvent); - DispatchSuccessEvent(&helper, successEvent); + helper.DispatchSuccessEvent(std::move(successEvent)); MOZ_ASSERT(!mDatabaseActor); @@ -1980,26 +1974,26 @@ BackgroundDatabaseChild::RecvPBackgroundIDBVersionChangeTransactionConstructor( RefPtr request = mOpenRequestActor->GetOpenDBRequest(); MOZ_ASSERT(request); - RefPtr transaction = IDBTransaction::CreateVersionChange( + SafeRefPtr transaction = IDBTransaction::CreateVersionChange( mDatabase, actor, request, aNextObjectStoreId, aNextIndexId); MOZ_ASSERT(transaction); transaction->AssertIsOnOwningThread(); - actor->SetDOMTransaction(transaction); + actor->SetDOMTransaction(transaction.clonePtr()); mDatabase->EnterSetVersionTransaction(aRequestedVersion); - request->SetTransaction(transaction); + request->SetTransaction(transaction.clonePtr()); RefPtr upgradeNeededEvent = IDBVersionChangeEvent::Create( request, nsDependentString(kUpgradeNeededEventType), aCurrentVersion, aRequestedVersion); MOZ_ASSERT(upgradeNeededEvent); - ResultHelper helper(request, transaction, mDatabase); + ResultHelper helper(request, std::move(transaction), mDatabase); - DispatchSuccessEvent(&helper, upgradeNeededEvent); + helper.DispatchSuccessEvent(std::move(upgradeNeededEvent)); return IPC_OK(); } @@ -2169,7 +2163,7 @@ bool BackgroundDatabaseRequestChild::HandleResponse( ResultHelper helper(mRequest, nullptr, mutableFile); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); mutableFileActor->ReleaseDOMObject(); @@ -2205,21 +2199,14 @@ mozilla::ipc::IPCResult BackgroundDatabaseRequestChild::Recv__delete__( * BackgroundTransactionBase ******************************************************************************/ -BackgroundTransactionBase::BackgroundTransactionBase() : mTransaction(nullptr) { - MOZ_COUNT_CTOR(indexedDB::BackgroundTransactionBase); -} - BackgroundTransactionBase::BackgroundTransactionBase( - IDBTransaction* aTransaction) - : mTemporaryStrongTransaction(aTransaction), mTransaction(aTransaction) { - MOZ_ASSERT(aTransaction); - aTransaction->AssertIsOnOwningThread(); + SafeRefPtr aTransaction) + : mTemporaryStrongTransaction(std::move(aTransaction)), + mTransaction(mTemporaryStrongTransaction.unsafeGetRawPtr()) { + MOZ_ASSERT(mTransaction); + mTransaction->AssertIsOnOwningThread(); - MOZ_COUNT_CTOR(indexedDB::BackgroundTransactionBase); -} - -BackgroundTransactionBase::~BackgroundTransactionBase() { - MOZ_COUNT_DTOR(indexedDB::BackgroundTransactionBase); + MOZ_COUNT_CTOR(BackgroundTransactionBase); } #ifdef DEBUG @@ -2247,15 +2234,15 @@ void BackgroundTransactionBase::NoteActorDestroyed() { } void BackgroundTransactionBase::SetDOMTransaction( - IDBTransaction* aTransaction) { + SafeRefPtr aTransaction) { AssertIsOnOwningThread(); MOZ_ASSERT(aTransaction); aTransaction->AssertIsOnOwningThread(); MOZ_ASSERT(!mTemporaryStrongTransaction); MOZ_ASSERT(!mTransaction); - mTemporaryStrongTransaction = aTransaction; - mTransaction = aTransaction; + mTemporaryStrongTransaction = std::move(aTransaction); + mTransaction = mTemporaryStrongTransaction.unsafeGetRawPtr(); } void BackgroundTransactionBase::NoteComplete() { @@ -2270,11 +2257,8 @@ void BackgroundTransactionBase::NoteComplete() { ******************************************************************************/ BackgroundTransactionChild::BackgroundTransactionChild( - IDBTransaction* aTransaction) - : BackgroundTransactionBase(aTransaction) { - MOZ_ASSERT(aTransaction); - aTransaction->AssertIsOnOwningThread(); - + SafeRefPtr aTransaction) + : BackgroundTransactionBase(std::move(aTransaction)) { MOZ_COUNT_CTOR(indexedDB::BackgroundTransactionChild); } @@ -2569,7 +2553,7 @@ bool BackgroundMutableFileChild::DeallocPBackgroundFileHandleChild( BackgroundRequestChild::BackgroundRequestChild(IDBRequest* aRequest) : BackgroundRequestChildBase(aRequest), - mTransaction(aRequest->GetTransaction()), + mTransaction(aRequest->AcquireTransaction()), mRunningPreprocessHelpers(0), mCurrentCloneDataIndex(0), mPreprocessResultCode(NS_OK), @@ -2658,23 +2642,23 @@ void BackgroundRequestChild::HandleResponse(nsresult aResponse) { MOZ_ASSERT(NS_ERROR_GET_MODULE(aResponse) == NS_ERROR_MODULE_DOM_INDEXEDDB); MOZ_ASSERT(mTransaction); - DispatchErrorEvent(mRequest, aResponse, mTransaction); + DispatchErrorEvent(mRequest, aResponse, mTransaction.clonePtr()); } void BackgroundRequestChild::HandleResponse(const Key& aResponse) { AssertIsOnOwningThread(); - ResultHelper helper(mRequest, mTransaction, &aResponse); + ResultHelper helper(mRequest, AcquireTransaction(), &aResponse); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); } void BackgroundRequestChild::HandleResponse(const nsTArray& aResponse) { AssertIsOnOwningThread(); - ResultHelper helper(mRequest, mTransaction, &aResponse); + ResultHelper helper(mRequest, AcquireTransaction(), &aResponse); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); } void BackgroundRequestChild::HandleResponse( @@ -2685,9 +2669,9 @@ void BackgroundRequestChild::HandleResponse( std::move(aResponse), mTransaction->Database(), [this] { return std::move(*GetNextCloneData()); }); - ResultHelper helper(mRequest, mTransaction, &cloneReadInfo); + ResultHelper helper(mRequest, AcquireTransaction(), &cloneReadInfo); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); } void BackgroundRequestChild::HandleResponse( @@ -2713,17 +2697,17 @@ void BackgroundRequestChild::HandleResponse( }); } - ResultHelper helper(mRequest, mTransaction, &cloneReadInfos); + ResultHelper helper(mRequest, AcquireTransaction(), &cloneReadInfos); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); } void BackgroundRequestChild::HandleResponse(JS::Handle aResponse) { AssertIsOnOwningThread(); - ResultHelper helper(mRequest, mTransaction, &aResponse); + ResultHelper helper(mRequest, AcquireTransaction(), &aResponse); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); } void BackgroundRequestChild::HandleResponse(uint64_t aResponse) { @@ -2731,9 +2715,9 @@ void BackgroundRequestChild::HandleResponse(uint64_t aResponse) { JS::Value response(JS::NumberValue(aResponse)); - ResultHelper helper(mRequest, mTransaction, &response); + ResultHelper helper(mRequest, AcquireTransaction(), &response); - DispatchSuccessEvent(&helper); + helper.DispatchSuccessEvent(); } nsresult BackgroundRequestChild::HandlePreprocess( @@ -3180,7 +3164,7 @@ BackgroundRequestChild::PreprocessHelper::OnFileMetadataReady( BackgroundCursorChildBase::BackgroundCursorChildBase(IDBRequest* const aRequest, const Direction aDirection) : mRequest(aRequest), - mTransaction(aRequest->GetTransaction()), + mTransaction(aRequest->MaybeTransactionRef()), mStrongRequest(aRequest), mDirection(aDirection) { MOZ_ASSERT(mTransaction); @@ -3412,8 +3396,12 @@ void BackgroundCursorChild::CompleteContinueRequestFromCache() { GetRequest()->LoggingSerialNumber(), mDelayedResponses.size() + mCachedResponses.size()); - ResultHelper helper(GetRequest(), mTransaction, cursor); - DispatchSuccessEvent(&helper); + ResultHelper helper(GetRequest(), + mTransaction ? SafeRefPtr{&mTransaction.ref(), + AcquireStrongRefFromRawPtr{}} + : nullptr, + cursor); + helper.DispatchSuccessEvent(); mTransaction->OnRequestFinished(/* aRequestCompletedSuccessfully */ true); } @@ -3425,7 +3413,7 @@ void BackgroundCursorChild::SendDeleteMeInternal() { MOZ_ASSERT(!mStrongCursor); mRequest.destroy(); - mTransaction = nullptr; + mTransaction = Nothing(); // TODO: The things until here could be pulled up to // BackgroundCursorChildBase. @@ -3498,7 +3486,9 @@ void BackgroundCursorChildBase::HandleResponse(nsresult aResponse) { MOZ_ASSERT(!mStrongRequest); MOZ_ASSERT(!mStrongCursor); - DispatchErrorEvent(GetRequest(), aResponse, mTransaction); + DispatchErrorEvent( + GetRequest(), aResponse, + SafeRefPtr{&mTransaction.ref(), AcquireStrongRefFromRawPtr{}}); } template @@ -3514,8 +3504,12 @@ void BackgroundCursorChild::HandleResponse( mCursor->Reset(); } - ResultHelper helper(GetRequest(), mTransaction, &JS::NullHandleValue); - DispatchSuccessEvent(&helper); + ResultHelper helper(GetRequest(), + mTransaction ? SafeRefPtr{&mTransaction.ref(), + AcquireStrongRefFromRawPtr{}} + : nullptr, + &JS::NullHandleValue); + helper.DispatchSuccessEvent(); if (!mCursor) { MOZ_ALWAYS_SUCCEEDS(this->GetActorEventTarget()->Dispatch( @@ -3601,8 +3595,12 @@ void BackgroundCursorChild::HandleMultipleCursorResponses( } } - ResultHelper helper(GetRequest(), mTransaction, mCursor); - DispatchSuccessEvent(&helper); + ResultHelper helper(GetRequest(), + mTransaction ? SafeRefPtr{&mTransaction.ref(), + AcquireStrongRefFromRawPtr{}} + : nullptr, + mCursor); + helper.DispatchSuccessEvent(); } template @@ -3681,7 +3679,7 @@ void BackgroundCursorChild::ActorDestroy(ActorDestroyReason aWhy) { #ifdef DEBUG mRequest.maybeDestroy(); - mTransaction = nullptr; + mTransaction = Nothing(); mSource.maybeDestroy(); #endif } @@ -3703,7 +3701,8 @@ mozilla::ipc::IPCResult BackgroundCursorChild::RecvResponse( const RefPtr cursor = std::move(mStrongCursor); Unused << cursor; // XXX see Bug 1605075 - const RefPtr transaction = mTransaction; + const auto transaction = + SafeRefPtr{&mTransaction.ref(), AcquireStrongRefFromRawPtr{}}; switch (aResponse.type()) { case CursorResponse::Tnsresult: diff --git a/dom/indexedDB/ActorsChild.h b/dom/indexedDB/ActorsChild.h index 334266181f2c..be25eff58010 100644 --- a/dom/indexedDB/ActorsChild.h +++ b/dom/indexedDB/ActorsChild.h @@ -75,7 +75,7 @@ class ThreadLocal { friend IDBFactory; LoggingInfo mLoggingInfo; - IDBTransaction* mCurrentTransaction; + Maybe mCurrentTransaction; nsCString mLoggingIdString; NS_DECL_OWNINGTHREAD @@ -124,13 +124,13 @@ class ThreadLocal { return mLoggingInfo.nextRequestSerialNumber()++; } - void SetCurrentTransaction(IDBTransaction* aCurrentTransaction) { + void SetCurrentTransaction(Maybe aCurrentTransaction) { AssertIsOnOwningThread(); mCurrentTransaction = aCurrentTransaction; } - IDBTransaction* GetCurrentTransaction() const { + Maybe MaybeCurrentTransactionRef() const { AssertIsOnOwningThread(); return mCurrentTransaction; @@ -405,12 +405,12 @@ class BackgroundTransactionBase { // mTemporaryStrongTransaction is strong and is only valid until the end of // NoteComplete() member function or until the NoteActorDestroyed() member // function is called. - RefPtr mTemporaryStrongTransaction; + SafeRefPtr mTemporaryStrongTransaction; protected: // mTransaction is weak and is valid until the NoteActorDestroyed() member // function is called. - IDBTransaction* mTransaction; + IDBTransaction* mTransaction = nullptr; public: #ifdef DEBUG @@ -425,10 +425,11 @@ class BackgroundTransactionBase { } protected: - BackgroundTransactionBase(); - explicit BackgroundTransactionBase(IDBTransaction* aTransaction); + MOZ_COUNTED_DEFAULT_CTOR(BackgroundTransactionBase); - virtual ~BackgroundTransactionBase(); + explicit BackgroundTransactionBase(SafeRefPtr aTransaction); + + MOZ_COUNTED_DTOR_VIRTUAL(BackgroundTransactionBase); void NoteActorDestroyed(); @@ -436,7 +437,7 @@ class BackgroundTransactionBase { private: // Only called by BackgroundVersionChangeTransactionChild. - void SetDOMTransaction(IDBTransaction* aTransaction); + void SetDOMTransaction(SafeRefPtr aTransaction); }; class BackgroundTransactionChild final : public BackgroundTransactionBase, @@ -455,7 +456,7 @@ class BackgroundTransactionChild final : public BackgroundTransactionBase, private: // Only created by IDBDatabase. - explicit BackgroundTransactionChild(IDBTransaction* aTransaction); + explicit BackgroundTransactionChild(SafeRefPtr aTransaction); // Only destroyed by BackgroundDatabaseChild. ~BackgroundTransactionChild(); @@ -502,9 +503,7 @@ class BackgroundVersionChangeTransactionChild final ~BackgroundVersionChangeTransactionChild(); // Only called by BackgroundDatabaseChild. - void SetDOMTransaction(IDBTransaction* aDOMObject) { - BackgroundTransactionBase::SetDOMTransaction(aDOMObject); - } + using BackgroundTransactionBase::SetDOMTransaction; public: // IPDL methods are only called by IPDL. @@ -580,7 +579,7 @@ class BackgroundRequestChild final : public BackgroundRequestChildBase, class PreprocessHelper; - RefPtr mTransaction; + SafeRefPtr mTransaction; nsTArray mCloneInfos; uint32_t mRunningPreprocessHelpers; uint32_t mCurrentCloneDataIndex; @@ -625,6 +624,10 @@ class BackgroundRequestChild final : public BackgroundRequestChildBase, nsresult HandlePreprocessInternal( const nsTArray& aPreprocessInfos); + SafeRefPtr AcquireTransaction() const { + return mTransaction.clonePtr(); + } + public: // IPDL methods are only called by IPDL. void ActorDestroy(ActorDestroyReason aWhy) override; @@ -644,7 +647,7 @@ class BackgroundCursorChildBase : public PBackgroundIDBCursorChild { NS_DECL_OWNINGTHREAD protected: InitializedOnceNotNull mRequest; - IDBTransaction* mTransaction; + Maybe mTransaction; // These are only set while a request is in progress. RefPtr mStrongRequest; diff --git a/dom/indexedDB/IDBCursor.cpp b/dom/indexedDB/IDBCursor.cpp index 05591540def5..e56b9a58e520 100644 --- a/dom/indexedDB/IDBCursor.cpp +++ b/dom/indexedDB/IDBCursor.cpp @@ -30,7 +30,7 @@ using namespace indexedDB; IDBCursor::IDBCursor(BackgroundCursorChildBase* const aBackgroundActor) : mBackgroundActor(aBackgroundActor), mRequest(aBackgroundActor->GetRequest()), - mTransaction(mRequest->GetTransaction()), + mTransaction(&mRequest->MutableTransactionRef()), mCachedKey(JS::UndefinedValue()), mCachedPrimaryKey(JS::UndefinedValue()), mCachedValue(JS::UndefinedValue()), @@ -44,7 +44,6 @@ IDBCursor::IDBCursor(BackgroundCursorChildBase* const aBackgroundActor) MOZ_ASSERT(aBackgroundActor); aBackgroundActor->AssertIsOnOwningThread(); MOZ_ASSERT(mRequest); - MOZ_ASSERT(mTransaction); mTransaction->RegisterCursor(this); } @@ -399,7 +398,7 @@ void IDBTypedCursor::Continue(JSContext* const aCx, "cursor(%s).continue(%s)", "IDBCursor.continue()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(mSource), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(mSource), IDB_LOG_STRINGIFY(mDirection), IDB_LOG_STRINGIFY(key)); } else { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( @@ -407,7 +406,7 @@ void IDBTypedCursor::Continue(JSContext* const aCx, "index(%s).cursor(%s).continue(%s)", "IDBCursor.continue()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(GetSourceRef().ObjectStore()), IDB_LOG_STRINGIFY(mSource), IDB_LOG_STRINGIFY(mDirection), IDB_LOG_STRINGIFY(key)); @@ -517,7 +516,7 @@ void IDBTypedCursor::ContinuePrimaryKey( "index(%s).cursor(%s).continuePrimaryKey(%s, %s)", "IDBCursor.continuePrimaryKey()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(&GetSourceObjectStoreRef()), IDB_LOG_STRINGIFY(mSource), IDB_LOG_STRINGIFY(mDirection), IDB_LOG_STRINGIFY(key), IDB_LOG_STRINGIFY(primaryKey)); @@ -558,7 +557,7 @@ void IDBTypedCursor::Advance(const uint32_t aCount, "cursor(%s).advance(%ld)", "IDBCursor.advance()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(mSource), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(mSource), IDB_LOG_STRINGIFY(mDirection), aCount); } else { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( @@ -566,7 +565,7 @@ void IDBTypedCursor::Advance(const uint32_t aCount, "index(%s).cursor(%s).advance(%ld)", "IDBCursor.advance()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(GetSourceRef().ObjectStore()), IDB_LOG_STRINGIFY(mSource), IDB_LOG_STRINGIFY(mDirection), aCount); } @@ -664,7 +663,7 @@ RefPtr IDBTypedCursor::Update( "IDBCursor.update()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(&objectStore), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(&objectStore), IDB_LOG_STRINGIFY(mDirection), IDB_LOG_STRINGIFY(&objectStore, primaryKey)); } else { @@ -674,7 +673,7 @@ RefPtr IDBTypedCursor::Update( "IDBCursor.update()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(&objectStore), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(&objectStore), IDB_LOG_STRINGIFY(mSource), IDB_LOG_STRINGIFY(mDirection), IDB_LOG_STRINGIFY(&objectStore, primaryKey)); } @@ -736,7 +735,7 @@ RefPtr IDBTypedCursor::Delete(JSContext* const aCx, "IDBCursor.delete()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(&objectStore), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(&objectStore), IDB_LOG_STRINGIFY(mDirection), IDB_LOG_STRINGIFY(&objectStore, primaryKey)); } else { @@ -746,7 +745,7 @@ RefPtr IDBTypedCursor::Delete(JSContext* const aCx, "IDBCursor.delete()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(&objectStore), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(&objectStore), IDB_LOG_STRINGIFY(mSource), IDB_LOG_STRINGIFY(mDirection), IDB_LOG_STRINGIFY(&objectStore, primaryKey)); } diff --git a/dom/indexedDB/IDBDatabase.cpp b/dom/indexedDB/IDBDatabase.cpp index b1168bea97ce..3208ffe3ff74 100644 --- a/dom/indexedDB/IDBDatabase.cpp +++ b/dom/indexedDB/IDBDatabase.cpp @@ -313,7 +313,8 @@ void IDBDatabase::RefreshSpec(bool aMayDelete) { AssertIsOnOwningThread(); for (auto iter = mTransactions.Iter(); !iter.Done(); iter.Next()) { - RefPtr transaction = iter.Get()->GetKey(); + const auto transaction = + SafeRefPtr{iter.Get()->GetKey(), AcquireStrongRefFromRawPtr{}}; MOZ_ASSERT(transaction); transaction->AssertIsOnOwningThread(); transaction->RefreshSpec(aMayDelete); @@ -355,7 +356,7 @@ RefPtr IDBDatabase::CreateObjectStore( ErrorResult& aRv) { AssertIsOnOwningThread(); - IDBTransaction* transaction = IDBTransaction::GetCurrent(); + const auto transaction = IDBTransaction::MaybeCurrent(); if (!transaction || transaction->Database() != this || transaction->GetMode() != IDBTransaction::Mode::VersionChange) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_NOT_ALLOWED_ERR); @@ -418,7 +419,7 @@ RefPtr IDBDatabase::CreateObjectStore( "database(%s).transaction(%s).createObjectStore(%s)", "IDBDatabase.createObjectStore()", transaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(this), - IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(objectStore)); + IDB_LOG_STRINGIFY(*transaction), IDB_LOG_STRINGIFY(objectStore)); return objectStore; } @@ -426,7 +427,7 @@ RefPtr IDBDatabase::CreateObjectStore( void IDBDatabase::DeleteObjectStore(const nsAString& aName, ErrorResult& aRv) { AssertIsOnOwningThread(); - IDBTransaction* transaction = IDBTransaction::GetCurrent(); + const auto transaction = IDBTransaction::MaybeCurrent(); if (!transaction || transaction->Database() != this || transaction->GetMode() != IDBTransaction::Mode::VersionChange) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_NOT_ALLOWED_ERR); @@ -467,7 +468,7 @@ void IDBDatabase::DeleteObjectStore(const nsAString& aName, ErrorResult& aRv) { "database(%s).transaction(%s).deleteObjectStore(\"%s\")", "IDBDatabase.deleteObjectStore()", transaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(this), - IDB_LOG_STRINGIFY(transaction), NS_ConvertUTF16toUTF8(aName).get()); + IDB_LOG_STRINGIFY(*transaction), NS_ConvertUTF16toUTF8(aName).get()); } RefPtr IDBDatabase::Transaction( @@ -589,7 +590,7 @@ RefPtr IDBDatabase::Transaction( MOZ_CRASH("Unknown mode!"); } - RefPtr transaction = + SafeRefPtr transaction = IDBTransaction::Create(aCx, this, sortedStoreNames, mode); if (NS_WARN_IF(!transaction)) { IDB_REPORT_INTERNAL_ERR(); @@ -600,12 +601,12 @@ RefPtr IDBDatabase::Transaction( } BackgroundTransactionChild* actor = - new BackgroundTransactionChild(transaction); + new BackgroundTransactionChild(transaction.clonePtr()); IDB_LOG_MARK_CHILD_TRANSACTION( "database(%s).transaction(%s)", "IDBDatabase.transaction()", transaction->LoggingSerialNumber(), IDB_LOG_STRINGIFY(this), - IDB_LOG_STRINGIFY(transaction)); + IDB_LOG_STRINGIFY(*transaction)); MOZ_ALWAYS_TRUE(mBackgroundActor->SendPBackgroundIDBTransactionConstructor( actor, sortedStoreNames, mode)); @@ -618,7 +619,7 @@ RefPtr IDBDatabase::Transaction( ExpireFileActors(/* aExpireAll */ true); } - return transaction; + return AsRefPtr(std::move(transaction)); } StorageType IDBDatabase::Storage() const { @@ -675,22 +676,20 @@ RefPtr IDBDatabase::CreateMutableFile( return request; } -void IDBDatabase::RegisterTransaction(IDBTransaction* aTransaction) { +void IDBDatabase::RegisterTransaction(IDBTransaction& aTransaction) { AssertIsOnOwningThread(); - MOZ_ASSERT(aTransaction); - aTransaction->AssertIsOnOwningThread(); - MOZ_ASSERT(!mTransactions.Contains(aTransaction)); + aTransaction.AssertIsOnOwningThread(); + MOZ_ASSERT(!mTransactions.Contains(&aTransaction)); - mTransactions.PutEntry(aTransaction); + mTransactions.PutEntry(&aTransaction); } -void IDBDatabase::UnregisterTransaction(IDBTransaction* aTransaction) { +void IDBDatabase::UnregisterTransaction(IDBTransaction& aTransaction) { AssertIsOnOwningThread(); - MOZ_ASSERT(aTransaction); - aTransaction->AssertIsOnOwningThread(); - MOZ_ASSERT(mTransactions.Contains(aTransaction)); + aTransaction.AssertIsOnOwningThread(); + MOZ_ASSERT(mTransactions.Contains(&aTransaction)); - mTransactions.RemoveEntry(aTransaction); + mTransactions.RemoveEntry(&aTransaction); } void IDBDatabase::AbortTransactions(bool aShouldWarn) { @@ -698,7 +697,7 @@ void IDBDatabase::AbortTransactions(bool aShouldWarn) { constexpr size_t StackExceptionLimit = 20; using StrongTransactionArray = - AutoTArray, StackExceptionLimit>; + AutoTArray, StackExceptionLimit>; using WeakTransactionArray = AutoTArray; if (!mTransactions.Count()) { @@ -720,7 +719,8 @@ void IDBDatabase::AbortTransactions(bool aShouldWarn) { // there is a race here and it's possible that the transaction has not // been successfully committed yet so we will warn the user. if (!transaction->IsFinished()) { - transactionsToAbort.AppendElement(transaction); + transactionsToAbort.EmplaceBack(transaction, + AcquireStrongRefFromRawPtr{}); } } MOZ_ASSERT(transactionsToAbort.Length() <= mTransactions.Count()); @@ -745,7 +745,7 @@ void IDBDatabase::AbortTransactions(bool aShouldWarn) { // We warn for any transactions that could have written data, but // ignore read-only transactions. if (aShouldWarn && transaction->IsWriteAllowed()) { - transactionsThatNeedWarning.AppendElement(transaction); + transactionsThatNeedWarning.AppendElement(transaction.unsafeGetRawPtr()); } transaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR); diff --git a/dom/indexedDB/IDBDatabase.h b/dom/indexedDB/IDBDatabase.h index eb7d37f7a85b..88cead435a00 100644 --- a/dom/indexedDB/IDBDatabase.h +++ b/dom/indexedDB/IDBDatabase.h @@ -147,9 +147,9 @@ class IDBDatabase final : public DOMEventTargetHelper { // DatabaseInfo. void RevertToPreviousState(); - void RegisterTransaction(IDBTransaction* aTransaction); + void RegisterTransaction(IDBTransaction& aTransaction); - void UnregisterTransaction(IDBTransaction* aTransaction); + void UnregisterTransaction(IDBTransaction& aTransaction); void AbortTransactions(bool aShouldWarn); diff --git a/dom/indexedDB/IDBIndex.cpp b/dom/indexedDB/IDBIndex.cpp index 3e8faa04a0ce..39eb2e4286f2 100644 --- a/dom/indexedDB/IDBIndex.cpp +++ b/dom/indexedDB/IDBIndex.cpp @@ -33,9 +33,10 @@ RefPtr GenerateRequest(JSContext* aCx, IDBIndex* aIndex) { MOZ_ASSERT(aIndex); aIndex->AssertIsOnOwningThread(); - auto* const transaction = aIndex->ObjectStore()->Transaction(); + auto transaction = aIndex->ObjectStore()->AcquireTransaction(); + auto* const database = transaction->Database(); - return IDBRequest::Create(aCx, aIndex, transaction->Database(), transaction); + return IDBRequest::Create(aCx, aIndex, database, std::move(transaction)); } } // namespace @@ -173,15 +174,15 @@ const nsString& IDBIndex::Name() const { void IDBIndex::SetName(const nsAString& aName, ErrorResult& aRv) { AssertIsOnOwningThread(); - IDBTransaction* const transaction = mObjectStore->Transaction(); + const auto& transaction = mObjectStore->TransactionRef(); - if (transaction->GetMode() != IDBTransaction::Mode::VersionChange || + if (transaction.GetMode() != IDBTransaction::Mode::VersionChange || mDeletedMetadata) { aRv.Throw(NS_ERROR_DOM_INVALID_STATE_ERR); return; } - if (!transaction->IsActive()) { + if (!transaction.IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return; } @@ -196,7 +197,7 @@ void IDBIndex::SetName(const nsAString& aName, ErrorResult& aRv) { const int64_t indexId = Id(); nsresult rv = - transaction->Database()->RenameIndex(mObjectStore->Id(), indexId, aName); + transaction.Database()->RenameIndex(mObjectStore->Id(), indexId, aName); if (NS_FAILED(rv)) { aRv.Throw(rv); @@ -210,12 +211,13 @@ void IDBIndex::SetName(const nsAString& aName, ErrorResult& aRv) { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "rename(%s)", - "IDBIndex.rename()", transaction->LoggingSerialNumber(), - requestSerialNumber, IDB_LOG_STRINGIFY(transaction->Database()), + "IDBIndex.rename()", transaction.LoggingSerialNumber(), + requestSerialNumber, IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), loggingOldIndex.get(), IDB_LOG_STRINGIFY(this)); - transaction->RenameIndex(mObjectStore, indexId, aName); + mObjectStore->MutableTransactionRef().RenameIndex(mObjectStore, indexId, + aName); } bool IDBIndex::Unique() const { @@ -312,8 +314,8 @@ RefPtr IDBIndex::GetInternal(bool aKeyOnly, JSContext* aCx, return nullptr; } - IDBTransaction* transaction = mObjectStore->Transaction(); - if (!transaction->IsActive()) { + const auto& transaction = mObjectStore->TransactionRef(); + if (!transaction.IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return nullptr; } @@ -351,28 +353,30 @@ RefPtr IDBIndex::GetInternal(bool aKeyOnly, JSContext* aCx, IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "getKey(%s)", - "IDBIndex.getKey()", transaction->LoggingSerialNumber(), + "IDBIndex.getKey()", transaction.LoggingSerialNumber(), request->LoggingSerialNumber(), - IDB_LOG_STRINGIFY(transaction->Database()), + IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange)); } else { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "get(%s)", - "IDBIndex.get()", transaction->LoggingSerialNumber(), + "IDBIndex.get()", transaction.LoggingSerialNumber(), request->LoggingSerialNumber(), - IDB_LOG_STRINGIFY(transaction->Database()), + IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange)); } + auto& mutableTransaction = mObjectStore->MutableTransactionRef(); + // TODO: This is necessary to preserve request ordering only. Proper // sequencing of requests should be done in a more sophisticated manner that // doesn't require invalidating cursor caches (Bug 1580499). - transaction->InvalidateCursorCaches(); + mutableTransaction.InvalidateCursorCaches(); - transaction->StartRequest(request, params); + mutableTransaction.StartRequest(request, params); return request; } @@ -388,8 +392,8 @@ RefPtr IDBIndex::GetAllInternal(bool aKeysOnly, JSContext* aCx, return nullptr; } - IDBTransaction* transaction = mObjectStore->Transaction(); - if (!transaction->IsActive()) { + const auto& transaction = mObjectStore->TransactionRef(); + if (!transaction.IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return nullptr; } @@ -425,9 +429,9 @@ RefPtr IDBIndex::GetAllInternal(bool aKeysOnly, JSContext* aCx, IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "getAllKeys(%s, %s)", - "IDBIndex.getAllKeys()", transaction->LoggingSerialNumber(), + "IDBIndex.getAllKeys()", transaction.LoggingSerialNumber(), request->LoggingSerialNumber(), - IDB_LOG_STRINGIFY(transaction->Database()), + IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aLimit)); @@ -435,20 +439,22 @@ RefPtr IDBIndex::GetAllInternal(bool aKeysOnly, JSContext* aCx, IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "getAll(%s, %s)", - "IDBIndex.getAll()", transaction->LoggingSerialNumber(), + "IDBIndex.getAll()", transaction.LoggingSerialNumber(), request->LoggingSerialNumber(), - IDB_LOG_STRINGIFY(transaction->Database()), + IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aLimit)); } + auto& mutableTransaction = mObjectStore->MutableTransactionRef(); + // TODO: This is necessary to preserve request ordering only. Proper // sequencing of requests should be done in a more sophisticated manner that // doesn't require invalidating cursor caches (Bug 1580499). - transaction->InvalidateCursorCaches(); + mutableTransaction.InvalidateCursorCaches(); - transaction->StartRequest(request, params); + mutableTransaction.StartRequest(request, params); return request; } @@ -464,8 +470,8 @@ RefPtr IDBIndex::OpenCursorInternal(bool aKeysOnly, JSContext* aCx, return nullptr; } - IDBTransaction* transaction = mObjectStore->Transaction(); - if (!transaction->IsActive()) { + const auto& transaction = mObjectStore->TransactionRef(); + if (!transaction.IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return nullptr; } @@ -502,9 +508,9 @@ RefPtr IDBIndex::OpenCursorInternal(bool aKeysOnly, JSContext* aCx, IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "openKeyCursor(%s, %s)", - "IDBIndex.openKeyCursor()", transaction->LoggingSerialNumber(), + "IDBIndex.openKeyCursor()", transaction.LoggingSerialNumber(), request->LoggingSerialNumber(), - IDB_LOG_STRINGIFY(transaction->Database()), + IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aDirection)); @@ -512,9 +518,9 @@ RefPtr IDBIndex::OpenCursorInternal(bool aKeysOnly, JSContext* aCx, IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "openCursor(%s, %s)", - "IDBIndex.openCursor()", transaction->LoggingSerialNumber(), + "IDBIndex.openCursor()", transaction.LoggingSerialNumber(), request->LoggingSerialNumber(), - IDB_LOG_STRINGIFY(transaction->Database()), + IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aDirection)); @@ -527,12 +533,14 @@ RefPtr IDBIndex::OpenCursorInternal(bool aKeysOnly, JSContext* aCx, : new BackgroundCursorChild(request, this, aDirection); + auto& mutableTransaction = mObjectStore->MutableTransactionRef(); + // TODO: This is necessary to preserve request ordering only. Proper // sequencing of requests should be done in a more sophisticated manner that // doesn't require invalidating cursor caches (Bug 1580499). - transaction->InvalidateCursorCaches(); + mutableTransaction.InvalidateCursorCaches(); - mObjectStore->Transaction()->OpenCursor(actor, params); + mutableTransaction.OpenCursor(actor, params); return request; } @@ -546,8 +554,8 @@ RefPtr IDBIndex::Count(JSContext* aCx, JS::Handle aKey, return nullptr; } - IDBTransaction* const transaction = mObjectStore->Transaction(); - if (!transaction->IsActive()) { + const auto& transaction = mObjectStore->TransactionRef(); + if (!transaction.IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return nullptr; } @@ -574,18 +582,19 @@ RefPtr IDBIndex::Count(JSContext* aCx, JS::Handle aKey, IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( "database(%s).transaction(%s).objectStore(%s).index(%s)." "count(%s)", - "IDBIndex.count()", transaction->LoggingSerialNumber(), - request->LoggingSerialNumber(), - IDB_LOG_STRINGIFY(transaction->Database()), + "IDBIndex.count()", transaction.LoggingSerialNumber(), + request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(transaction.Database()), IDB_LOG_STRINGIFY(transaction), IDB_LOG_STRINGIFY(mObjectStore), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange)); + auto& mutableTransaction = mObjectStore->MutableTransactionRef(); + // TODO: This is necessary to preserve request ordering only. Proper // sequencing of requests should be done in a more sophisticated manner that // doesn't require invalidating cursor caches (Bug 1580499). - transaction->InvalidateCursorCaches(); + mutableTransaction.InvalidateCursorCaches(); - transaction->StartRequest(request, params); + mutableTransaction.StartRequest(request, params); return request; } diff --git a/dom/indexedDB/IDBObjectStore.cpp b/dom/indexedDB/IDBObjectStore.cpp index e0354733d6e4..fda5bc71d6e6 100644 --- a/dom/indexedDB/IDBObjectStore.cpp +++ b/dom/indexedDB/IDBObjectStore.cpp @@ -139,10 +139,11 @@ RefPtr GenerateRequest(JSContext* aCx, MOZ_ASSERT(aObjectStore); aObjectStore->AssertIsOnOwningThread(); - IDBTransaction* const transaction = aObjectStore->Transaction(); + auto transaction = aObjectStore->AcquireTransaction(); + auto* const database = transaction->Database(); - return IDBRequest::Create(aCx, aObjectStore, transaction->Database(), - transaction); + return IDBRequest::Create(aCx, aObjectStore, database, + std::move(transaction)); } bool StructuredCloneWriteCallback(JSContext* aCx, @@ -473,15 +474,15 @@ const JSClass IDBObjectStore::sDummyPropJSClass = { "IDBObjectStore Dummy", 0 /* flags */ }; -IDBObjectStore::IDBObjectStore(IDBTransaction* aTransaction, +IDBObjectStore::IDBObjectStore(SafeRefPtr aTransaction, ObjectStoreSpec* aSpec) - : mTransaction(aTransaction), + : mTransaction(std::move(aTransaction)), mCachedKeyPath(JS::UndefinedValue()), mSpec(aSpec), mId(aSpec->metadata().id()), mRooted(false) { - MOZ_ASSERT(aTransaction); - aTransaction->AssertIsOnOwningThread(); + MOZ_ASSERT(mTransaction); + mTransaction->AssertIsOnOwningThread(); MOZ_ASSERT(aSpec); } @@ -495,12 +496,12 @@ IDBObjectStore::~IDBObjectStore() { } // static -RefPtr IDBObjectStore::Create(IDBTransaction* aTransaction, - ObjectStoreSpec& aSpec) { +RefPtr IDBObjectStore::Create( + SafeRefPtr aTransaction, ObjectStoreSpec& aSpec) { MOZ_ASSERT(aTransaction); aTransaction->AssertIsOnOwningThread(); - return new IDBObjectStore(aTransaction, &aSpec); + return new IDBObjectStore(std::move(aTransaction), &aSpec); } // static @@ -924,7 +925,7 @@ RefPtr IDBObjectStore::AddOrPut(JSContext* aCx, "IDBObjectStore.put()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(key)); } else { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( @@ -932,7 +933,7 @@ RefPtr IDBObjectStore::AddOrPut(JSContext* aCx, "IDBObjectStore.add()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(key)); } } @@ -993,7 +994,7 @@ RefPtr IDBObjectStore::GetAllInternal( "IDBObjectStore.getAllKeys()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aLimit)); } else { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( @@ -1002,7 +1003,7 @@ RefPtr IDBObjectStore::GetAllInternal( "IDBObjectStore.getAll()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aLimit)); } @@ -1090,7 +1091,7 @@ RefPtr IDBObjectStore::Clear(JSContext* aCx, ErrorResult& aRv) { "IDBObjectStore.clear()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this)); + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this)); mTransaction->InvalidateCursorCaches(); @@ -1309,7 +1310,7 @@ RefPtr IDBObjectStore::GetInternal(bool aKeyOnly, JSContext* aCx, "IDBObjectStore.get()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange)); // TODO: This is necessary to preserve request ordering only. Proper @@ -1368,7 +1369,7 @@ RefPtr IDBObjectStore::DeleteInternal(JSContext* aCx, "IDBObjectStore.delete()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange)); } @@ -1390,7 +1391,7 @@ RefPtr IDBObjectStore::CreateIndex( return nullptr; } - IDBTransaction* const transaction = IDBTransaction::GetCurrent(); + const auto transaction = IDBTransaction::MaybeCurrent(); if (!transaction || transaction != mTransaction || !transaction->IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return nullptr; @@ -1477,7 +1478,7 @@ RefPtr IDBObjectStore::CreateIndex( "database(%s).transaction(%s).objectStore(%s).createIndex(%s)", "IDBObjectStore.createIndex()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(index)); return index; @@ -1492,7 +1493,7 @@ void IDBObjectStore::DeleteIndex(const nsAString& aName, ErrorResult& aRv) { return; } - IDBTransaction* transaction = IDBTransaction::GetCurrent(); + const auto transaction = IDBTransaction::MaybeCurrent(); if (!transaction || transaction != mTransaction || !transaction->IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return; @@ -1543,7 +1544,7 @@ void IDBObjectStore::DeleteIndex(const nsAString& aName, ErrorResult& aRv) { "deleteIndex(\"%s\")", "IDBObjectStore.deleteIndex()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), NS_ConvertUTF16toUTF8(aName).get()); transaction->DeleteIndex(this, foundId); @@ -1587,7 +1588,7 @@ RefPtr IDBObjectStore::Count(JSContext* aCx, "IDBObjectStore.count()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange)); // TODO: This is necessary to preserve request ordering only. Proper @@ -1652,7 +1653,7 @@ RefPtr IDBObjectStore::OpenCursorInternal( "IDBObjectStore.openKeyCursor()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aDirection)); } else { IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( @@ -1661,7 +1662,7 @@ RefPtr IDBObjectStore::OpenCursorInternal( "IDBObjectStore.openCursor()", mTransaction->LoggingSerialNumber(), request->LoggingSerialNumber(), IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), IDB_LOG_STRINGIFY(this), + IDB_LOG_STRINGIFY(*mTransaction), IDB_LOG_STRINGIFY(this), IDB_LOG_STRINGIFY(keyRange), IDB_LOG_STRINGIFY(aDirection)); } @@ -1757,7 +1758,7 @@ void IDBObjectStore::SetName(const nsAString& aName, ErrorResult& aRv) { return; } - IDBTransaction* transaction = IDBTransaction::GetCurrent(); + const auto transaction = IDBTransaction::MaybeCurrent(); if (!transaction || transaction != mTransaction || !transaction->IsActive()) { aRv.Throw(NS_ERROR_DOM_INDEXEDDB_TRANSACTION_INACTIVE_ERR); return; @@ -1786,7 +1787,7 @@ void IDBObjectStore::SetName(const nsAString& aName, ErrorResult& aRv) { "database(%s).transaction(%s).objectStore(%s).rename(%s)", "IDBObjectStore.rename()", mTransaction->LoggingSerialNumber(), requestSerialNumber, IDB_LOG_STRINGIFY(mTransaction->Database()), - IDB_LOG_STRINGIFY(mTransaction), loggingOldObjectStore.get(), + IDB_LOG_STRINGIFY(*mTransaction), loggingOldObjectStore.get(), IDB_LOG_STRINGIFY(this)); transaction->RenameObjectStore(mSpec->metadata().id(), aName); diff --git a/dom/indexedDB/IDBObjectStore.h b/dom/indexedDB/IDBObjectStore.h index 90ba2990b253..b75d57927237 100644 --- a/dom/indexedDB/IDBObjectStore.h +++ b/dom/indexedDB/IDBObjectStore.h @@ -58,7 +58,7 @@ class IDBObjectStore final : public nsISupports, public nsWrapperCache { // TODO: This could be made const if Bug 1575173 is resolved. It is // initialized in the constructor and never modified/cleared. - RefPtr mTransaction; + SafeRefPtr mTransaction; JS::Heap mCachedKeyPath; // This normally points to the ObjectStoreSpec owned by the parent IDBDatabase @@ -96,7 +96,7 @@ class IDBObjectStore final : public nsISupports, public nsWrapperCache { }; static MOZ_MUST_USE RefPtr Create( - IDBTransaction* aTransaction, ObjectStoreSpec& aSpec); + SafeRefPtr aTransaction, ObjectStoreSpec& aSpec); static void AppendIndexUpdateInfo(int64_t aIndexID, const KeyPath& aKeyPath, bool aMultiEntry, const nsCString& aLocale, @@ -150,10 +150,28 @@ class IDBObjectStore final : public nsISupports, public nsWrapperCache { MOZ_MUST_USE RefPtr IndexNames(); - IDBTransaction* Transaction() const { + const IDBTransaction& TransactionRef() const { AssertIsOnOwningThread(); - return mTransaction; + return *mTransaction; + } + + IDBTransaction& MutableTransactionRef() { + AssertIsOnOwningThread(); + + return *mTransaction; + } + + SafeRefPtr AcquireTransaction() const { + AssertIsOnOwningThread(); + + return mTransaction.clonePtr(); + } + + RefPtr Transaction() const { + AssertIsOnOwningThread(); + + return AsRefPtr(mTransaction.clonePtr()); } MOZ_MUST_USE RefPtr Add(JSContext* aCx, @@ -236,7 +254,8 @@ class IDBObjectStore final : public nsISupports, public nsWrapperCache { JS::Handle aGivenProto) override; private: - IDBObjectStore(IDBTransaction* aTransaction, ObjectStoreSpec* aSpec); + IDBObjectStore(SafeRefPtr aTransaction, + ObjectStoreSpec* aSpec); ~IDBObjectStore(); diff --git a/dom/indexedDB/IDBRequest.cpp b/dom/indexedDB/IDBRequest.cpp index 5552c1d9dd12..ef8696fca4a1 100644 --- a/dom/indexedDB/IDBRequest.cpp +++ b/dom/indexedDB/IDBRequest.cpp @@ -89,7 +89,7 @@ void IDBRequest::InitMembers() { // static RefPtr IDBRequest::Create(JSContext* aCx, IDBDatabase* aDatabase, - IDBTransaction* aTransaction) { + SafeRefPtr aTransaction) { MOZ_ASSERT(aCx); MOZ_ASSERT(aDatabase); aDatabase->AssertIsOnOwningThread(); @@ -97,7 +97,7 @@ RefPtr IDBRequest::Create(JSContext* aCx, IDBDatabase* aDatabase, RefPtr request = new IDBRequest(aDatabase); CaptureCaller(aCx, request->mFilename, &request->mLineNo, &request->mColumn); - request->mTransaction = aTransaction; + request->mTransaction = std::move(aTransaction); return request; } @@ -106,11 +106,11 @@ RefPtr IDBRequest::Create(JSContext* aCx, IDBDatabase* aDatabase, RefPtr IDBRequest::Create(JSContext* aCx, IDBObjectStore* aSourceAsObjectStore, IDBDatabase* aDatabase, - IDBTransaction* aTransaction) { + SafeRefPtr aTransaction) { MOZ_ASSERT(aSourceAsObjectStore); aSourceAsObjectStore->AssertIsOnOwningThread(); - auto request = Create(aCx, aDatabase, aTransaction); + auto request = Create(aCx, aDatabase, std::move(aTransaction)); request->mSourceAsObjectStore = aSourceAsObjectStore; @@ -120,11 +120,11 @@ RefPtr IDBRequest::Create(JSContext* aCx, // static RefPtr IDBRequest::Create(JSContext* aCx, IDBIndex* aSourceAsIndex, IDBDatabase* aDatabase, - IDBTransaction* aTransaction) { + SafeRefPtr aTransaction) { MOZ_ASSERT(aSourceAsIndex); aSourceAsIndex->AssertIsOnOwningThread(); - auto request = Create(aCx, aDatabase, aTransaction); + auto request = Create(aCx, aDatabase, std::move(aTransaction)); request->mSourceAsIndex = aSourceAsIndex; @@ -264,6 +264,10 @@ void IDBRequest::GetResult(JS::MutableHandle aResult, aResult.set(mResultVal); } +// XXX This function should be renamed, it doesn't set the callback, but uses +// the callback to set the result. In addition, the ResultCallback class can be +// removed, a functor can just be passed instead. The same should be done for +// IDBFileRequest::SetResultCallback. void IDBRequest::SetResultCallback(ResultCallback* aCallback) { AssertIsOnOwningThread(); MOZ_ASSERT(aCallback); @@ -366,7 +370,7 @@ void IDBRequest::GetEventTargetParent(EventChainPreVisitor& aVisitor) { AssertIsOnOwningThread(); aVisitor.mCanHandle = true; - aVisitor.SetParentTarget(mTransaction, false); + aVisitor.SetParentTarget(mTransaction.unsafeGetRawPtr(), false); } IDBOpenDBRequest::IDBOpenDBRequest(SafeRefPtr aFactory, @@ -417,12 +421,12 @@ RefPtr IDBOpenDBRequest::Create( return request; } -void IDBOpenDBRequest::SetTransaction(IDBTransaction* aTransaction) { +void IDBOpenDBRequest::SetTransaction(SafeRefPtr aTransaction) { AssertIsOnOwningThread(); MOZ_ASSERT(!aTransaction || !mTransaction); - mTransaction = aTransaction; + mTransaction = std::move(aTransaction); } void IDBOpenDBRequest::DispatchNonTransactionError(nsresult aErrorCode) { diff --git a/dom/indexedDB/IDBRequest.h b/dom/indexedDB/IDBRequest.h index 680d30187e03..9aa3b3153590 100644 --- a/dom/indexedDB/IDBRequest.h +++ b/dom/indexedDB/IDBRequest.h @@ -50,7 +50,7 @@ class IDBRequest : public DOMEventTargetHelper { RefPtr mSourceAsIndex; RefPtr mSourceAsCursor; - RefPtr mTransaction; + SafeRefPtr mTransaction; JS::Heap mResultVal; RefPtr mError; @@ -65,19 +65,17 @@ class IDBRequest : public DOMEventTargetHelper { public: class ResultCallback; - static MOZ_MUST_USE RefPtr Create(JSContext* aCx, - IDBDatabase* aDatabase, - IDBTransaction* aTransaction); + static MOZ_MUST_USE RefPtr Create( + JSContext* aCx, IDBDatabase* aDatabase, + SafeRefPtr aTransaction); - static MOZ_MUST_USE RefPtr Create(JSContext* aCx, - IDBObjectStore* aSource, - IDBDatabase* aDatabase, - IDBTransaction* aTransaction); + static MOZ_MUST_USE RefPtr Create( + JSContext* aCx, IDBObjectStore* aSource, IDBDatabase* aDatabase, + SafeRefPtr aTransaction); - static MOZ_MUST_USE RefPtr Create(JSContext* aCx, - IDBIndex* aSource, - IDBDatabase* aDatabase, - IDBTransaction* aTransaction); + static MOZ_MUST_USE RefPtr Create( + JSContext* aCx, IDBIndex* aSource, IDBDatabase* aDatabase, + SafeRefPtr aTransaction); static void CaptureCaller(JSContext* aCx, nsAString& aFilename, uint32_t* aLineNo, uint32_t* aColumn); @@ -138,10 +136,29 @@ class IDBRequest : public DOMEventTargetHelper { GetResult(aResult, aRv); } - IDBTransaction* GetTransaction() const { + Maybe MaybeTransactionRef() const { AssertIsOnOwningThread(); - return mTransaction; + return mTransaction ? SomeRef(*mTransaction) : Nothing(); + } + + IDBTransaction& MutableTransactionRef() const { + AssertIsOnOwningThread(); + + return *mTransaction; + } + + SafeRefPtr AcquireTransaction() const { + AssertIsOnOwningThread(); + + return mTransaction.clonePtr(); + } + + // For WebIDL binding. + RefPtr GetTransaction() const { + AssertIsOnOwningThread(); + + return AsRefPtr(mTransaction.clonePtr()); } IDBRequestReadyState ReadyState() const; @@ -196,7 +213,7 @@ class IDBOpenDBRequest final : public IDBRequest { bool IsFileHandleDisabled() const { return mFileHandleDisabled; } - void SetTransaction(IDBTransaction* aTransaction); + void SetTransaction(SafeRefPtr aTransaction); void DispatchNonTransactionError(nsresult aErrorCode); diff --git a/dom/indexedDB/IDBTransaction.cpp b/dom/indexedDB/IDBTransaction.cpp index 4437ae7f84d1..4a17eac5c300 100644 --- a/dom/indexedDB/IDBTransaction.cpp +++ b/dom/indexedDB/IDBTransaction.cpp @@ -138,7 +138,7 @@ IDBTransaction::~IDBTransaction() { MOZ_ASSERT_IF(HasTransactionChild(), mFiredCompleteOrAbort); if (mRegistered) { - mDatabase->UnregisterTransaction(this); + mDatabase->UnregisterTransaction(*this); #ifdef DEBUG mRegistered = false; #endif @@ -160,7 +160,7 @@ IDBTransaction::~IDBTransaction() { } // static -RefPtr IDBTransaction::CreateVersionChange( +SafeRefPtr IDBTransaction::CreateVersionChange( IDBDatabase* const aDatabase, BackgroundVersionChangeTransactionChild* const aActor, IDBOpenDBRequest* const aOpenRequest, const int64_t aNextObjectStoreId, @@ -177,7 +177,7 @@ RefPtr IDBTransaction::CreateVersionChange( nsString filename; uint32_t lineNo, column; aOpenRequest->GetCallerLocation(filename, &lineNo, &column); - auto transaction = MakeRefPtr( + auto transaction = MakeSafeRefPtr( aDatabase, emptyObjectStoreNames, Mode::VersionChange, std::move(filename), lineNo, column, CreatedFromFactoryFunction{}); @@ -187,14 +187,14 @@ RefPtr IDBTransaction::CreateVersionChange( transaction->mNextObjectStoreId = aNextObjectStoreId; transaction->mNextIndexId = aNextIndexId; - aDatabase->RegisterTransaction(transaction); + aDatabase->RegisterTransaction(*transaction); transaction->mRegistered = true; return transaction; } // static -RefPtr IDBTransaction::Create( +SafeRefPtr IDBTransaction::Create( JSContext* const aCx, IDBDatabase* const aDatabase, const nsTArray& aObjectStoreNames, const Mode aMode) { MOZ_ASSERT(aDatabase); @@ -206,7 +206,7 @@ RefPtr IDBTransaction::Create( nsString filename; uint32_t lineNo, column; IDBRequest::CaptureCaller(aCx, filename, &lineNo, &column); - auto transaction = MakeRefPtr( + auto transaction = MakeSafeRefPtr( aDatabase, aObjectStoreNames, aMode, std::move(filename), lineNo, column, CreatedFromFactoryFunction{}); @@ -217,7 +217,8 @@ RefPtr IDBTransaction::Create( workerPrivate->AssertIsOnWorkerThread(); RefPtr workerRef = StrongWorkerRef::Create( - workerPrivate, "IDBTransaction", [transaction]() { + workerPrivate, "IDBTransaction", + [transaction = AsRefPtr(transaction.clonePtr())]() { transaction->AssertIsOnOwningThread(); if (!transaction->IsCommittingOrFinished()) { IDB_REPORT_INTERNAL_ERR(); @@ -236,22 +237,23 @@ RefPtr IDBTransaction::Create( transaction->mWorkerRef = std::move(workerRef); } - nsCOMPtr runnable = do_QueryObject(transaction); + nsCOMPtr runnable = + do_QueryObject(transaction.unsafeGetRawPtr()); nsContentUtils::AddPendingIDBTransaction(runnable.forget()); - aDatabase->RegisterTransaction(transaction); + aDatabase->RegisterTransaction(*transaction); transaction->mRegistered = true; return transaction; } // static -IDBTransaction* IDBTransaction::GetCurrent() { +Maybe IDBTransaction::MaybeCurrent() { using namespace mozilla::ipc; MOZ_ASSERT(BackgroundChild::GetForCurrentThread()); - return GetIndexedDBThreadLocal()->GetCurrentTransaction(); + return GetIndexedDBThreadLocal()->MaybeCurrentTransactionRef(); } #ifdef DEBUG @@ -402,9 +404,11 @@ void IDBTransaction::SendCommit(const bool aAutoCommit) { // https://w3c.github.io/IndexedDB/#async-execute-request, step 5.3.). With // automatic commit, this is not necessary, as the transaction's state will // only be set to committing after the last request completed. - const bool dispatchingEventForThisTransaction = + const auto maybeCurrentTransaction = BackgroundChildImpl::GetThreadLocalForCurrentThread() - ->mIndexedDBThreadLocal->GetCurrentTransaction() == this; + ->mIndexedDBThreadLocal->MaybeCurrentTransactionRef(); + const bool dispatchingEventForThisTransaction = + maybeCurrentTransaction && &maybeCurrentTransaction.ref() == this; return Some(requestSerialNumber ? (requestSerialNumber - @@ -506,7 +510,8 @@ RefPtr IDBTransaction::CreateObjectStore( mBackgroundActor.mVersionChangeBackgroundActor->SendCreateObjectStore( aSpec.metadata())); - RefPtr objectStore = IDBObjectStore::Create(this, aSpec); + RefPtr objectStore = IDBObjectStore::Create( + SafeRefPtr{this, AcquireStrongRefFromRawPtr{}}, aSpec); MOZ_ASSERT(objectStore); mObjectStores.AppendElement(objectStore); @@ -931,7 +936,8 @@ RefPtr IDBTransaction::ObjectStore(const nsAString& aName, if (foundIt != mObjectStores.cend()) { objectStore = *foundIt; } else { - objectStore = IDBObjectStore::Create(this, *spec); + objectStore = IDBObjectStore::Create( + SafeRefPtr{this, AcquireStrongRefFromRawPtr{}}, *spec); MOZ_ASSERT(objectStore); mObjectStores.AppendElement(objectStore); diff --git a/dom/indexedDB/IDBTransaction.h b/dom/indexedDB/IDBTransaction.h index 82289d631e00..58f8e85abbeb 100644 --- a/dom/indexedDB/IDBTransaction.h +++ b/dom/indexedDB/IDBTransaction.h @@ -122,17 +122,17 @@ class IDBTransaction final #endif public: - static MOZ_MUST_USE RefPtr CreateVersionChange( + static MOZ_MUST_USE SafeRefPtr CreateVersionChange( IDBDatabase* aDatabase, indexedDB::BackgroundVersionChangeTransactionChild* aActor, IDBOpenDBRequest* aOpenRequest, int64_t aNextObjectStoreId, int64_t aNextIndexId); - static MOZ_MUST_USE RefPtr Create( + static MOZ_MUST_USE SafeRefPtr Create( JSContext* aCx, IDBDatabase* aDatabase, const nsTArray& aObjectStoreNames, Mode aMode); - static IDBTransaction* GetCurrent(); + static Maybe MaybeCurrent(); void AssertIsOnOwningThread() const #ifdef DEBUG @@ -382,6 +382,14 @@ class IDBTransaction final bool HasTransactionChild() const; }; +inline bool ReferenceEquals(const Maybe& aLHS, + const Maybe& aRHS) { + if (aLHS.isNothing() != aRHS.isNothing()) { + return false; + } + return aLHS.isNothing() || &aLHS.ref() == &aRHS.ref(); +} + } // namespace dom } // namespace mozilla diff --git a/dom/indexedDB/ProfilerHelpers.h b/dom/indexedDB/ProfilerHelpers.h index c90fe2b24e4b..d9d851788f2c 100644 --- a/dom/indexedDB/ProfilerHelpers.h +++ b/dom/indexedDB/ProfilerHelpers.h @@ -88,13 +88,11 @@ class MOZ_STACK_CLASS LoggingString final : public nsAutoCString { Append(kQuote); } - explicit LoggingString(IDBTransaction* aTransaction) + explicit LoggingString(const IDBTransaction& aTransaction) : nsAutoCString(kOpenBracket) { - MOZ_ASSERT(aTransaction); - NS_NAMED_LITERAL_CSTRING(kCommaSpace, ", "); - const nsTArray& stores = aTransaction->ObjectStoreNamesInternal(); + const nsTArray& stores = aTransaction.ObjectStoreNamesInternal(); for (uint32_t count = stores.Length(), index = 0; index < count; index++) { Append(kQuote); @@ -109,7 +107,7 @@ class MOZ_STACK_CLASS LoggingString final : public nsAutoCString { Append(kCloseBracket); Append(kCommaSpace); - switch (aTransaction->GetMode()) { + switch (aTransaction.GetMode()) { case IDBTransaction::Mode::ReadOnly: AppendLiteral("\"readonly\""); break; diff --git a/js/src/new-regexp/RegExpAPI.cpp b/js/src/new-regexp/RegExpAPI.cpp index 8613461f4236..923196323595 100644 --- a/js/src/new-regexp/RegExpAPI.cpp +++ b/js/src/new-regexp/RegExpAPI.cpp @@ -493,8 +493,7 @@ bool CompilePattern(JSContext* cx, MutableHandleRegExpShared re, #ifdef DEBUG UniquePtr tracer_masm; if (jit::JitOptions.traceRegExpAssembler) { - tracer_masm = MakeUnique(cx->isolate, - masm_ptr); + tracer_masm = MakeUnique(cx->isolate, masm_ptr); masm_ptr = tracer_masm.get(); } #endif