Bug 1151017 - Support the 'close' Event on Databases. r=khuey

Outline of this patch:

1. Define a new ipdl message called |CloseAfterInvalidationComplete| to trigger the close event after all transactions are complete only if the database is invalidated by the user agent.
2. Make sure the following event sequence is consistent during invalidation according to the steps in |5.2. Closing a database| by the following 2 solutions:
     IDBRequest.onerror -> IDBTransaction.onerror -> IDBTransaction.onabort -> IDBDatabase.onclose.
   2.1. In parent process, do not force to abort the transactions after invalidation but wait for all the transactions in its child process are complete.
   2.2. In child process, make sure that each IDBTransaction will notify its completion to the parent after all its pending IDBRequests are finished.
3. Add test_database_onclose.js to test the close event especially when read/write operation is ongoing.
4. Add test_database_close_without_onclose.js as a XPCShell test because setTimeout() is not preferred in Mochitest to ensure that the IDBDatabase.onclose event won't be sent after closed normally.

dom/indexedDB/ActorsChild.cpp

@@ -1872,6 +1872,20 @@ BackgroundDatabaseChild::RecvInvalidate()
   return true;
 }
 
+bool
+BackgroundDatabaseChild::RecvCloseAfterInvalidationComplete()
+{
+  AssertIsOnOwningThread();
+
+  MaybeCollectGarbageOnIPCMessage();
+
+  if (mDatabase) {
+    mDatabase->DispatchTrustedEvent(nsDependentString(kCloseEventType));
+  }
+
+  return true;
+}
+
 /*******************************************************************************
  * BackgroundDatabaseRequestChild
  ******************************************************************************/

dom/indexedDB/ActorsChild.h

@@ -419,6 +419,9 @@ private:
   virtual bool
   RecvInvalidate() override;
 
+  virtual bool
+  RecvCloseAfterInvalidationComplete() override;
+
   bool
   SendDeleteMe() = delete;
 };

dom/indexedDB/ActorsParent.cpp

@@ -13811,6 +13811,14 @@ Database::ConnectionClosedCallback()
   mDirectoryLock = nullptr;
 
   CleanupMetadata();
+
+  if (IsInvalidated() && IsActorAlive()) {
+    // Step 3 and 4 of "5.2 Closing a Database":
+    // 1. Wait for all transactions to complete.
+    // 2. Fire a close event if forced flag is set, i.e., IsInvalidated() in our
+    //    implementation.
+    Unused << SendCloseAfterInvalidationComplete();
+  }
 }
 
 void
@@ -14937,7 +14945,7 @@ TransactionBase::Invalidate()
     mInvalidated = true;
     mInvalidatedOnAnyThread = true;
 
-    Abort(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR, /* aForce */ true);
+    Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR, /* aForce */ false);
   }
 }
 
@@ -16186,10 +16194,6 @@ Cursor::Start(const OpenCursorParams& aParams)
       aParams.get_IndexOpenCursorParams().optionalKeyRange() :
       aParams.get_IndexOpenKeyCursorParams().optionalKeyRange();
 
-  if (mTransaction->IsInvalidated()) {
-    return true;
-  }
-
   RefPtr<OpenOp> openOp = new OpenOp(this, optionalKeyRange);
 
   if (NS_WARN_IF(!openOp->Init(mTransaction))) {
@@ -16349,10 +16353,6 @@ Cursor::RecvContinue(const CursorRequestParams& aParams, const Key& aKey)
     return false;
   }
 
