Bug 965084 - "ROLLBACK TO SAVEPOINT savepoint" doesn't fire appropriate triggers leaving orphaned files. r=bent

dom/indexedDB/IDBTransaction.cpp

@@ -298,6 +298,10 @@ IDBTransaction::StartSavepoint()
   nsresult rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, false);
 
+  if (IsWriteAllowed()) {
+    mUpdateFileRefcountFunction->StartSavepoint();
+  }
+
   ++mSavepointCount;
 
   return true;
@@ -321,6 +325,10 @@ IDBTransaction::ReleaseSavepoint()
   nsresult rv = stmt->Execute();
   NS_ENSURE_SUCCESS(rv, NS_OK);
 
+  if (IsWriteAllowed()) {
+    mUpdateFileRefcountFunction->ReleaseSavepoint();
+  }
+
   --mSavepointCount;
 
   return NS_OK;
@@ -344,6 +352,10 @@ IDBTransaction::RollbackSavepoint()
 
   nsresult rv = stmt->Execute();
   NS_ENSURE_SUCCESS_VOID(rv);
+
+  if (IsWriteAllowed()) {
+    mUpdateFileRefcountFunction->RollbackSavepoint();
+  }
 }
 
 nsresult
@@ -1078,12 +1090,22 @@ UpdateRefcountFunction::ProcessValue(mozIStorageValueArray* aValues,
       entry = newEntry.forget();
     }
 
