IDB: deleteDatabase() interface should be asynchronous
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Nov 2013 06:16:47 +0000 (06:16 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Nov 2013 06:16:47 +0000 (06:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=123787

Reviewed by Tim Horton.

No new tests (No behavior change for a tested port).

deleteDatabase now has no return value, but calls back to a bool function:
* Modules/indexeddb/IDBBackingStoreInterface.h:
* Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.cpp:
(WebCore::IDBBackingStoreLevelDB::deleteDatabase):
* Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h:

Refactor to account for the new async deleteDatabase:
* Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
(WebCore::IDBDatabaseBackendImpl::processPendingCalls):
(WebCore::IDBDatabaseBackendImpl::deleteDatabase):
(WebCore::IDBDatabaseBackendImpl::deleteDatabaseAsync):
* Modules/indexeddb/IDBDatabaseBackendImpl.h:

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBBackingStoreInterface.h
Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp
Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.h
Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.cpp
Source/WebCore/Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h

index 4d46efa..8c0957e 100644 (file)
@@ -1,5 +1,27 @@
 2013-11-04  Brady Eidson  <beidson@apple.com>
 
+        IDB: deleteDatabase() interface should be asynchronous
+        https://bugs.webkit.org/show_bug.cgi?id=123787
+
+        Reviewed by Tim Horton.
+
+        No new tests (No behavior change for a tested port).
+
+        deleteDatabase now has no return value, but calls back to a bool function:
+        * Modules/indexeddb/IDBBackingStoreInterface.h:
+        * Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.cpp:
+        (WebCore::IDBBackingStoreLevelDB::deleteDatabase):
+        * Modules/indexeddb/leveldb/IDBBackingStoreLevelDB.h:
+
+        Refactor to account for the new async deleteDatabase:
+        * Modules/indexeddb/IDBDatabaseBackendImpl.cpp:
+        (WebCore::IDBDatabaseBackendImpl::processPendingCalls):
+        (WebCore::IDBDatabaseBackendImpl::deleteDatabase):
+        (WebCore::IDBDatabaseBackendImpl::deleteDatabaseAsync):
+        * Modules/indexeddb/IDBDatabaseBackendImpl.h:
+
+2013-11-04  Brady Eidson  <beidson@apple.com>
+
         Add Modules/indexeddb/leveldb to the WebCore.xcodeproj
 
         Rubberstamped by Andreas Kling.
index ffc5020..5647baf 100644 (file)
@@ -82,15 +82,19 @@ public:
 
     virtual std::unique_ptr<Transaction> createBackingStoreTransaction() = 0;
 
+    // New-style asynchronous callbacks
     typedef std::function<void (const IDBDatabaseMetadata&, bool success)> GetIDBDatabaseMetadataFunction;
     virtual void getOrEstablishIDBDatabaseMetadata(const String& name, GetIDBDatabaseMetadataFunction) = 0;
 
+    typedef std::function<void (bool success)> BoolCallbackFunction;
+    virtual void deleteDatabase(const String& name, BoolCallbackFunction) = 0;
+
+    // Old-style synchronous callbacks
     virtual bool keyExistsInObjectStore(IDBBackingStoreInterface::Transaction&, int64_t databaseId, int64_t objectStoreId, const IDBKey&, RefPtr<IDBRecordIdentifier>& foundIDBRecordIdentifier) = 0;
 
     virtual bool putIndexDataForRecord(IDBBackingStoreInterface::Transaction&, int64_t databaseId, int64_t objectStoreId, int64_t indexId, const IDBKey&, const IDBRecordIdentifier*) = 0;
     virtual bool keyExistsInIndex(IDBBackingStoreInterface::Transaction&, int64_t databaseId, int64_t objectStoreId, int64_t indexId, const IDBKey&, RefPtr<IDBKey>& foundPrimaryKey, bool& exists) = 0;
 
-    virtual bool deleteDatabase(const String& name) = 0;
     virtual bool updateIDBDatabaseVersion(IDBBackingStoreInterface::Transaction&, int64_t rowId, uint64_t version) = 0;
 
     virtual bool getRecord(IDBBackingStoreInterface::Transaction&, int64_t databaseId, int64_t objectStoreId, const IDBKey&, Vector<char>& record) = 0;
index ab2189d..ef1840d 100644 (file)
@@ -415,11 +415,18 @@ void IDBDatabaseBackendImpl::processPendingCalls()
         return;
     while (!m_pendingDeleteCalls.isEmpty()) {
         OwnPtr<IDBPendingDeleteCall> pendingDeleteCall = m_pendingDeleteCalls.takeFirst();
-        deleteDatabaseFinal(pendingDeleteCall->callbacks());
+        m_deleteCallbacksWaitingCompletion.add(pendingDeleteCall->callbacks());
+        deleteDatabaseAsync(pendingDeleteCall->callbacks());
     }
-    // deleteDatabaseFinal should never re-queue calls.
+
+    // deleteDatabaseAsync should never re-queue calls.
     ASSERT(m_pendingDeleteCalls.isEmpty());
 
+    // If there are any database deletions waiting for completion, we're done for now.
+    // Further callbacks will be handled in a future call to processPendingCalls().
+    if (m_deleteCallbacksWaitingCompletion.isEmpty())
+        return;
+
     if (m_runningVersionChangeTransaction)
         return;
 
@@ -574,7 +581,7 @@ void IDBDatabaseBackendImpl::deleteDatabase(PassRefPtr<IDBCallbacks> prpCallback
         m_pendingDeleteCalls.append(IDBPendingDeleteCall::create(callbacks.release()));
         return;
     }
-    deleteDatabaseFinal(callbacks.release());
+    deleteDatabaseAsync(callbacks.release());
 }
 
 bool IDBDatabaseBackendImpl::isDeleteDatabaseBlocked()
@@ -582,18 +589,30 @@ bool IDBDatabaseBackendImpl::isDeleteDatabaseBlocked()
     return connectionCount();
 }
 