-  if (mTransaction->IsInvalidated()) {
-    return true;
-  }
-
   RefPtr<ContinueOp> continueOp = new ContinueOp(this, aParams, aKey);
   if (NS_WARN_IF(!continueOp->Init(mTransaction))) {
     continueOp->Cleanup();
@@ -22744,12 +22744,9 @@ TransactionDatabaseOperationBase::RunOnConnectionThread()
 
   // There are several cases where we don't actually have to to any work here.
 
-  if (mTransactionIsAborted) {
-    // This transaction is already set to be aborted.
+  if (mTransactionIsAborted || mTransaction->IsInvalidatedOnAnyThread()) {
+    // This transaction is already set to be aborted or invalidated.
     mResultCode = NS_ERROR_DOM_INDEXEDDB_ABORT_ERR;
-  } else if (mTransaction->IsInvalidatedOnAnyThread()) {
-    // This transaction is being invalidated.
-    mResultCode = NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
   } else if (!OperationMayProceed()) {
     // The operation was canceled in some way, likely because the child process
     // has crashed.
@@ -22820,9 +22817,7 @@ TransactionDatabaseOperationBase::RunOnOwningThread()
       mResultCode = NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
     }
   } else {
-    if (mTransaction->IsInvalidated()) {
-      mResultCode = NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR;
-    } else if (mTransaction->IsAborted()) {
+    if (mTransaction->IsInvalidated() || mTransaction->IsAborted()) {
       // Aborted transactions always see their requests fail with ABORT_ERR,
       // even if the request succeeded or failed with another error.
       mResultCode = NS_ERROR_DOM_INDEXEDDB_ABORT_ERR;
@@ -26741,6 +26736,14 @@ CursorOpBase::SendFailureResult(nsresult aResultCode)
   if (!IsActorDestroyed()) {
     mResponse = ClampResultCode(aResultCode);
 
+    // This is an expected race when the transaction is invalidated after
+    // data is retrieved from database. We clear the retrieved files to prevent
+    // the assertion failure in SendResponseInternal when mResponse.type() is
+    // CursorResponse::Tnsresult.
+    if (Transaction()->IsInvalidated() && !mFiles.IsEmpty()) {
+      mFiles.Clear();
+    }
+
     mCursor->SendResponseInternal(mResponse, mFiles);
   }
 

dom/indexedDB/IDBDatabase.cpp

@@ -873,7 +873,7 @@ IDBDatabase::AbortTransactions(bool aShouldWarn)
           }
         }
 
-        transaction->Abort(NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
+        transaction->Abort(NS_ERROR_DOM_INDEXEDDB_ABORT_ERR);
       }
 
       static const char kWarningMessage[] =

dom/indexedDB/IDBDatabase.h

@@ -248,6 +248,7 @@ public:
   Storage() const;
 
   IMPL_EVENT_HANDLER(abort)
+  IMPL_EVENT_HANDLER(close)
   IMPL_EVENT_HANDLER(error)
   IMPL_EVENT_HANDLER(versionchange)
 

dom/indexedDB/IDBEvents.cpp

@@ -26,6 +26,7 @@ const char16_t* kErrorEventType = MOZ_UTF16("error");
 const char16_t* kSuccessEventType = MOZ_UTF16("success");
 const char16_t* kUpgradeNeededEventType = MOZ_UTF16("upgradeneeded");
 const char16_t* kVersionChangeEventType = MOZ_UTF16("versionchange");
+const char16_t* kCloseEventType = MOZ_UTF16("close");
 
 already_AddRefed<nsIDOMEvent>
 CreateGenericEvent(EventTarget* aOwner,

dom/indexedDB/IDBEvents.h

@@ -47,6 +47,7 @@ extern const char16_t* kErrorEventType;
 extern const char16_t* kSuccessEventType;
 extern const char16_t* kUpgradeNeededEventType;
 extern const char16_t* kVersionChangeEventType;
+extern const char16_t* kCloseEventType;
 
 already_AddRefed<nsIDOMEvent>
 CreateGenericEvent(EventTarget* aOwner,

dom/indexedDB/IDBTransaction.cpp

@@ -376,7 +376,7 @@ IDBTransaction::OnRequestFinished(bool aActorDestroyedNormally)
 
   --mPendingRequestCount;
 
-  if (!mPendingRequestCount && !mDatabase->IsInvalidated()) {
+  if (!mPendingRequestCount) {
     mReadyState = COMMITTING;
 
     if (aActorDestroyedNormally) {
@@ -641,15 +641,6 @@ IDBTransaction::AbortInternal(nsresult aAbortCode,
   const bool isInvalidated = mDatabase->IsInvalidated();
   bool needToSendAbort = mReadyState == INITIAL && !isInvalidated;
 
-  if (isInvalidated) {
-#ifdef DEBUG
-    mSentCommitOrAbort = true;
-#endif
-    // Increment the serial number counter here to account for the aborted
-    // transaction and keep the parent in sync.
-    IDBRequest::NextSerialNumber();
-  }
-
   mAbortCode = aAbortCode;
   mReadyState = DONE;
   mError = error.forget();

dom/indexedDB/PBackgroundIDBDatabase.ipdl

@@ -72,6 +72,8 @@ child:
 
   async Invalidate();
 
+  async CloseAfterInvalidationComplete();
+
   async PBackgroundIDBVersionChangeTransaction(uint64_t currentVersion,
                                                uint64_t requestedVersion,
                                                int64_t nextObjectStoreId,

dom/indexedDB/test/helpers.js

@@ -157,6 +157,12 @@ function testHarnessSteps() {
           worker._expectingUncaughtException = message.expecting;
           break;
 
+        case "clearAllDatabases":
+          clearAllDatabases(function(){
+            worker.postMessage({ op: "clearAllDatabasesDone" });
+          });
+          break;
+
         default:
           ok(false,
              "Received a bad message from worker: " + JSON.stringify(message));
@@ -511,6 +517,12 @@ function workerScript() {
     self.postMessage({ op: "expectUncaughtException", expecting: !!_expecting_ });
   };
 
+  self._clearAllDatabasesCallback = undefined;
+  self.clearAllDatabases = function(_callback_) {
+    self._clearAllDatabasesCallback = _callback_;
+    self.postMessage({ op: "clearAllDatabases" });
+  }
+
   self.onerror = function(_message_, _file_, _line_) {
     if (self._expectingUncaughtException) {
       self._expectingUncaughtException = false;
@@ -542,6 +554,13 @@ function workerScript() {
         });
         break;
 
+      case "clearAllDatabasesDone":
+        info("Worker: all databases are cleared");
+        if (self._clearAllDatabasesCallback) {
+          self._clearAllDatabasesCallback();
+        }
+        break;
+
       default:
         throw new Error("Received a bad message from parent: " +
                         JSON.stringify(message));

dom/indexedDB/test/mochitest.ini

@@ -36,6 +36,7 @@ support-files =
   unit/test_cursor_mutation.js
   unit/test_cursor_update_updates_indexes.js
   unit/test_cursors.js
+  unit/test_database_onclose.js
   unit/test_deleteDatabase.js
   unit/test_deleteDatabase_interactions.js
   unit/test_deleteDatabase_onblocked.js
@@ -169,6 +170,8 @@ skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_cursors.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
+[test_database_onclose.html]
+skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_deleteDatabase.html]
 skip-if = (buildapp == 'b2g' && toolkit != 'gonk') # Bug 931116
 [test_deleteDatabase_interactions.html]

dom/indexedDB/test/test_database_onclose.html

@@ -0,0 +1,19 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database DeleteDatabase Test</title>
+
+  <script type="text/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script type="text/javascript;version=1.7" src="unit/test_database_onclose.js"></script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+
+</head>
+
+<body onload="runTest();"></body>
+
+</html>

dom/indexedDB/test/unit/test_database_close_without_onclose.js

@@ -0,0 +1,49 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+var testGenerator = testSteps();
+
+function testSteps()
+{
+  const name = this.window ? window.location.pathname :
+                             "test_database_close_without_onclose.js";
+
+  const checkpointSleepTimeSec = 10;
+
+  let openRequest = indexedDB.open(name, 1);
+  openRequest.onerror = errorHandler;
+  openRequest.onsuccess = unexpectedSuccessHandler;
+  openRequest.onupgradeneeded = grabEventAndContinueHandler;
+
+  ok(openRequest instanceof IDBOpenDBRequest, "Expect an IDBOpenDBRequest");
+
+  let event = yield undefined;
+
+  is(event.type, "upgradeneeded", "Expect an upgradeneeded event");
+  ok(event instanceof IDBVersionChangeEvent, "Expect a versionchange event");
+
+  let db = event.target.result;
+  db.createObjectStore("store");
+
+  openRequest.onsuccess = grabEventAndContinueHandler;
+
+  event = yield undefined;
+
+  is(event.type, "success", "Expect a success event");
+  is(event.target, openRequest, "Event has right target");
+  ok(event.target.result instanceof IDBDatabase, "Result should be a database");
+  is(db.objectStoreNames.length, 1, "Expect an objectStore here");
+
+  db.onclose = errorHandler;
+
+  db.close();
+  setTimeout(continueToNextStepSync, checkpointSleepTimeSec * 1000);
+  yield undefined;
+
+  ok(true, "The close event should not be fired after closed normally!");
+
+  finishTest();
+  yield undefined;
+}

dom/indexedDB/test/unit/test_database_onclose.js

@@ -0,0 +1,245 @@
+/**
+ * Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/
+ */
+
+var testGenerator = testSteps();
+
+function testSteps()
+{
+  function testInvalidStateError(aDb, aTxn) {
+    try {
+      info("The db shall become invalid after closed.");
+      aDb.transaction("store");
+      ok(false, "InvalidStateError shall be thrown.");
+    } catch (e) {
+      ok(e instanceof DOMException, "got a database exception");
+      is(e.name, "InvalidStateError", "correct error");
+    }
+
+    try {
+      info("The txn shall become invalid after closed.");
+      aTxn.objectStore("store");
+      ok(false, "InvalidStateError shall be thrown.");
+    } catch (e) {
+      ok(e instanceof DOMException, "got a database exception");
+      is(e.name, "InvalidStateError", "correct error");
+    }
+  }
+
+  const name = this.window ? window.location.pathname :
+                             "test_database_onclose.js";
+
+  info("#1: Verifying IDBDatabase.onclose after cleared by the agent.");
+  let openRequest = indexedDB.open(name, 1);
+  openRequest.onerror = errorHandler;
+  openRequest.onsuccess = unexpectedSuccessHandler;
+  openRequest.onupgradeneeded = grabEventAndContinueHandler;
+
+  ok(openRequest instanceof IDBOpenDBRequest, "Expect an IDBOpenDBRequest");
+
+  let event = yield undefined;
+
+  is(event.type, "upgradeneeded", "Expect an upgradeneeded event");
+  ok(event instanceof IDBVersionChangeEvent, "Expect a versionchange event");
+
+  let db = event.target.result;
+  db.createObjectStore("store");
+
+  openRequest.onsuccess = grabEventAndContinueHandler;
+
+  event = yield undefined;
+
+  is(event.type, "success", "Expect a success event");
+  is(event.target, openRequest, "Event has right target");
+  ok(event.target.result instanceof IDBDatabase, "Result should be a database");
+  is(db.objectStoreNames.length, 1, "Expect an objectStore here");
+
+  let txn = db.transaction("store", "readwrite");
+  let objectStore = txn.objectStore("store");
+
+  clearAllDatabases(continueToNextStep);
+
+  db.onclose = grabEventAndContinueHandler;
+  event = yield undefined;
+  is(event.type, "close", "Expect a close event");
+  is(event.target, db, "Correct target");
+
+  info("Wait for callback of clearAllDatabases().");
+  yield undefined;
+
+  testInvalidStateError(db, txn);
+
+  info("#2: Verifying IDBDatabase.onclose && IDBTransaction.onerror " +
+  		 "in *write* operation after cleared by the agent.");
+  openRequest = indexedDB.open(name, 1);
+  openRequest.onerror = errorHandler;
+  openRequest.onsuccess = unexpectedSuccessHandler;
+  openRequest.onupgradeneeded = grabEventAndContinueHandler;
+
+  ok(openRequest instanceof IDBOpenDBRequest, "Expect an IDBOpenDBRequest");
+
+  event = yield undefined;
+
+  is(event.type, "upgradeneeded", "Expect an upgradeneeded event");
+  ok(event instanceof IDBVersionChangeEvent, "Expect a versionchange event");
+
+  db = event.target.result;
+  db.createObjectStore("store");
+
+  openRequest.onsuccess = grabEventAndContinueHandler;
+
+  event = yield undefined;
+
+  is(event.type, "success", "Expect a success event");
+  is(event.target, openRequest, "Event has right target");
+  ok(event.target.result instanceof IDBDatabase, "Result should be a database");
+  is(db.objectStoreNames.length, 1, "Expect an objectStore here");
+
+  txn = db.transaction("store", "readwrite");
+  objectStore = txn.objectStore("store");
+
+  let objectId = 0;
+  while(true) {
+    let addRequest = objectStore.add({foo: "foo"}, objectId);
+    addRequest.onerror = function(event) {
+      info("addRequest.onerror, objectId: " + objectId);
+      txn.onerror = grabEventAndContinueHandler;
+      testGenerator.send(true);
+    }
+    addRequest.onsuccess = function() {
+      testGenerator.send(false);
+    }
+
+    if (objectId == 0) {
+      clearAllDatabases(() => {
+        info("clearAllDatabases is done.");
+        continueToNextStep();
+      });
+    }
+
+    objectId++;
+
+    let aborted = yield undefined;
+    if (aborted) {
+      break;
+    }
+  }
+
+  event = yield undefined;
+  is(event.type, "error", "Got an error event");
+  is(event.target.error.name, "AbortError", "Expected AbortError was thrown.");
+  event.preventDefault();
+
+  txn.onabort = grabEventAndContinueHandler;
+  event = yield undefined;
+  is(event.type, "abort", "Got an abort event");
+  is(event.target.error.name, "AbortError", "Expected AbortError was thrown.");
+
+  db.onclose = grabEventAndContinueHandler;
+  event = yield undefined;
+  is(event.type, "close", "Expect a close event");
+  is(event.target, db, "Correct target");
+  testInvalidStateError(db, txn);
+
+  info("Wait for the callback of clearAllDatabases().");
+  yield undefined;
+
+  info("#3: Verifying IDBDatabase.onclose && IDBTransaction.onerror " +
+  "in *read* operation after cleared by the agent.");
+  openRequest = indexedDB.open(name, 1);
+  openRequest.onerror = errorHandler;
+  openRequest.onsuccess = unexpectedSuccessHandler;
+  openRequest.onupgradeneeded = grabEventAndContinueHandler;
+
+  ok(openRequest instanceof IDBOpenDBRequest, "Expect an IDBOpenDBRequest");
+
+  event = yield undefined;
+
+  is(event.type, "upgradeneeded", "Expect an upgradeneeded event");
+  ok(event instanceof IDBVersionChangeEvent, "Expect a versionchange event");
+
+  db = event.target.result;
+  objectStore =
+    db.createObjectStore("store", { keyPath: "id", autoIncrement: true });
+  // The number of read records varies between 1~2000 before the db is cleared
+  // during testing.
+  let numberOfObjects = 3000;
+  objectId = 0;
+  while(true) {
+    let addRequest = objectStore.add({foo: "foo"});
+    addRequest.onsuccess = function() {
+      objectId++;
+      testGenerator.send(objectId == numberOfObjects);
+    }
+    addRequest.onerror = errorHandler;
+
+    let done = yield undefined;
+    if (done) {
+      break;
+    }
+  }
+
+  openRequest.onsuccess = grabEventAndContinueHandler;
+
+  event = yield undefined;
+
+  is(event.type, "success", "Expect a success event");
+  is(event.target, openRequest, "Event has right target");
+  ok(event.target.result instanceof IDBDatabase, "Result should be a database");
+  is(db.objectStoreNames.length, 1, "Expect an objectStore here");
+
+  txn = db.transaction("store");
+  objectStore = txn.objectStore("store");
+
+  let numberOfReadObjects = 0;
+  let readRequest = objectStore.openCursor();
+  readRequest.onerror = function(event) {
+    info("readRequest.onerror, numberOfReadObjects: " + numberOfReadObjects);
+    testGenerator.send(true);
+  }
+  readRequest.onsuccess = function(event) {
+    let cursor = event.target.result;
+    if (cursor) {
+      numberOfReadObjects++;
+      event.target.result.continue();
+    } else {
+      info("Cursor is invalid, numberOfReadObjects: " + numberOfReadObjects);
+      todo(false, "All records are iterated before database is cleared!");
+      testGenerator.send(false);
+    }
+  }
+
+  clearAllDatabases(() => {
+    info("clearAllDatabases is done.");
+    continueToNextStep();
+  });
+
+  readRequestError = yield undefined;
+  if (readRequestError) {
+    txn.onerror = grabEventAndContinueHandler;
+
+    event = yield undefined;
+    is(event.type, "error", "Got an error event");
+    is(event.target.error.name, "AbortError", "Expected AbortError was thrown.");
+    event.preventDefault();
+
+    txn.onabort = grabEventAndContinueHandler;
+    event = yield undefined;
+    is(event.type, "abort", "Got an abort event");
+    is(event.target.error.name, "AbortError", "Expected AbortError was thrown.");
+
+    db.onclose = grabEventAndContinueHandler;
+    event = yield undefined;
+    is(event.type, "close", "Expect a close event");
+    is(event.target, db, "Correct target");
+
+    testInvalidStateError(db, txn);
+  }
+
+  info("Wait for the callback of clearAllDatabases().");
+  yield undefined;
+
+  finishTest();
+  yield undefined;
+}

dom/indexedDB/test/unit/xpcshell-parent-process.ini

@@ -32,6 +32,8 @@ support-files =
 [test_blob_file_backed.js]
 [test_bug1056939.js]
 [test_cleanup_transaction.js]
+[test_database_close_without_onclose.js]
+[test_database_onclose.js]
 [test_defaultStorageUpgrade.js]
 [test_idbSubdirUpgrade.js]
 [test_globalObjects_ipc.js]

dom/webidl/IDBDatabase.webidl

@@ -30,6 +30,7 @@ interface IDBDatabase : EventTarget {
     void           close ();
 
                 attribute EventHandler       onabort;
+                attribute EventHandler       onclose;
                 attribute EventHandler       onerror;
                 attribute EventHandler       onversionchange;
 };

testing/web-platform/tests/IndexedDB/interfaces.idl

@@ -83,6 +83,7 @@ interface IDBDatabase : EventTarget {
     IDBTransaction transaction ((DOMString or sequence<DOMString>) storeNames, optional IDBTransactionMode mode = "readonly");
     void           close ();
                 attribute EventHandler       onabort;
+                attribute EventHandler       onclose;
                 attribute EventHandler       onerror;
                 attribute EventHandler       onversionchange;
 };