+    if (mInSavepoint) {
+      mSavepointEntriesIndex.Put(id, entry);
+    }
+
     switch (aUpdateType) {
       case eIncrement:
         entry->mDelta++;
+        if (mInSavepoint) {
+          entry->mSavepointDelta++;
+        }
         break;
       case eDecrement:
         entry->mDelta--;
+        if (mInSavepoint) {
+          entry->mSavepointDelta--;
+        }
         break;
       default:
         NS_NOTREACHED("Unknown update type!");
@@ -1165,6 +1187,16 @@ UpdateRefcountFunction::FileInfoUpdateCallback(const uint64_t& aKey,
   return PL_DHASH_NEXT;
 }
 
+PLDHashOperator
+UpdateRefcountFunction::RollbackSavepointCallback(const uint64_t& aKey,
+                                                  FileInfoEntry* aValue,
+                                                  void* aUserArg)
+{
+  aValue->mDelta -= aValue->mSavepointDelta;
+
+  return PL_DHASH_NEXT;
+}
+
 bool
 UpdateRefcountFunction::DatabaseUpdateFunction::Update(int64_t aId,
                                                        int32_t aDelta)

dom/indexedDB/IDBTransaction.h

@@ -367,12 +367,40 @@ public:
   NS_DECL_MOZISTORAGEFUNCTION
 
   UpdateRefcountFunction(FileManager* aFileManager)
-  : mFileManager(aFileManager)
+  : mFileManager(aFileManager), mInSavepoint(false)
   { }
 
   ~UpdateRefcountFunction()
   { }
 
+  void StartSavepoint()
+  {
+    MOZ_ASSERT(!mInSavepoint);
+    MOZ_ASSERT(!mSavepointEntriesIndex.Count());
+
+    mInSavepoint = true;
+  }
+
+  void ReleaseSavepoint()
+  {
+    MOZ_ASSERT(mInSavepoint);
+
+    mSavepointEntriesIndex.Clear();
+
+    mInSavepoint = false;
+  }
+
+  void RollbackSavepoint()
+  {
+    MOZ_ASSERT(mInSavepoint);
+
+    mInSavepoint = false;
+
+    mSavepointEntriesIndex.EnumerateRead(RollbackSavepointCallback, nullptr);
+
+    mSavepointEntriesIndex.Clear();
+  }
+
   void ClearFileInfoEntries()
   {
     mFileInfoEntries.Clear();
@@ -387,7 +415,7 @@ private:
   {
   public:
     FileInfoEntry(FileInfo* aFileInfo)
-    : mFileInfo(aFileInfo), mDelta(0)
+    : mFileInfo(aFileInfo), mDelta(0), mSavepointDelta(0)
     { }
 
     ~FileInfoEntry()
@@ -395,6 +423,7 @@ private:
 
     nsRefPtr<FileInfo> mFileInfo;
     int32_t mDelta;
+    int32_t mSavepointDelta;
   };
 
   enum UpdateType {
@@ -447,12 +476,20 @@ private:
                          FileInfoEntry* aValue,
                          void* aUserArg);
 
+  static PLDHashOperator
+  RollbackSavepointCallback(const uint64_t& aKey,
+                            FileInfoEntry* aValue,
+                            void* aUserArg);
+
   FileManager* mFileManager;
   nsClassHashtable<nsUint64HashKey, FileInfoEntry> mFileInfoEntries;
+  nsDataHashtable<nsUint64HashKey, FileInfoEntry*> mSavepointEntriesIndex;
 
   nsTArray<int64_t> mJournalsToCreateBeforeCommit;
   nsTArray<int64_t> mJournalsToRemoveAfterCommit;
   nsTArray<int64_t> mJournalsToRemoveAfterAbort;
+
+  bool mInSavepoint;
 };
 
 END_INDEXEDDB_NAMESPACE

dom/indexedDB/test/test_file_os_delete.html

@@ -21,7 +21,14 @@
     getUsage(grabFileUsageAndContinueHandler);
     let startUsage = yield undefined;
 
-    const fileData = { key: 1, file: getRandomFile("random.bin", 100000) };
+    const fileData1 = {
+      key: 1,
+      obj: { id: 1, file: getRandomFile("random.bin", 100000) }
+    };
+    const fileData2 = {
+      key: 2,
+      obj: { id: 1, file: getRandomFile("random.bin", 100000) }
+    };
 
     {
       let request = indexedDB.open(name, 1);
@@ -37,7 +44,14 @@
 
       let objectStore = db.createObjectStore(objectStoreName, { });
 
-      objectStore.add(fileData.file, fileData.key);
+      objectStore.createIndex("index", "id", { unique: true });
+
+      objectStore.add(fileData1.obj, fileData1.key);
+
+      request = objectStore.add(fileData2.obj, fileData2.key);
+      request.addEventListener("error", new ExpectError("ConstraintError", true));
+      request.onsuccess = unexpectedSuccessHandler;
+      yield undefined;
 
       event = yield undefined;
 
@@ -46,10 +60,11 @@
       getUsage(grabFileUsageAndContinueHandler);
       let usage = yield undefined;
 
-      is(usage, startUsage + fileData.file.size, "Correct file usage");
+      is(usage, startUsage + fileData1.obj.file.size + fileData2.obj.file.size,
+         "Correct file usage");
 
       let trans = db.transaction([objectStoreName], READ_WRITE);
-      trans.objectStore(objectStoreName).delete(fileData.key);
+      trans.objectStore(objectStoreName).delete(fileData1.key);
       trans.oncomplete = grabEventAndContinueHandler;
       event = yield undefined;
 
@@ -58,9 +73,11 @@
       getUsage(grabFileUsageAndContinueHandler);
       usage = yield undefined;
 
-      is(usage, startUsage + fileData.file.size, "OS file exists");
+      is(usage, startUsage + fileData1.obj.file.size + fileData2.obj.file.size,
+         "OS files exists");
 
-      fileData.file = null;
+      fileData1.obj.file = null;
+      fileData2.obj.file = null;
     }
 
     scheduleGC();
@@ -69,7 +86,7 @@
     getUsage(grabFileUsageAndContinueHandler);
     let endUsage = yield undefined;
 
-    is(endUsage, startUsage, "OS file deleted");
+    is(endUsage, startUsage, "OS files deleted");
 
     finishTest();
     yield undefined;