Finishing off: Modern IDB: Website data store management.
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 May 2016 22:58:13 +0000 (22:58 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 19 May 2016 22:58:13 +0000 (22:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=157626

Reviewed by Alex Christensen.

Source/WebCore:

Test: storage/indexeddb/modern/new-database-after-user-delete.html

This patch does two primary things:
1 - Implements the actual "delete files on disk" code for unopened databases,
    taken from existing WK2 code.

2 - When a UniqueIBDDatabase is told to close immediately for user delete, it also
    queues up a "delete backing store unconditionally" operation in the database task queue.

    That way, all of the open databases are deleted before the primary "delete all files"
    pass, and they are also deleted before any future openDB requests are handled that might
    recreate the files.

* Modules/indexeddb/server/IDBServer.cpp:
(WebCore::IDBServer::IDBServer::getOrCreateUniqueIDBDatabase):
(WebCore::IDBServer::IDBServer::deleteDatabase):
(WebCore::IDBServer::IDBServer::closeUniqueIDBDatabase):
(WebCore::IDBServer::IDBServer::openDBRequestCancelled):
(WebCore::IDBServer::removeAllDatabasesForOriginPath):
(WebCore::IDBServer::IDBServer::performCloseAndDeleteDatabasesModifiedSince):
(WebCore::IDBServer::IDBServer::performCloseAndDeleteDatabasesForOrigins):

* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::performUnconditionalDeleteBackingStore):
(WebCore::IDBServer::UniqueIDBDatabase::doneWithHardClose):
(WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
* Modules/indexeddb/server/UniqueIDBDatabase.h:

Source/WebKit2:

* DatabaseProcess/DatabaseProcess.cpp:
(WebKit::DatabaseProcess::deleteWebsiteDataForOrigins):
(WebKit::removeAllDatabasesForOriginPath): Deleted.
(WebKit::DatabaseProcess::deleteIndexedDatabaseEntriesForOrigins): Deleted.
(WebKit::DatabaseProcess::deleteIndexedDatabaseEntriesModifiedSince): Deleted.
* DatabaseProcess/DatabaseProcess.h:

LayoutTests:

* storage/indexeddb/modern/new-database-after-user-delete-expected.txt: Added.
* storage/indexeddb/modern/new-database-after-user-delete.html: Added.
* storage/indexeddb/modern/resources/new-database-after-user-delete.js: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@201195 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/storage/indexeddb/modern/new-database-after-user-delete-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/modern/new-database-after-user-delete.html [new file with mode: 0644]
LayoutTests/storage/indexeddb/modern/resources/new-database-after-user-delete.js [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h
Source/WebKit2/ChangeLog
Source/WebKit2/DatabaseProcess/DatabaseProcess.cpp
Source/WebKit2/DatabaseProcess/DatabaseProcess.h

index 982ffb8..46aa008 100644 (file)
@@ -1,3 +1,14 @@
+2016-05-19  Brady Eidson  <beidson@apple.com>
+
+        Finishing off: Modern IDB: Website data store management.
+        https://bugs.webkit.org/show_bug.cgi?id=157626
+
+        Reviewed by Alex Christensen.
+
+        * storage/indexeddb/modern/new-database-after-user-delete-expected.txt: Added.
+        * storage/indexeddb/modern/new-database-after-user-delete.html: Added.
+        * storage/indexeddb/modern/resources/new-database-after-user-delete.js: Added.
+        
 2016-05-19  Enrica Casucci  <enrica@apple.com>
 
         Drag cannot start if no drag data is available in the Pasteboard.
diff --git a/LayoutTests/storage/indexeddb/modern/new-database-after-user-delete-expected.txt b/LayoutTests/storage/indexeddb/modern/new-database-after-user-delete-expected.txt
new file mode 100644 (file)
index 0000000..5c38c5c
--- /dev/null
@@ -0,0 +1,21 @@
+Tests that if you perform a user delete with an open database connection, and then you re-open the same database, that the new database is actually new.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+indexedDB = self.indexedDB || self.webkitIndexedDB || self.mozIndexedDB || self.msIndexedDB || self.OIndexedDB;
+
+indexedDB.deleteDatabase(dbname)
+indexedDB.open(dbname)
+Initial upgrade needed: Old version - 0 New version - 1
+Created an objectStore an put a value in it
+Value was stored in objectStore
+Version change transaction completed. Version is now 1. About to request clearAllDatabases
+Requested clearAllDatabases
+Database connection error'ed out. Opening a new connection.
+Reopened upgrade needed: Old version - 0 New version - 1
+[PASS] The database has no object stores, meaning it is new
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/modern/new-database-after-user-delete.html b/LayoutTests/storage/indexeddb/modern/new-database-after-user-delete.html
new file mode 100644 (file)
index 0000000..8880e91
--- /dev/null
@@ -0,0 +1,10 @@
+<html>
+<head>
+<script src="../../../resources/js-test.js"></script>
+<script src="../resources/shared.js"></script>
+</head>
+<body>
+
+<script src="resources/new-database-after-user-delete.js"></script>
+</body>
+</html>
diff --git a/LayoutTests/storage/indexeddb/modern/resources/new-database-after-user-delete.js b/LayoutTests/storage/indexeddb/modern/resources/new-database-after-user-delete.js
new file mode 100644 (file)
index 0000000..cc8e4bc
--- /dev/null
@@ -0,0 +1,77 @@
+description("Tests that if you perform a user delete with an open database connection, and then you re-open the same database, that the new database is actually new.");
+
+indexedDBTest(prepareDatabase);
+
+function done()
+{
+    finishJSTest();
+}
+
+function log(message)
+{
+    debug(message);
+}
+
+var dbName;
+function prepareDatabase(event)
+{
+    log("Initial upgrade needed: Old version - " + event.oldVersion + " New version - " + event.newVersion);
+
+    var versionTransaction = event.target.transaction;
+    var database = event.target.result;
+    dbName = database.name;
+    
+    var objectStore = database.createObjectStore("TestObjectStore");
+    objectStore.put("bar", "foo").onsuccess = function() {
+        log("Value was stored in objectStore");
+    };
+    
+    log("Created an objectStore an put a value in it");
+    
+    database.onerror = function(event) {
+        log("Database connection error'ed out. Opening a new connection.");
+        continueTest();
+    }
+
+    versionTransaction.onabort = function(event) {
+        log("Initial upgrade versionchange transaction unexpected abort: " + event.type + " " + versionTransaction.error.name + ", " + versionTransaction.error.message);
+        done();
+    }
+
+    versionTransaction.oncomplete = function() {
+        log("Version change transaction completed. Version is now " + database.version + ". About to request clearAllDatabases");
+        if (window.testRunner) {
+            setTimeout("testRunner.clearAllDatabases();", 0);
+            log("Requested clearAllDatabases");
+        }
+    }
+
+    versionTransaction.onerror = function(event) {
+        log("Initial upgrade versionchange transaction unexpected error: " + event.type + " " + versionTransaction.error.name + ", " + versionTransaction.error.message);
+        done();
+    }
+}
+
+function continueTest()
+{
+    var openReq = indexedDB.open(dbName);
+    
+    openReq.onblocked = function() {
+        log("Second open request unexpected blocked");
+        done();
+    }
+
+    openReq.onupgradeneeded = function(event) {
+        log("Reopened upgrade needed: Old version - " + event.oldVersion + " New version - " + event.newVersion);
+        if (event.target.result.objectStoreNames.length)
+            log("[FAIL] The database has object stores already, but shouldn't");
+        else
+            log("[PASS] The database has no object stores, meaning it is new");
+        done();
+    }
+
+    openReq.onsuccess = function() {
+        log("Second open request unexpected success");
+        done();
+    }
+}
index 9450199..905d89e 100644 (file)
@@ -1,5 +1,40 @@
 2016-05-19  Brady Eidson  <beidson@apple.com>
 
+        Finishing off: Modern IDB: Website data store management.
+        https://bugs.webkit.org/show_bug.cgi?id=157626
+
+        Reviewed by Alex Christensen.
+
+        Test: storage/indexeddb/modern/new-database-after-user-delete.html
+
+        This patch does two primary things:
+        1 - Implements the actual "delete files on disk" code for unopened databases,
+            taken from existing WK2 code.
+
+        2 - When a UniqueIBDDatabase is told to close immediately for user delete, it also
+            queues up a "delete backing store unconditionally" operation in the database task queue.
+
+            That way, all of the open databases are deleted before the primary "delete all files"
+            pass, and they are also deleted before any future openDB requests are handled that might
+            recreate the files.
+
+        * Modules/indexeddb/server/IDBServer.cpp:
+        (WebCore::IDBServer::IDBServer::getOrCreateUniqueIDBDatabase):
+        (WebCore::IDBServer::IDBServer::deleteDatabase):
+        (WebCore::IDBServer::IDBServer::closeUniqueIDBDatabase):
+        (WebCore::IDBServer::IDBServer::openDBRequestCancelled):
+        (WebCore::IDBServer::removeAllDatabasesForOriginPath):
+        (WebCore::IDBServer::IDBServer::performCloseAndDeleteDatabasesModifiedSince):
+        (WebCore::IDBServer::IDBServer::performCloseAndDeleteDatabasesForOrigins):
+
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::performUnconditionalDeleteBackingStore):
+        (WebCore::IDBServer::UniqueIDBDatabase::doneWithHardClose):
+        (WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
+2016-05-19  Brady Eidson  <beidson@apple.com>
+
         REGRESSION(201098) GuardMalloc / ASan crashes in WebCore::IDBServer::UniqueIDBDatabase::executeNextDatabaseTaskReply
         https://bugs.webkit.org/show_bug.cgi?id=157917
 
index 41c0d06..4579dd8 100644 (file)
@@ -111,6 +111,8 @@ void IDBServer::unregisterDatabaseConnection(UniqueIDBDatabaseConnection& connec
 
 UniqueIDBDatabase& IDBServer::getOrCreateUniqueIDBDatabase(const IDBDatabaseIdentifier& identifier)
 {
+    ASSERT(isMainThread());
+
     auto uniqueIDBDatabase = m_uniqueIDBDatabaseMap.add(identifier, nullptr);
     if (uniqueIDBDatabase.isNewEntry)
         uniqueIDBDatabase.iterator->value = UniqueIDBDatabase::create(*this, identifier);
@@ -147,7 +149,8 @@ void IDBServer::openDatabase(const IDBRequestData& requestData)
 void IDBServer::deleteDatabase(const IDBRequestData& requestData)
 {
     LOG(IndexedDB, "IDBServer::deleteDatabase - %s", requestData.databaseIdentifier().debugString().utf8().data());
-    
+    ASSERT(isMainThread());
+
     auto connection = m_connectionMap.get(requestData.requestIdentifier().connectionIdentifier());
     if (!connection) {
         // If the connection back to the client is gone, there's no way to delete the database as
@@ -165,9 +168,9 @@ void IDBServer::deleteDatabase(const IDBRequestData& requestData)
 void IDBServer::closeUniqueIDBDatabase(UniqueIDBDatabase& database)
 {
     LOG(IndexedDB, "IDBServer::closeUniqueIDBDatabase");
+    ASSERT(isMainThread());
 
-    auto deletedDatabase = m_uniqueIDBDatabaseMap.take(database.identifier());
-    ASSERT_UNUSED(deletedDatabase, deletedDatabase.get() == &database);
+    m_uniqueIDBDatabaseMap.remove(database.identifier());
 }
 
 void IDBServer::abortTransaction(const IDBResourceIdentifier& transactionIdentifier)
@@ -382,6 +385,7 @@ void IDBServer::didFireVersionChangeEvent(uint64_t databaseConnectionIdentifier,
 void IDBServer::openDBRequestCancelled(const IDBRequestData& requestData)
 {
     LOG(IndexedDB, "IDBServer::openDBRequestCancelled");
+    ASSERT(isMainThread());
 
     auto* uniqueIDBDatabase = m_uniqueIDBDatabaseMap.get(requestData.databaseIdentifier());
     if (!uniqueIDBDatabase)
@@ -530,15 +534,52 @@ void IDBServer::closeAndDeleteDatabasesForOrigins(const Vector<SecurityOriginDat
     postDatabaseTask(createCrossThreadTask(*this, &IDBServer::performCloseAndDeleteDatabasesForOrigins, origins, callbackID));
 }
 
-void IDBServer::performCloseAndDeleteDatabasesModifiedSince(std::chrono::system_clock::time_point, uint64_t callbackID)
+static void removeAllDatabasesForOriginPath(const String& originPath, std::chrono::system_clock::time_point modifiedSince)
+{
+    Vector<String> databasePaths = listDirectory(originPath, "*");
+
+    for (auto& databasePath : databasePaths) {
+        String databaseFile = pathByAppendingComponent(databasePath, "IndexedDB.sqlite3");
+
+        if (!fileExists(databaseFile))
+            continue;
+
+        if (modifiedSince > std::chrono::system_clock::time_point::min()) {
+            time_t modificationTime;
+            if (!getFileModificationTime(databaseFile, modificationTime))
+                continue;
+
+            if (std::chrono::system_clock::from_time_t(modificationTime) < modifiedSince)
+                continue;
+        }
+
+        SQLiteFileSystem::deleteDatabaseFile(databaseFile);
+        deleteEmptyDirectory(databasePath);
+    }
+
+    deleteEmptyDirectory(originPath);
+}
+
+void IDBServer::performCloseAndDeleteDatabasesModifiedSince(std::chrono::system_clock::time_point modifiedSince, uint64_t callbackID)
 {
-    // FIXME: Implement deleting the files.
+    if (!m_databaseDirectoryPath.isEmpty()) {
+        Vector<String> originPaths = listDirectory(m_databaseDirectoryPath, "*");
+        for (auto& originPath : originPaths)
+            removeAllDatabasesForOriginPath(originPath, modifiedSince);
+    }
+
     postDatabaseTaskReply(createCrossThreadTask(*this, &IDBServer::didPerformCloseAndDeleteDatabases, callbackID));
 }
 
-void IDBServer::performCloseAndDeleteDatabasesForOrigins(const Vector<SecurityOriginData>&, uint64_t callbackID)
+void IDBServer::performCloseAndDeleteDatabasesForOrigins(const Vector<SecurityOriginData>& origins, uint64_t callbackID)
 {
-    // FIXME: Implement deleting the files.
+    if (!m_databaseDirectoryPath.isEmpty()) {
+        for (const auto& origin : origins) {
+            String originPath = pathByAppendingComponent(m_databaseDirectoryPath, origin.securityOrigin()->databaseIdentifier());
+            removeAllDatabasesForOriginPath(originPath, std::chrono::system_clock::time_point::min());
+        }
+    }
+
     postDatabaseTaskReply(createCrossThreadTask(*this, &IDBServer::didPerformCloseAndDeleteDatabases, callbackID));
 }
 
index 637fc9d..bdc3e22 100644 (file)
@@ -246,6 +246,20 @@ void UniqueIDBDatabase::deleteBackingStore(const IDBDatabaseIdentifier& identifi
     postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didDeleteBackingStore, deletedVersion));
 }
 
+void UniqueIDBDatabase::performUnconditionalDeleteBackingStore()
+{
+    ASSERT(!isMainThread());
+    LOG(IndexedDB, "(db) UniqueIDBDatabase::performUnconditionalDeleteBackingStore");
+
+    if (!m_backingStore)
+        return;
+
+    m_backingStore->deleteBackingStore();
+    m_backingStore = nullptr;
+    m_backingStoreSupportsSimultaneousTransactions = false;
+    m_backingStoreIsEphemeral = false;
+}
+
 void UniqueIDBDatabase::didDeleteBackingStore(uint64_t deletedVersion)
 {
     ASSERT(isMainThread());
@@ -1530,7 +1544,7 @@ void UniqueIDBDatabase::executeNextDatabaseTaskReply()
 
 bool UniqueIDBDatabase::doneWithHardClose()
 {
-    return (!m_queuedTaskCount && m_serverClosePendingDatabaseConnections.isEmpty());
+    return !m_queuedTaskCount && m_clientClosePendingDatabaseConnections.isEmpty() && m_serverClosePendingDatabaseConnections.isEmpty();
 }
 
 static void errorOpenDBRequestForUserDelete(ServerOpenDBRequest& request)
@@ -1605,8 +1619,10 @@ void UniqueIDBDatabase::immediateCloseForUserDelete()
     // Set up the database to remain alive-but-inert until all of its background activity finishes and all
     // database connections confirm that they have closed.
     m_hardClosedForUserDelete = true;
-    if (!doneWithHardClose())
-        m_hardCloseProtector = this;
+    m_hardCloseProtector = this;
+
+    // Have the database unconditionally delete itself on the database task queue.
+    postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performUnconditionalDeleteBackingStore));
 
     // Remove the database from the IDBServer's set of open databases.
     // If there is no in-progress background thread activity for this database, it will be deleted here.
index ae504d6..011d64f 100644 (file)
@@ -153,6 +153,7 @@ private:
     void performOpenCursor(uint64_t callbackIdentifier, const IDBResourceIdentifier& transactionIdentifier, const IDBCursorInfo&);
     void performIterateCursor(uint64_t callbackIdentifier, const IDBResourceIdentifier& transactionIdentifier, const IDBResourceIdentifier& cursorIdentifier, const IDBKeyData&, unsigned long count);
     void performActivateTransactionInBackingStore(uint64_t callbackIdentifier, const IDBTransactionInfo&);
+    void performUnconditionalDeleteBackingStore();
 
     // Main thread callbacks
     void didDeleteBackingStore(uint64_t deletedVersion);
index c9fa7ae..f8e6081 100644 (file)
@@ -1,3 +1,17 @@
+2016-05-19  Brady Eidson  <beidson@apple.com>
+
+        Finishing off: Modern IDB: Website data store management.
+        https://bugs.webkit.org/show_bug.cgi?id=157626
+
+        Reviewed by Alex Christensen.
+
+        * DatabaseProcess/DatabaseProcess.cpp:
+        (WebKit::DatabaseProcess::deleteWebsiteDataForOrigins):
+        (WebKit::removeAllDatabasesForOriginPath): Deleted.
+        (WebKit::DatabaseProcess::deleteIndexedDatabaseEntriesForOrigins): Deleted.
+        (WebKit::DatabaseProcess::deleteIndexedDatabaseEntriesModifiedSince): Deleted.
+        * DatabaseProcess/DatabaseProcess.h:
+
 2016-05-19  Brent Fulgham  <bfulgham@apple.com>
 
         [OS X][WK2] Expand sandbox for new mach endpoints
index 7e831cf..beeb8a8 100644 (file)
@@ -287,17 +287,8 @@ void DatabaseProcess::deleteWebsiteDataForOrigins(WebCore::SessionID, OptionSet<
     }));
 
 #if ENABLE(INDEXED_DATABASE)
-    if (websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases)) {
-        Vector<RefPtr<WebCore::SecurityOrigin>> securityOrigins;
-        for (const auto& securityOriginData : securityOriginDatas)
-            securityOrigins.append(securityOriginData.securityOrigin());
-
-        postDatabaseTask(std::make_unique<CrossThreadTask>([this, securityOrigins, callbackAggregator] {
-            deleteIndexedDatabaseEntriesForOrigins(securityOrigins);
-
-            RunLoop::main().dispatch([callbackAggregator] { });
-        }));
-    }
+    if (websiteDataTypes.contains(WebsiteDataType::IndexedDBDatabases))
+        idbServer().closeAndDeleteDatabasesForOrigins(securityOriginDatas, [callbackAggregator] { });
 #endif
 }
 
@@ -345,59 +336,6 @@ Vector<RefPtr<WebCore::SecurityOrigin>> DatabaseProcess::indexedDatabaseOrigins(
     return securityOrigins;
 }
 
-static void removeAllDatabasesForOriginPath(const String& originPath, std::chrono::system_clock::time_point modifiedSince)
-{
-    // FIXME: We should also close/invalidate any live handles to the database files we are about to delete.
-    // Right now:
-    //     - For read-only operations, they will continue functioning as normal on the unlinked file.
-    //     - For write operations, they will start producing errors as SQLite notices the missing backing store.
-    // This is tracked by https://bugs.webkit.org/show_bug.cgi?id=135347
-
-    Vector<String> databasePaths = listDirectory(originPath, "*");
-
-    for (auto& databasePath : databasePaths) {
-        String databaseFile = pathByAppendingComponent(databasePath, "IndexedDB.sqlite3");
-
-        if (!fileExists(databaseFile))
-            continue;
-
-        if (modifiedSince > std::chrono::system_clock::time_point::min()) {
-            time_t modificationTime;
-            if (!getFileModificationTime(databaseFile, modificationTime))
-                continue;
-
-            if (std::chrono::system_clock::from_time_t(modificationTime) < modifiedSince)
-                continue;
-        }
-
-        deleteFile(databaseFile);
-        deleteEmptyDirectory(databasePath);
-    }
-
-    deleteEmptyDirectory(originPath);
-}
-
-void DatabaseProcess::deleteIndexedDatabaseEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>& securityOrigins)
-{
-    if (m_indexedDatabaseDirectory.isEmpty())
-        return;
-
-    for (const auto& securityOrigin : securityOrigins) {
-        String originPath = pathByAppendingComponent(m_indexedDatabaseDirectory, securityOrigin->databaseIdentifier());
-
-        removeAllDatabasesForOriginPath(originPath, std::chrono::system_clock::time_point::min());
-    }
-}
-
-void DatabaseProcess::deleteIndexedDatabaseEntriesModifiedSince(std::chrono::system_clock::time_point modifiedSince)
-{
-    if (m_indexedDatabaseDirectory.isEmpty())
-        return;
-
-    Vector<String> originPaths = listDirectory(m_indexedDatabaseDirectory, "*");
-    for (auto& originPath : originPaths)
-        removeAllDatabasesForOriginPath(originPath, modifiedSince);
-}
 #endif
 
 void DatabaseProcess::getSandboxExtensionsForBlobFiles(const Vector<String>& filenames, std::function<void (const SandboxExtension::HandleArray&)> completionHandler)
index e789022..71e1ed0 100644 (file)
@@ -110,8 +110,6 @@ private:
 
 #if ENABLE(INDEXED_DATABASE)
     Vector<RefPtr<WebCore::SecurityOrigin>> indexedDatabaseOrigins();
-    void deleteIndexedDatabaseEntriesForOrigins(const Vector<RefPtr<WebCore::SecurityOrigin>>&);
-    void deleteIndexedDatabaseEntriesModifiedSince(std::chrono::system_clock::time_point modifiedSince);
 #endif
 
     // For execution on work queue thread only