-void IDBDatabaseBackendImpl::deleteDatabaseFinal(PassRefPtr<IDBCallbacks> callbacks)
+void IDBDatabaseBackendImpl::deleteDatabaseAsync(PassRefPtr<IDBCallbacks> callbacks)
 {
     ASSERT(!isDeleteDatabaseBlocked());
     ASSERT(m_backingStore);
-    if (!m_backingStore->deleteDatabase(m_metadata.name)) {
-        callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error deleting database."));
-        return;
-    }
-    m_metadata.id = InvalidId;
-    m_metadata.version = IDBDatabaseMetadata::NoIntVersion;
-    m_metadata.objectStores.clear();
-    callbacks->onSuccess();
+
+    RefPtr<IDBDatabaseBackendImpl> self(this);
+    m_backingStore->deleteDatabase(m_metadata.name, [self, callbacks](bool success) {
+        ASSERT(self->m_deleteCallbacksWaitingCompletion.contains(callbacks));
+        self->m_deleteCallbacksWaitingCompletion.remove(callbacks);
+
+        // If this IDBDatabaseBackend was closed while waiting for deleteDatabase to complete, no point in performing any callbacks.
+        if (!self->m_backingStore)
+            return;
+
+        if (success) {
+            self->m_metadata.id = InvalidId;
+            self->m_metadata.version = IDBDatabaseMetadata::NoIntVersion;
+            self->m_metadata.objectStores.clear();
+            callbacks->onSuccess();
+        } else
+            callbacks->onError(IDBDatabaseError::create(IDBDatabaseException::UnknownError, "Internal error deleting database."));
+
+        self->processPendingCalls();
+    });
 }
 
 void IDBDatabaseBackendImpl::close(PassRefPtr<IDBDatabaseCallbacks> prpCallbacks)
index 6fba9f7..f8cf104 100644 (file)
@@ -116,13 +116,13 @@ private:
     void processPendingOpenCalls(bool success);
 
     bool isDeleteDatabaseBlocked();
-    void deleteDatabaseFinal(PassRefPtr<IDBCallbacks>);
+    void deleteDatabaseAsync(PassRefPtr<IDBCallbacks>);
 
     RefPtr<IDBBackingStoreInterface> m_backingStore;
     IDBDatabaseMetadata m_metadata;
 
     String m_identifier;
