Bug 618590 Part 1: Change Abort() so that it's possible to abort from outside of transaction callbacks. r=bent a=blocker

dom/indexedDB/IDBTransaction.cpp

@@ -174,9 +174,9 @@ IDBTransaction::OnRequestFinished()
   NS_ASSERTION(mPendingRequests, "Mismatched calls!");
   --mPendingRequests;
   if (!mPendingRequests) {
-    if (!mAborted) {
-      NS_ASSERTION(mReadyState == nsIIDBTransaction::LOADING, "Bad state!");
-    }
+    NS_ASSERTION(mAborted || mReadyState == nsIIDBTransaction::LOADING,
+                 "Bad state!");
+    mReadyState = IDBTransaction::COMMITTING;
     CommitOrRollback();
   }
 }
@@ -762,7 +762,10 @@ IDBTransaction::Abort()
 {
   NS_ASSERTION(NS_IsMainThread(), "Wrong thread!");
 
-  if (!IsOpen()) {
+  // We can't use IsOpen here since we need it to be possible to call Abort()
+  // even from outside of transaction callbacks.
+  if (mReadyState != IDBTransaction::INITIAL &&
+      mReadyState != IDBTransaction::LOADING) {
     return NS_ERROR_DOM_INDEXEDDB_NOT_ALLOWED_ERR;
   }
 
@@ -979,7 +982,7 @@ CommitHelper::Run()
     if (!mAborted) {
       NS_NAMED_LITERAL_CSTRING(release, "END TRANSACTION");
       if (NS_FAILED(mConnection->ExecuteSimpleSQL(release))) {
-        mAborted = PR_TRUE;
+        mAborted = true;
       }
     }
 

dom/indexedDB/nsIIDBTransaction.idl

@@ -57,7 +57,8 @@ interface nsIIDBTransaction : nsISupports
 
   const unsigned short INITIAL = 0;
   const unsigned short LOADING = 1;
-  const unsigned short DONE = 2;
+  const unsigned short COMMITTING = 2;
+  const unsigned short DONE = 3;
   readonly attribute unsigned short readyState;
 
   const unsigned short READ_ONLY = 0;

dom/indexedDB/test/test_transaction_abort.html

@@ -15,7 +15,9 @@
     {
       const Ci = Components.interfaces;
 
+      const INITIAL = Ci.nsIIDBTransaction.INITIAL;
       const LOADING = Ci.nsIIDBTransaction.LOADING;
+      const COMMITTING = Ci.nsIIDBTransaction.COMMITTING;
       const DONE = Ci.nsIIDBTransaction.DONE;
       const READ_ONLY = Ci.nsIIDBTransaction.READ_ONLY;
       const READ_WRITE = Ci.nsIIDBTransaction.READ_WRITE;
@@ -48,7 +50,7 @@
       is(transaction.mode, VERSION_CHANGE, "Correct mode");
       is(transaction.objectStoreNames.length, 1, "Correct names length");
       is(transaction.objectStoreNames.item(0), "foo", "Correct name");
-      is(transaction.objectStore("foo").name, "foo", "Can get stores");
+      is(transaction.objectStore("foo"), objectStore, "Can get stores");
       is(transaction.oncomplete, null, "No complete listener");
       is(transaction.onabort, null, "No abort listener");
       is(transaction.ontimeout, null, "No timeout listener");
@@ -208,16 +210,17 @@
 
       let keys = [];
       let abortEventCount = 0;
-      objectStore = db.transaction("foo", READ_WRITE).objectStore("foo");
-
-      for (let i = 0; i < 10; i++) {
-        request = objectStore.add({});
-        request.onerror = function(event) {
+      function abortErrorHandler(event) {
           is(event.target.errorCode, IDBDatabaseException.ABORT_ERR,
              "Good code");
           abortEventCount++;
           event.preventDefault();
-        };
+      };
+      objectStore = db.transaction("foo", READ_WRITE).objectStore("foo");
+
+      for (let i = 0; i < 10; i++) {
+        request = objectStore.add({});
+        request.onerror = abortErrorHandler;
         request.onsuccess = function(event) {
           keys.push(event.target.result);
           if (keys.length == 5) {
@@ -241,6 +244,129 @@
         is(event.target.result, undefined, "Object was removed by abort");
       }
 
+      // Set up some predictible data
+      transaction = db.transaction("foo", READ_WRITE);
+      objectStore = transaction.objectStore("foo");
+      objectStore.clear();
+      objectStore.add({}, 1);
+      objectStore.add({}, 2);
+      request = objectStore.add({}, 1);
+      request.onsuccess = function() {
+        ok(false, "inserting duplicate key should fail");
+      }
+      request.onerror = function(event) {
+        ok(true, "inserting duplicate key should fail");
+        event.preventDefault();
+      }
+      transaction.oncomplete = grabEventAndContinueHandler;
+      yield;
+
+      // Check when aborting is allowed
+      abortEventCount = 0;
+      let expectedAbortEventCount = 0;
+
+      // During INITIAL
+      transaction = db.transaction("foo");
+      is(transaction.readyState, INITIAL, "in INITIAL state");
+      transaction.abort();
+      is(transaction.readyState, DONE, "in DONE state after abort()");
+      try {
+        transaction.abort();
+        ok(false, "second abort should throw an error");
+      }
+      catch (ex) {
+        ok(true, "second abort should throw an error");
+      }
+
+      // During LOADING
+      transaction = db.transaction("foo");
+      transaction.objectStore("foo").get(1).onerror = abortErrorHandler;
+      expectedAbortEventCount++;
+      is(transaction.readyState, LOADING, "in LOADING state");
+      transaction.abort();
+      is(transaction.readyState, DONE, "in DONE state after abort()");
+      try {
+        transaction.abort();
+        ok(false, "second abort should throw an error");
+      }
+      catch (ex) {
+        ok(true, "second abort should throw an error");
+      }
+
+      // During LOADING from callback
+      transaction = db.transaction("foo");
+      transaction.objectStore("foo").get(1).onsuccess = grabEventAndContinueHandler;
+      event = yield;
+      transaction.objectStore("foo").get(1).onerror = abortErrorHandler;
+      expectedAbortEventCount++
+      is(transaction.readyState, LOADING, "in LOADING state");
+      transaction.abort();
+      is(transaction.readyState, DONE, "in DONE state after abort()");
+      try {
+        transaction.abort();
+        ok(false, "second abort should throw an error");
+      }
+      catch (ex) {
+        ok(true, "second abort should throw an error");
+      }
+
+      // During LOADING from error callback
+      transaction = db.transaction("foo", READ_WRITE);
+      transaction.objectStore("foo").add({}, 1).onerror = function(event) {
+        event.preventDefault();
+
+        transaction.objectStore("foo").get(1).onerror = abortErrorHandler;
+        expectedAbortEventCount++
+
+        is(transaction.readyState, LOADING, "in LOADING state");
+        transaction.abort();
+        is(transaction.readyState, DONE, "in DONE state after abort()");
+        continueToNextStep();
+      }
+      yield;
+
+      // In between callbacks
+      transaction = db.transaction("foo");
+      function makeNewRequest() {
+        let r = transaction.objectStore("foo").get(1);
+        r.onsuccess = makeNewRequest;
+        r.onerror = abortErrorHandler;
+      }
+      makeNewRequest();
+      transaction.objectStore("foo").get(1).onsuccess = function(event) {
+        SimpleTest.executeSoon(function() {
+          is(transaction.readyState, LOADING, "in LOADING state");
+          transaction.abort();
+          expectedAbortEventCount++;
+          is(transaction.readyState, DONE, "in DONE state after abort()");
+          continueToNextStep();
+        });
+      };
+      yield;
+      
+      // During COMMITTING
+      transaction = db.transaction("foo", READ_WRITE);
+      transaction.objectStore("foo").put({hello: "world"}, 1).onsuccess = function(event) {
+        continueToNextStep();
+      };
+      yield;
+      is(transaction.readyState, COMMITTING, "in COMMITTING state");
+      try {
+        transaction.abort();
+        ok(false, "second abort should throw an error");
+      }
+      catch (ex) {
+        ok(true, "second abort should throw an error");
+      }
+      transaction.oncomplete = grabEventAndContinueHandler;
+      event = yield;
+      is(transaction.readyState, DONE, "in DONE state");
+
+      // Since the previous transaction shouldn't have caused any error events,
+      // we know that all events should have fired by now.
+      is(abortEventCount, expectedAbortEventCount,
+         "All abort errors fired");
+
       finishTest();
       yield;
     }