diff --git a/dom/indexedDB/ActorsChild.cpp b/dom/indexedDB/ActorsChild.cpp index 56590791ef8f..a13dbc427df5 100644 --- a/dom/indexedDB/ActorsChild.cpp +++ b/dom/indexedDB/ActorsChild.cpp @@ -692,7 +692,7 @@ void DispatchErrorEvent(IDBRequest* aRequest, nsresult aErrorCode, } MOZ_ASSERT(!transaction || transaction->IsActive() || - transaction->IsAborted()); + transaction->IsAborted() || transaction->WasExplicitlyCommitted()); if (transaction && transaction->IsActive()) { transaction->TransitionToInactive(); @@ -760,7 +760,7 @@ void DispatchSuccessEvent(ResultHelper* aResultHelper, IDB_LOG_STRINGIFY(aEvent, kSuccessEventType)); } - MOZ_ASSERT_IF(transaction, + MOZ_ASSERT_IF(transaction && !transaction->WasExplicitlyCommitted(), transaction->IsActive() && !transaction->IsAborted()); IgnoredErrorResult rv; diff --git a/dom/indexedDB/ActorsParent.cpp b/dom/indexedDB/ActorsParent.cpp index 53171f8ccab7..86bc3c763496 100644 --- a/dom/indexedDB/ActorsParent.cpp +++ b/dom/indexedDB/ActorsParent.cpp @@ -6288,6 +6288,7 @@ class TransactionBase { FlippedOnce mCommitOrAbortReceived; FlippedOnce mCommittedOrAborted; FlippedOnce mForceAborted; + bool mHasFailedRequest = false; public: void AssertIsOnConnectionThread() const { @@ -6380,6 +6381,8 @@ class TransactionBase { void NoteFinishedRequest(); + void NoteFailedRequest(nsresult aResultCode); + void Invalidate(); protected: @@ -14079,6 +14082,10 @@ bool TransactionBase::RecvCommit() { mCommitOrAbortReceived.Flip(); + // Ignore requests that failed before we were committing (cf. + // https://w3c.github.io/IndexedDB/#async-execute-request step 5.3 vs. 5.4). + mHasFailedRequest = false; + MaybeCommitOrAbort(); return true; } @@ -14635,6 +14642,15 @@ void TransactionBase::NoteFinishedRequest() { MaybeCommitOrAbort(); } +void TransactionBase::NoteFailedRequest(const nsresult aResultCode) { + AssertIsOnConnectionThread(); + + // TODO: Should we store the result code? Maybe also the request that failed? + MOZ_ASSERT(NS_FAILED(aResultCode)); + + mHasFailedRequest = true; +} + void TransactionBase::Invalidate() { AssertIsOnBackgroundThread(); MOZ_ASSERT(mInvalidated == mInvalidatedOnAnyThread); @@ -22593,6 +22609,8 @@ void TransactionDatabaseOperationBase::RunOnConnectionThread() { } if (NS_FAILED(rv)) { + (*mTransaction)->NoteFailedRequest(rv); + SetFailureCode(rv); } } @@ -22901,6 +22919,10 @@ TransactionBase::CommitOp::Run() { MOZ_ASSERT(mTransaction); mTransaction->AssertIsOnConnectionThread(); + if (NS_SUCCEEDED(mResultCode) && mTransaction->mHasFailedRequest) { + mResultCode = NS_ERROR_DOM_INDEXEDDB_ABORT_ERR; + } + AUTO_PROFILER_LABEL("TransactionBase::CommitOp::Run", DOM); IDB_LOG_MARK_PARENT_TRANSACTION_REQUEST( diff --git a/dom/indexedDB/IDBTransaction.cpp b/dom/indexedDB/IDBTransaction.cpp index 7de100c0526c..20dab73a981f 100644 --- a/dom/indexedDB/IDBTransaction.cpp +++ b/dom/indexedDB/IDBTransaction.cpp @@ -340,30 +340,32 @@ void IDBTransaction::OnNewRequest() { void IDBTransaction::OnRequestFinished( const bool aRequestCompletedSuccessfully) { AssertIsOnOwningThread(); - MOZ_ASSERT(mReadyState == ReadyState::Inactive || - mReadyState == ReadyState::Finished); + MOZ_ASSERT(mReadyState != ReadyState::Active); MOZ_ASSERT_IF(mReadyState == ReadyState::Finished, !NS_SUCCEEDED(mAbortCode)); MOZ_ASSERT(mPendingRequestCount); --mPendingRequestCount; if (!mPendingRequestCount) { + MOZ_ASSERT(mReadyState != ReadyState::Active || mStarted); + if (mSentCommitOrAbort) { + return; + } + if (mReadyState == ReadyState::Inactive) { mReadyState = ReadyState::Committing; } if (aRequestCompletedSuccessfully) { if (NS_SUCCEEDED(mAbortCode)) { - SendCommit(); + SendCommit(true); } else { SendAbort(mAbortCode); } } else { // Don't try to send any more messages to the parent if the request actor // was killed. -#ifdef DEBUG mSentCommitOrAbort.Flip(); -#endif IDB_LOG_MARK_CHILD_TRANSACTION( "Request actor was killed, transaction will be aborted", "IDBTransaction abort", LoggingSerialNumber()); @@ -371,25 +373,23 @@ void IDBTransaction::OnRequestFinished( } } -void IDBTransaction::SendCommit() { +void IDBTransaction::SendCommit(const bool aAutoCommit) { AssertIsOnOwningThread(); MOZ_ASSERT(NS_SUCCEEDED(mAbortCode)); MOZ_ASSERT(IsCommittingOrFinished()); - MOZ_ASSERT(!mPendingRequestCount); // Don't do this in the macro because we always need to increment the serial // number to keep in sync with the parent. const uint64_t requestSerialNumber = IDBRequest::NextSerialNumber(); IDB_LOG_MARK_CHILD_TRANSACTION_REQUEST( - "All requests complete, committing transaction", "IDBTransaction commit", - LoggingSerialNumber(), requestSerialNumber); + "Committing transaction (%s)", "IDBTransaction commit (%s)", + LoggingSerialNumber(), requestSerialNumber, + aAutoCommit ? "automatically" : "explicitly"); DoWithTransactionChild([](auto& actor) { actor.SendCommit(); }); -#ifdef DEBUG mSentCommitOrAbort.Flip(); -#endif } void IDBTransaction::SendAbort(const nsresult aResultCode) { @@ -408,9 +408,7 @@ void IDBTransaction::SendAbort(const nsresult aResultCode) { DoWithTransactionChild( [aResultCode](auto& actor) { actor.SendAbort(aResultCode); }); -#ifdef DEBUG mSentCommitOrAbort.Flip(); -#endif } void IDBTransaction::NoteActiveTransaction() { @@ -704,13 +702,26 @@ void IDBTransaction::Abort(ErrorResult& aRv) { void IDBTransaction::Commit(ErrorResult& aRv) { AssertIsOnOwningThread(); - if (IsCommittingOrFinished()) { - aRv = NS_ERROR_DOM_INDEXEDDB_NOT_ALLOWED_ERR; + if (mReadyState != ReadyState::Active || !mNotedActiveTransaction) { + aRv = NS_ERROR_DOM_INVALID_STATE_ERR; return; } - // TODO - aRv = NS_ERROR_NOT_IMPLEMENTED; + MOZ_ASSERT(!mSentCommitOrAbort); + + MOZ_ASSERT(mReadyState == ReadyState::Active); + mReadyState = ReadyState::Committing; + if (NS_WARN_IF(NS_FAILED(mAbortCode))) { + SendAbort(mAbortCode); + aRv = mAbortCode; + return; + } + +#ifdef DEBUG + mWasExplicitlyCommitted.Flip(); +#endif + + SendCommit(false); } void IDBTransaction::FireCompleteOrAbortEvents(const nsresult aResult) { @@ -968,6 +979,10 @@ IDBTransaction::Run() { return NS_OK; } + if (ReadyState::Committing == mReadyState) { + MOZ_ASSERT(mWasExplicitlyCommitted); + return NS_OK; + } // We're back at the event loop, no longer newborn, so // return to Inactive state: @@ -990,7 +1005,7 @@ void IDBTransaction::CommitIfNotStarted() { MOZ_ASSERT(!mPendingRequestCount); mReadyState = ReadyState::Finished; - SendCommit(); + SendCommit(true); } } diff --git a/dom/indexedDB/IDBTransaction.h b/dom/indexedDB/IDBTransaction.h index ecff3c0dfbb1..3ff1ef61e9b8 100644 --- a/dom/indexedDB/IDBTransaction.h +++ b/dom/indexedDB/IDBTransaction.h @@ -113,10 +113,11 @@ class IDBTransaction final ///< incomplete). FlippedOnce mAbortedByScript; bool mNotedActiveTransaction; + FlippedOnce mSentCommitOrAbort; #ifdef DEBUG - FlippedOnce mSentCommitOrAbort; FlippedOnce mFiredCompleteOrAbort; + FlippedOnce mWasExplicitlyCommitted; #endif public: @@ -203,6 +204,10 @@ class IDBTransaction final return NS_FAILED(mAbortCode); } +#ifdef DEBUG + bool WasExplicitlyCommitted() const { return mWasExplicitlyCommitted; } +#endif + template class AutoRestoreState { public: @@ -357,7 +362,7 @@ class IDBTransaction final void AbortInternal(nsresult aAbortCode, RefPtr aError); - void SendCommit(); + void SendCommit(bool aAutoCommit); void SendAbort(nsresult aResultCode); diff --git a/testing/web-platform/meta/IndexedDB/idb-explicit-commit-throw.any.js.ini b/testing/web-platform/meta/IndexedDB/idb-explicit-commit-throw.any.js.ini deleted file mode 100644 index bfe98bc05d64..000000000000 --- a/testing/web-platform/meta/IndexedDB/idb-explicit-commit-throw.any.js.ini +++ /dev/null @@ -1,9 +0,0 @@ -[idb-explicit-commit-throw.any.worker.html] - [Any errors in callbacks that run after an explicit commit will not stop the commit from being processed.] - expected: FAIL - - -[idb-explicit-commit-throw.any.html] - [Any errors in callbacks that run after an explicit commit will not stop the commit from being processed.] - expected: FAIL - diff --git a/testing/web-platform/meta/IndexedDB/idb-explicit-commit.any.js.ini b/testing/web-platform/meta/IndexedDB/idb-explicit-commit.any.js.ini deleted file mode 100644 index d32bd24f6039..000000000000 --- a/testing/web-platform/meta/IndexedDB/idb-explicit-commit.any.js.ini +++ /dev/null @@ -1,69 +0,0 @@ -[idb-explicit-commit.any.worker.html] - [Calling commit on a committed transaction throws.] - expected: FAIL - - [Explicitly committed data can be read back out.] - expected: FAIL - - [Calling txn.commit() when txn is inactive should throw.] - expected: FAIL - - [commit() on a version change transaction does not cause errors.] - expected: FAIL - - [Puts issued after commit are not fulfilled.] - expected: FAIL - - [Calling abort on a committed transaction throws and does not prevent persisting the data.] - expected: FAIL - - [A committed transaction becomes inactive immediately.] - expected: FAIL - - [A committed transaction is inactive in future request callbacks.] - expected: FAIL - - [Transactions that explicitly commit and have errors should abort.] - expected: FAIL - - [Transactions with same scope should stay in program order, even if one calls commit.] - expected: FAIL - - [Transactions that handle all errors properly should behave as expected when an explicit commit is called in an onerror handler.] - expected: FAIL - - -[idb-explicit-commit.any.html] - [Calling commit on a committed transaction throws.] - expected: FAIL - - [Explicitly committed data can be read back out.] - expected: FAIL - - [Calling txn.commit() when txn is inactive should throw.] - expected: FAIL - - [commit() on a version change transaction does not cause errors.] - expected: FAIL - - [Puts issued after commit are not fulfilled.] - expected: FAIL - - [Calling abort on a committed transaction throws and does not prevent persisting the data.] - expected: FAIL - - [A committed transaction becomes inactive immediately.] - expected: FAIL - - [A committed transaction is inactive in future request callbacks.] - expected: FAIL - - [Transactions that explicitly commit and have errors should abort.] - expected: FAIL - - [Transactions with same scope should stay in program order, even if one calls commit.] - expected: FAIL - - [Transactions that handle all errors properly should behave as expected when an explicit commit is called in an onerror handler.] - expected: FAIL -