-    // This might not need to be a RefPtr since the factory's lifetime is that of the page group, but it's better to be conservitive than sorry.
+    // This might not need to be a RefPtr since the factory's lifetime is that of the page group, but it's better to be conservative than sorry.
     RefPtr<IDBFactoryBackendInterface> m_factory;
 
     OwnPtr<IDBTransactionCoordinator> m_transactionCoordinator;
@@ -135,6 +135,7 @@ private:
     OwnPtr<IDBPendingOpenCall> m_pendingSecondHalfOpen;
 
     Deque<OwnPtr<IDBPendingDeleteCall>> m_pendingDeleteCalls;
+    HashSet<RefPtr<IDBCallbacks>> m_deleteCallbacksWaitingCompletion;
 
     typedef ListHashSet<RefPtr<IDBDatabaseCallbacks>> DatabaseCallbacksSet;
     DatabaseCallbacksSet m_databaseCallbacksSet;
index 81df4ff..b95c538 100644 (file)
@@ -661,7 +661,7 @@ static void deleteRange(LevelDBTransaction* transaction, const Vector<char>& beg
         transaction->remove(it->key());
 }
 
-bool IDBBackingStoreLevelDB::deleteDatabase(const String& name)
+void IDBBackingStoreLevelDB::deleteDatabase(const String& name, BoolCallbackFunction boolCallbackFunction)
 {
     LOG(StorageAPI, "IDBBackingStoreLevelDB::deleteDatabase");
     OwnPtr<LevelDBWriteOnlyTransaction> transaction = LevelDBWriteOnlyTransaction::create(m_db.get());
@@ -669,10 +669,15 @@ bool IDBBackingStoreLevelDB::deleteDatabase(const String& name)
     IDBDatabaseMetadata metadata;
     bool success = false;
     bool ok = getIDBDatabaseMetaData(name, &metadata, success);
-    if (!ok)
-        return false;
-    if (!success)
-        return true;
+    if (!ok) {
+        boolCallbackFunction(false);
+        return;
+    }
+
+    if (!success) {
+        boolCallbackFunction(true);
+        return;
+    }
 
     const Vector<char> startKey = DatabaseMetaDataKey::encode(metadata.id, DatabaseMetaDataKey::OriginName);
     const Vector<char> stopKey = DatabaseMetaDataKey::encode(metadata.id + 1, DatabaseMetaDataKey::OriginName);
@@ -685,9 +690,10 @@ bool IDBBackingStoreLevelDB::deleteDatabase(const String& name)
 
     if (!transaction->commit()) {
         INTERNAL_WRITE_ERROR(DeleteDatabase);
-        return false;
+        boolCallbackFunction(false);
+        return;
     }
-    return true;
+    boolCallbackFunction(true);
 }
 
 static bool checkObjectStoreAndMetaDataType(const LevelDBIterator* it, const Vector<char>& stopKey, int64_t objectStoreId, int64_t metaDataType)
index f130bc0..e7865d5 100644 (file)
@@ -71,10 +71,12 @@ public:
 
     virtual Vector<String> getDatabaseNames();
 
+    // New-style asynchronous callbacks
     virtual void getOrEstablishIDBDatabaseMetadata(const String& name, GetIDBDatabaseMetadataFunction) OVERRIDE;
+    virtual void deleteDatabase(const String& name, BoolCallbackFunction) OVERRIDE;
 
+    // Old-style synchronous callbacks
     virtual bool updateIDBDatabaseVersion(IDBBackingStoreInterface::Transaction&, int64_t rowId, uint64_t version) OVERRIDE;
-    virtual bool deleteDatabase(const String& name) OVERRIDE;
 
     virtual bool createObjectStore(IDBBackingStoreInterface::Transaction&, int64_t databaseId, int64_t objectStoreId, const String& name, const IDBKeyPath&, bool autoIncrement) OVERRIDE;
     virtual bool deleteObjectStore(IDBBackingStoreInterface::Transaction&, int64_t databaseId, int64_t objectStoreId) OVERRIDE WARN_UNUSED_RETURN;