Bug 1497007 - Fix state model, return to Inactive state after creation when returning to event loop. r=dom-workers-and-storage-reviewers,asuth

Also fix the behaviour of IDBTransaction::CanAcceptRequests, which, after the
new state model introduced along with the addition of IDBTransaction.commit to
the spec, should only depend on mReadyState. This makes the mCreated flag
redundant, which is removed by this patch.

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

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

dom/indexedDB/ActorsChild.cpp

@@ -779,7 +779,7 @@ void DispatchSuccessEvent(ResultHelper* aResultHelper,
       transaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR);
     } else {
       // To handle upgrade transaction.
-      transaction->Run();
+      transaction->CommitIfNotStarted();
     }
   }
 }

dom/indexedDB/IDBTransaction.cpp

@@ -104,7 +104,6 @@ IDBTransaction::IDBTransaction(IDBDatabase* const aDatabase,
       mLineNo(aLineNo),
       mColumn(aColumn),
       mMode(aMode),
-      mCreating(false),
       mRegistered(false),
       mNotedActiveTransaction(false) {
   MOZ_ASSERT(aDatabase);
@@ -133,7 +132,7 @@ IDBTransaction::IDBTransaction(IDBDatabase* const aDatabase,
 IDBTransaction::~IDBTransaction() {
   AssertIsOnOwningThread();
   MOZ_ASSERT(!mPendingRequestCount);
-  MOZ_ASSERT(!mCreating);
+  MOZ_ASSERT(mReadyState == ReadyState::Finished);
   MOZ_ASSERT(!mNotedActiveTransaction);
   MOZ_ASSERT(mSentCommitOrAbort);
   MOZ_ASSERT_IF(HasTransactionChild(), mFiredCompleteOrAbort);
@@ -240,8 +239,6 @@ RefPtr<IDBTransaction> IDBTransaction::Create(
   nsCOMPtr<nsIRunnable> runnable = do_QueryObject(transaction);
   nsContentUtils::AddPendingIDBTransaction(runnable.forget());
 
-  transaction->mCreating = true;
-
   aDatabase->RegisterTransaction(transaction);
   transaction->mRegistered = true;
 
@@ -436,14 +433,7 @@ void IDBTransaction::MaybeNoteInactiveTransaction() {
 bool IDBTransaction::CanAcceptRequests() const {
   AssertIsOnOwningThread();
 
-  // If we haven't started anything then we can accept requests.
-  // If we've already started then we need to check to see if we still have the
-  // mCreating flag set. If we do (i.e. we haven't returned to the event loop
-  // from the time we were created) then we can accept requests. Otherwise check
-  // the currently running transaction to see if it's the same. We only allow
-  // other requests to be made if this transaction is currently running.
-  return mReadyState == ReadyState::Active &&
-         (!mStarted || mCreating || GetCurrent() == this);
+  return mReadyState == ReadyState::Active;
 }
 
 IDBTransaction::AutoRestoreState<IDBTransaction::ReadyState::Inactive,
@@ -969,22 +959,40 @@ NS_IMETHODIMP
 IDBTransaction::Run() {
   AssertIsOnOwningThread();
 
-  // We're back at the event loop, no longer newborn.
-  mCreating = false;
+  // TODO: Instead of checking for Finished and Committing states here, we could
+  // remove the transaction from the pending IDB transactions list on
+  // abort/commit.
 
-  MOZ_ASSERT_IF(mReadyState == ReadyState::Finished, IsAborted());
-
-  // Maybe commit if there were no requests generated.
-  if (!mStarted && mReadyState != ReadyState::Finished) {
-    MOZ_ASSERT(mReadyState == ReadyState::Inactive ||
-               mReadyState == ReadyState::Active);
-    mReadyState = ReadyState::Finished;
-
-    SendCommit();
+  if (ReadyState::Finished == mReadyState) {
+    MOZ_ASSERT(IsAborted());
+    return NS_OK;
   }
 
+
+  // We're back at the event loop, no longer newborn, so
+  // return to Inactive state:
+  // https://w3c.github.io/IndexedDB/#cleanup-indexed-database-transactions.
+  MOZ_ASSERT(ReadyState::Active == mReadyState);
+  mReadyState = ReadyState::Inactive;
+
+  CommitIfNotStarted();
+
   return NS_OK;
 }
 
+void IDBTransaction::CommitIfNotStarted() {
+  AssertIsOnOwningThread();
+
+  MOZ_ASSERT(ReadyState::Inactive == mReadyState);
+
+  // Maybe commit if there were no requests generated.
+  if (!mStarted) {
+    MOZ_ASSERT(!mPendingRequestCount);
+    mReadyState = ReadyState::Finished;
+
+    SendCommit();
+  }
+}
+
 }  // namespace dom
 }  // namespace mozilla

dom/indexedDB/IDBTransaction.h

@@ -108,8 +108,6 @@ class IDBTransaction final
   FlippedOnce<false> mStarted;
   const Mode mMode;
 
-  bool mCreating;    ///< Set between successful creation until the transaction
-                     ///< has run on the event-loop.
   bool mRegistered;  ///< Whether mDatabase->RegisterTransaction() has been
                      ///< called (which may not be the case if construction was
                      ///< incomplete).
@@ -316,6 +314,8 @@ class IDBTransaction final
   NS_DECL_NSIRUNNABLE
   NS_DECL_CYCLE_COLLECTION_CLASS_INHERITED(IDBTransaction, DOMEventTargetHelper)
 
+  void CommitIfNotStarted();
+
   // nsWrapperCache
   JSObject* WrapObject(JSContext* aCx,
                        JS::Handle<JSObject*> aGivenProto) override;