Bug 711727 - Followup cleanup for files in IndexedDB. r=sicking/bent

content/base/public/nsDOMFile.h

@@ -134,10 +134,6 @@ protected:
     return false;
   }
 
-  indexedDB::FileInfo*
-  GetFileInfoInternal(indexedDB::FileManager* aFileManager,
-                      PRUint32 aStartIndex);
-
   bool mIsFile;
   bool mImmutable;
   nsString mContentType;

db/sqlite3/src/sqlite.def

@@ -181,7 +181,6 @@ EXPORTS
         sqlite3_vfs_unregister
         sqlite3_vfs_register
         sqlite3_vmprintf
-        sqlite3_win32_utf8_to_mbcs
 #ifdef SQLITE_DEBUG
         sqlite3_mutex_held
         sqlite3_mutex_notheld

dom/base/nsDOMWindowUtils.cpp

@@ -1954,19 +1954,27 @@ nsDOMWindowUtils::GetFileReferences(const nsAString& aDatabaseName,
   NS_ENSURE_SUCCESS(rv, rv);
 
   nsRefPtr<indexedDB::IndexedDatabaseManager> mgr =
-    indexedDB::IndexedDatabaseManager::GetOrCreate();
-  NS_ENSURE_TRUE(mgr, NS_ERROR_FAILURE);
+    indexedDB::IndexedDatabaseManager::Get();
 
-  nsRefPtr<indexedDB::FileManager> fileManager =
-    mgr->GetOrCreateFileManager(origin, aDatabaseName);
-  NS_ENSURE_TRUE(fileManager, NS_ERROR_FAILURE);
+  if (mgr) {
+    nsRefPtr<indexedDB::FileManager> fileManager =
+      mgr->GetFileManager(origin, aDatabaseName);
 
-  nsRefPtr<indexedDB::FileInfo> fileInfo = fileManager->GetFileInfo(aId);
-  if (fileInfo) {
-    fileInfo->GetReferences(aRefCnt, aDBRefCnt, aSliceRefCnt);
-    *aRefCnt--;
-    *aResult = true;
-    return NS_OK;
+    if (fileManager) {
+      nsRefPtr<indexedDB::FileInfo> fileInfo = fileManager->GetFileInfo(aId);
+
+      if (fileInfo) {
+        fileInfo->GetReferences(aRefCnt, aDBRefCnt, aSliceRefCnt);
+
+        if (*aRefCnt != -1) {
+          // We added an extra temp ref, so account for that accordingly.
+          (*aRefCnt)--;
+        }
+
+        *aResult = true;
+        return NS_OK;
+      }
+    }
   }
 
   *aRefCnt = *aDBRefCnt = *aSliceRefCnt = -1;

dom/indexedDB/FileInfo.h

@@ -131,7 +131,7 @@ private:
 class FileInfo##_bits : public FileInfo                                       \
 {                                                                             \
 public:                                                                       \
-  FileInfo##_bits(FileManager* aFileManager, PRInt64 aId)                     \
+  FileInfo##_bits(FileManager* aFileManager, PRInt##_bits aId)                \
   : FileInfo(aFileManager), mId(aId)                                          \
   { }                                                                         \
                                                                               \

dom/indexedDB/FileManager.cpp

@@ -71,21 +71,13 @@ EnumerateToTArray(const PRUint64& aKey,
 } // anonymous namespace
 
 nsresult
-FileManager::Init()
+FileManager::Init(nsIFile* aDirectory,
+                  mozIStorageConnection* aConnection)
 {
   NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
 
   NS_ENSURE_TRUE(mFileInfos.Init(), NS_ERROR_OUT_OF_MEMORY);
 
-  return NS_OK;
-}
-
-nsresult
-FileManager::InitDirectory(nsIFile* aDirectory,
-                           mozIStorageConnection* aConnection)
-{
-  NS_ASSERTION(!NS_IsMainThread(), "Wrong thread!");
-
   bool exists;
   nsresult rv = aDirectory->Exists(&exists);
   NS_ENSURE_SUCCESS(rv, rv);
@@ -142,7 +134,8 @@ FileManager::InitDirectory(nsIFile* aDirectory,
     if (flag) {
       rv = ss->UpdateQuotaInformationForFile(file);
       NS_ENSURE_SUCCESS(rv, rv);
-    } else {
+    }
+    else {
       rv = file->Remove(false);
       if (NS_FAILED(rv)) {
         NS_WARNING("Failed to remove orphaned file!");
@@ -158,9 +151,6 @@ FileManager::InitDirectory(nsIFile* aDirectory,
   rv = aDirectory->GetPath(mDirectoryPath);
   NS_ENSURE_SUCCESS(rv, rv);
 
-  rv = aDirectory->GetLeafName(mDirectoryName);
-  NS_ENSURE_SUCCESS(rv, rv);
-
   return NS_OK;
 }
 

dom/indexedDB/FileManager.h

@@ -77,12 +77,7 @@ public:
     return mDatabaseName;
   }
 
-  const nsAString& DirectoryName() const
-  {
-    return mDirectoryName;
-  }
-
-  bool IsDirectoryInited() const
+  bool Inited() const
   {
     return !mDirectoryPath.IsEmpty();
   }
@@ -97,10 +92,8 @@ public:
     return mInvalidated;
   }
 
-  nsresult Init();
-
-  nsresult InitDirectory(nsIFile* aDirectory,
-                         mozIStorageConnection* aConnection);
+  nsresult Init(nsIFile* aDirectory,
+                mozIStorageConnection* aConnection);
 
   nsresult Load(mozIStorageConnection* aConnection);
 
@@ -120,7 +113,6 @@ private:
   nsString mDatabaseName;
 
   nsString mDirectoryPath;
-  nsString mDirectoryName;
 
   PRInt64 mLastFileId;
 

dom/indexedDB/IDBObjectStore.cpp

@@ -2152,7 +2152,8 @@ AddHelper::DoDatabaseWork(mozIStorageConnection* aConnection)
 
   if (fileIds.IsEmpty()) {
     rv = stmt->BindNullByName(NS_LITERAL_CSTRING("file_ids"));
-  } else {
+  }
+  else {
     rv = stmt->BindStringByName(NS_LITERAL_CSTRING("file_ids"), fileIds);
   }
   NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);

dom/indexedDB/IndexedDatabaseManager.cpp

@@ -684,12 +684,15 @@ IndexedDatabaseManager::EnsureOriginIsInitialized(const nsACString& aOrigin,
   // are database files then we need to create file managers for them and also
   // tell SQLite about all of them.
 
-  nsAutoTArray<nsString, 20> subdirectories;
+  nsAutoTArray<nsString, 20> subdirsToProcess;
   nsAutoTArray<nsCOMPtr<nsIFile> , 20> unknownFiles;
 
   nsAutoPtr<nsTArray<nsRefPtr<FileManager> > > fileManagers(
     new nsTArray<nsRefPtr<FileManager> >());
 
+  nsTHashtable<nsStringHashKey> validSubdirs;
+  NS_ENSURE_TRUE(validSubdirs.Init(20), NS_ERROR_OUT_OF_MEMORY);
+  
   nsCOMPtr<nsISimpleEnumerator> entries;
   rv = directory->GetDirectoryEntries(getter_AddRefs(entries));
   NS_ENSURE_SUCCESS(rv, rv);
@@ -712,7 +715,9 @@ IndexedDatabaseManager::EnsureOriginIsInitialized(const nsACString& aOrigin,
     NS_ENSURE_SUCCESS(rv, rv);
 
     if (isDirectory) {
-      subdirectories.AppendElement(leafName);
+      if (!validSubdirs.GetEntry(leafName)) {
+        subdirsToProcess.AppendElement(leafName);
+      }
       continue;
     }
 
@@ -759,30 +764,24 @@ IndexedDatabaseManager::EnsureOriginIsInitialized(const nsACString& aOrigin,
 
     nsRefPtr<FileManager> fileManager = new FileManager(aOrigin, databaseName);
 
-    rv = fileManager->Init();
-    NS_ENSURE_SUCCESS(rv, rv);
-
-    rv = fileManager->InitDirectory(fileManagerDirectory, connection);
+    rv = fileManager->Init(fileManagerDirectory, connection);
     NS_ENSURE_SUCCESS(rv, rv);
 
     fileManagers->AppendElement(fileManager);
 
     rv = ss->UpdateQuotaInformationForFile(file);
     NS_ENSURE_SUCCESS(rv, rv);
+
+    if (!validSubdirs.PutEntry(dbBaseFilename)) {
+      NS_WARNING("Out of memory?");
+      return NS_ERROR_OUT_OF_MEMORY;
+    }
   }
   NS_ENSURE_SUCCESS(rv, rv);
 
-  for (PRUint32 i = 0; i < subdirectories.Length(); i++) {
-    const nsString& subdirectory = subdirectories[i];
-    bool unknown = true;
-    for (PRUint32 j = 0; j < fileManagers->Length(); j++) {
-      nsRefPtr<FileManager>& fileManager = fileManagers->ElementAt(j);
-      if (fileManager->DirectoryName().Equals(subdirectory)) {
-        unknown = false;
-        break;
-      }
-    }
-    if (unknown) {
+  for (PRUint32 i = 0; i < subdirsToProcess.Length(); i++) {
+    const nsString& subdir = subdirsToProcess[i];
+    if (!validSubdirs.GetEntry(subdir)) {
       NS_WARNING("Unknown subdirectory found!");
       return NS_ERROR_UNEXPECTED;
     }
@@ -791,6 +790,8 @@ IndexedDatabaseManager::EnsureOriginIsInitialized(const nsACString& aOrigin,
   for (PRUint32 i = 0; i < unknownFiles.Length(); i++) {
     nsCOMPtr<nsIFile>& unknownFile = unknownFiles[i];
 
+    // Some temporary SQLite files could disappear, so we have to check if the
+    // unknown file still exists.
     bool exists;
     rv = unknownFile->Exists(&exists);
     NS_ENSURE_SUCCESS(rv, rv);
@@ -799,6 +800,7 @@ IndexedDatabaseManager::EnsureOriginIsInitialized(const nsACString& aOrigin,
       nsString leafName;
       unknownFile->GetLeafName(leafName);
 
+      // The journal file may exists even after db has been correctly opened.
       if (!StringEndsWith(leafName, NS_LITERAL_STRING(".sqlite-journal"))) {
         NS_WARNING("Unknown file found!");
         return NS_ERROR_UNEXPECTED;
@@ -941,17 +943,33 @@ IndexedDatabaseManager::GetOrCreateFileManager(const nsACString& aOrigin,
   if (!fileManager) {
     fileManager = new FileManager(aOrigin, aDatabaseName);
 
-    if (NS_FAILED(fileManager->Init())) {
-      NS_WARNING("Failed to initialize file manager!");
-      return nsnull;
-    }
-
     array->AppendElement(fileManager);
   }
 
   return fileManager.forget();
 }
 
+already_AddRefed<FileManager>
+IndexedDatabaseManager::GetFileManager(const nsACString& aOrigin,
+                                       const nsAString& aDatabaseName)
+{
+  nsTArray<nsRefPtr<FileManager> >* array;
+  if (!mFileManagers.Get(aOrigin, &array)) {
+    return nsnull;
+  }
+
+  for (PRUint32 i = 0; i < array->Length(); i++) {
+    nsRefPtr<FileManager>& fileManager = array->ElementAt(i);
+
+    if (fileManager->DatabaseName().Equals(aDatabaseName)) {
+      nsRefPtr<FileManager> result = fileManager;
+      return result.forget();
+    }
+  }
+  
+  return nsnull;
+}
+
 void
 IndexedDatabaseManager::InvalidateFileManagersForOrigin(
                                                      const nsACString& aOrigin)
@@ -1354,7 +1372,8 @@ IncrementUsage(PRUint64* aUsage, PRUint64 aDelta)
   if ((LL_MAXINT - *aUsage) <= aDelta) {
     NS_WARNING("Database sizes exceed max we can report!");
     *aUsage = LL_MAXINT;
-  } else {
+  }
+  else {
     *aUsage += aDelta;
   }
 }
@@ -1439,7 +1458,8 @@ IndexedDatabaseManager::AsyncUsageRunnable::GetUsageForDirectory(
     if (isDirectory) {
       if (aUsage == &mFileUsage) {
         NS_WARNING("Unknown directory found!");
-      } else {
+      }
+      else {
         rv = GetUsageForDirectory(file, &mFileUsage);
         NS_ENSURE_SUCCESS(rv, rv);
       }

dom/indexedDB/IndexedDatabaseManager.h

@@ -181,6 +181,10 @@ public:
   GetOrCreateFileManager(const nsACString& aOrigin,
                          const nsAString& aDatabaseName);
 
+  already_AddRefed<FileManager>
+  GetFileManager(const nsACString& aOrigin,
+                 const nsAString& aDatabaseName);
+
   void InvalidateFileManagersForOrigin(const nsACString& aOrigin);
 
   void InvalidateFileManager(const nsACString& aOrigin,

dom/indexedDB/OpenDatabaseHelper.cpp

@@ -1019,6 +1019,11 @@ UpgradeSchemaFrom10_0To11_0(mozIStorageConnection* aConnection)
   ));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "DROP TRIGGER object_data_insert_trigger;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "INSERT INTO object_data (object_store_id, key_value, data, file_ids) "
       "SELECT object_store_id, id, data, file_ids "
@@ -1026,6 +1031,17 @@ UpgradeSchemaFrom10_0To11_0(mozIStorageConnection* aConnection)
   ));
   NS_ENSURE_SUCCESS(rv, rv);
 
+  rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
+    "CREATE TRIGGER object_data_insert_trigger "
+    "AFTER INSERT ON object_data "
+    "FOR EACH ROW "
+    "WHEN NEW.file_ids IS NOT NULL "
+    "BEGIN "
+      "SELECT update_refcount(NULL, NEW.file_ids); "
+    "END;"
+  ));
+  NS_ENSURE_SUCCESS(rv, rv);
+
   rv = aConnection->ExecuteSimpleSQL(NS_LITERAL_CSTRING(
     "INSERT INTO index_data (index_id, value, object_data_key, object_data_id) "
       "SELECT ai_index_data.index_id, ai_index_data.value, ai_index_data.ai_object_data_id, object_data.id "
@@ -1680,8 +1696,8 @@ OpenDatabaseHelper::DoDatabaseWork()
   mFileManager = mgr->GetOrCreateFileManager(mASCIIOrigin, mName);
   NS_ENSURE_TRUE(mFileManager, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
 
-  if (!mFileManager->IsDirectoryInited()) {
-    rv = mFileManager->InitDirectory(fileManagerDirectory, connection);
+  if (!mFileManager->Inited()) {
+    rv = mFileManager->Init(fileManagerDirectory, connection);
     NS_ENSURE_SUCCESS(rv, NS_ERROR_DOM_INDEXEDDB_UNKNOWN_ERR);
   }
 

dom/indexedDB/test/Makefile.in

@@ -78,6 +78,7 @@ TEST_FILES = \
   test_file_cross_database_copying.html \
   test_file_delete.html \
   test_file_os_delete.html \
+  test_file_put_get_object.html \
   test_file_put_get_values.html \
   test_file_resurrection_delete.html \
   test_file_resurrection_transaction_abort.html \

dom/indexedDB/test/file.js

@@ -111,7 +111,8 @@ function verifyBlob(blob1, blob2, fileId, blobReadHandler)
         verifyBuffers(buffer1, buffer2);
         if (blobReadHandler) {
           blobReadHandler();
-        } else {
+        }
+        else {
           testGenerator.next();
         }
       }
@@ -126,7 +127,8 @@ function verifyBlob(blob1, blob2, fileId, blobReadHandler)
       verifyBuffers(buffer1, buffer2);
       if (blobReadHandler) {
         blobReadHandler();
-      } else {
+      }
+      else {
         testGenerator.next();
       }
     }
@@ -147,7 +149,8 @@ function verifyBlobArray(blobs1, blobs2, expectedFileIds)
   function blobReadHandler() {
     if (++verifiedCount == blobs1.length) {
       testGenerator.next();
-    } else {
+    }
+    else {
       verifyBlob(blobs1[verifiedCount], blobs2[verifiedCount],
                  expectedFileIds[verifiedCount], blobReadHandler);
     }

dom/indexedDB/test/helpers.js

@@ -124,14 +124,16 @@ function addPermission(type, allow, url)
     uri = Components.classes["@mozilla.org/network/io-service;1"]
                     .getService(Components.interfaces.nsIIOService)
                     .newURI(url, null, null);
-  } else {
+  }
+  else {
     uri = SpecialPowers.getDocumentURIObject(window.document);
   }
 
   let permission;
   if (allow) {
     permission = Components.interfaces.nsIPermissionManager.ALLOW_ACTION;
-  } else {
+  }
+  else {
     permission = Components.interfaces.nsIPermissionManager.DENY_ACTION;
   }
 

dom/indexedDB/test/test_file_put_get_object.html

@@ -0,0 +1,87 @@
+<!--
+  Any copyright is dedicated to the Public Domain.
+  http://creativecommons.org/publicdomain/zero/1.0/
+-->
+<html>
+<head>
+  <title>Indexed Database Property 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">
+  function testSteps()
+  {
+    const READ_WRITE = IDBTransaction.READ_WRITE;
+
+    const name = window.location.pathname;
+    const description = "My Test Database";
+
+    const objectStoreName = "Blobs";
+
+    const blob = getRandomBlob(1000);
+    const file = getRandomFile("random.bin", 100000);
+
+    const objectData1 = { key: 1, object: { foo: blob, bar: blob } };
+    const objectData2 = { key: 2, object: { foo: file, bar: file } };
+
+    let request = mozIndexedDB.open(name, 1, description);
+    request.onerror = errorHandler;
+    request.onupgradeneeded = grabEventAndContinueHandler;
+    request.onsuccess = grabEventAndContinueHandler;
+    let event = yield;
+
+    is(event.type, "upgradeneeded", "Got correct event type");
+
+    let db = event.target.result;
+    db.onerror = errorHandler;
+
+    let objectStore = db.createObjectStore(objectStoreName, { });
+
+    objectStore.add(objectData1.object, objectData1.key);
+    objectStore.add(objectData2.object, objectData2.key);
+
+    event = yield;
+
+    is(event.type, "success", "Got correct event type");
+
+    objectStore = db.transaction([objectStoreName])
+                    .objectStore(objectStoreName);
+    request = objectStore.get(objectData1.key);
+    request.onsuccess = grabEventAndContinueHandler;
+    event = yield;
+
+    let result = event.target.result;
+
+    verifyBlob(result.foo, blob, 1);
+    yield;
+
+    verifyBlob(result.bar, blob, 1);
+    yield;
+
+    objectStore = db.transaction([objectStoreName])
+                    .objectStore(objectStoreName);
+    request = objectStore.get(objectData2.key);
+    request.onsuccess = grabEventAndContinueHandler;
+    event = yield;
+
+    result = event.target.result;
+
+    verifyBlob(result.foo, file, 2);
+    yield;
+
+    verifyBlob(result.bar, file, 2);
+    yield;
+
+    finishTest();
+    yield;
+  }
+  </script>
+  <script type="text/javascript;version=1.7" src="file.js"></script>
+  <script type="text/javascript;version=1.7" src="helpers.js"></script>
+
+</head>
+
+<body onload="runTest();"></body>
+
+</html>

dom/indexedDB/test/test_file_resurrection_delete.html

@@ -22,8 +22,6 @@
     const fileData = { key: 1, file: getRandomFile("random.bin", 100000) };
 
     {
-      let file = getRandomFile("random1.bin", 100000);
-
       let request = mozIndexedDB.open(name, 1, description);
       request.onerror = errorHandler;
       request.onupgradeneeded = grabEventAndContinueHandler;
@@ -71,7 +69,7 @@
     scheduleGC();
     yield;
 
-    is(getFileRefCount(name, 1), 1, "Correct ref count");
+    is(getFileRefCount(name, 1), 0, "Correct ref count");
 
     {
       let request = mozIndexedDB.open(name, 1, description);
@@ -120,7 +118,7 @@
     scheduleGC();
     yield;
 
-    is(getFileRefCount(name, 1), 1, "Correct ref count");
+    is(getFileRefCount(name, 1), 0, "Correct ref count");
 
     finishTest();
     yield;

dom/indexedDB/test/test_file_resurrection_transaction_abort.html

@@ -77,7 +77,7 @@
     scheduleGC();
     yield;
 
-    is(getFileRefCount(name, 1), 1, "Correct ref count");
+    is(getFileRefCount(name, 1), 0, "Correct ref count");
 
     finishTest();
     yield;