Bug 1497007 - Implemented IDBTransaction.commit. r=dom-workers-and-storage-reviewers,ytausky

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

--HG--
extra : moz-landing-system : lando

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;

dom/indexedDB/ActorsParent.cpp

@@ -6288,6 +6288,7 @@ class TransactionBase {
   FlippedOnce<false> mCommitOrAbortReceived;
   FlippedOnce<false> mCommittedOrAborted;
   FlippedOnce<false> 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(

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);
   }
 }
 

dom/indexedDB/IDBTransaction.h

@@ -113,10 +113,11 @@ class IDBTransaction final
                      ///< incomplete).
   FlippedOnce<false> mAbortedByScript;
   bool mNotedActiveTransaction;
+  FlippedOnce<false> mSentCommitOrAbort;
 
 #ifdef DEBUG
-  FlippedOnce<false> mSentCommitOrAbort;
   FlippedOnce<false> mFiredCompleteOrAbort;
+  FlippedOnce<false> mWasExplicitlyCommitted;
 #endif
 
  public:
@@ -203,6 +204,10 @@ class IDBTransaction final
     return NS_FAILED(mAbortCode);
   }
 
+#ifdef DEBUG
+  bool WasExplicitlyCommitted() const { return mWasExplicitlyCommitted; }
+#endif
+
   template <ReadyState OriginalState, ReadyState TemporaryState>
   class AutoRestoreState {
    public:
@@ -357,7 +362,7 @@ class IDBTransaction final
 
   void AbortInternal(nsresult aAbortCode, RefPtr<DOMException> aError);
 
-  void SendCommit();
+  void SendCommit(bool aAutoCommit);
 
   void SendAbort(nsresult aResultCode);
 

testing/web-platform/meta/IndexedDB/idb-explicit-commit-throw.any.js.ini

@@ -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
-

testing/web-platform/meta/IndexedDB/idb-explicit-commit.any.js.ini

@@ -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
-