Modern IDB: Don't open any new connections until after version change transactions...
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Dec 2015 07:40:29 +0000 (07:40 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 24 Dec 2015 07:40:29 +0000 (07:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=152441

Reviewed by Alex Christensen.

Source/WebCore:

No new tests (At least 4 failing tests now pass).

When a version change transaction is in progress for a database, the server should not open any new connections
to that database until the version change transaction has been 100% completed.

This means *all* events related to finishing the transaction must fire.

To support this, a new message from client -> server is added.

* Modules/indexeddb/client/IDBConnectionToServer.cpp:
(WebCore::IDBClient::IDBConnectionToServer::didFinishHandlingVersionChangeTransaction):
* Modules/indexeddb/client/IDBConnectionToServer.h:
* Modules/indexeddb/client/IDBConnectionToServerDelegate.h:

* Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp:
(WebCore::IDBClient::IDBOpenDBRequest::dispatchEvent):
* Modules/indexeddb/client/IDBOpenDBRequestImpl.h:
* Modules/indexeddb/client/IDBRequestImpl.h:

* Modules/indexeddb/server/IDBServer.cpp:
(WebCore::IDBServer::IDBServer::didFinishHandlingVersionChangeTransaction):
* Modules/indexeddb/server/IDBServer.h:

* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::handleDatabaseOperations):
(WebCore::IDBServer::UniqueIDBDatabase::didFinishHandlingVersionChange):
(WebCore::IDBServer::UniqueIDBDatabase::commitTransaction): Deleted.
(WebCore::IDBServer::UniqueIDBDatabase::didPerformAbortTransaction): Deleted.
(WebCore::IDBServer::UniqueIDBDatabase::inProgressTransactionCompleted): Deleted.
* Modules/indexeddb/server/UniqueIDBDatabase.h:

* Modules/indexeddb/server/UniqueIDBDatabaseTransaction.cpp:
(WebCore::IDBServer::UniqueIDBDatabaseTransaction::didFinishHandlingVersionChange):
* Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h:

* Modules/indexeddb/shared/IDBTransactionInfo.cpp:
(WebCore::IDBTransactionInfo::loggingString):
* Modules/indexeddb/shared/IDBTransactionInfo.h:

* Modules/indexeddb/shared/InProcessIDBServer.cpp:
(WebCore::InProcessIDBServer::didFinishHandlingVersionChangeTransaction):
* Modules/indexeddb/shared/InProcessIDBServer.h:

LayoutTests:

* platform/mac-wk1/TestExpectations: Enable 4 now-passing tests.

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

19 files changed:
LayoutTests/ChangeLog
LayoutTests/platform/mac-wk1/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.cpp
Source/WebCore/Modules/indexeddb/client/IDBConnectionToServer.h
Source/WebCore/Modules/indexeddb/client/IDBConnectionToServerDelegate.h
Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp
Source/WebCore/Modules/indexeddb/client/IDBOpenDBRequestImpl.h
Source/WebCore/Modules/indexeddb/client/IDBRequestImpl.h
Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
Source/WebCore/Modules/indexeddb/server/IDBServer.h
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.cpp
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h
Source/WebCore/Modules/indexeddb/shared/IDBTransactionInfo.cpp
Source/WebCore/Modules/indexeddb/shared/IDBTransactionInfo.h
Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.cpp
Source/WebCore/Modules/indexeddb/shared/InProcessIDBServer.h

index ec5e3c3..7388552 100644 (file)
@@ -1,5 +1,14 @@
 2015-12-23  Brady Eidson  <beidson@apple.com>
 
+        Modern IDB: Don't open any new connections until after version change transactions are completely handled.
+        https://bugs.webkit.org/show_bug.cgi?id=152441
+
+        Reviewed by Alex Christensen.
+
+        * platform/mac-wk1/TestExpectations: Enable 4 now-passing tests.
+
+2015-12-23  Brady Eidson  <beidson@apple.com>
+
         Modern IDB: storage/indexeddb/transaction-basics.html fails.
         https://bugs.webkit.org/show_bug.cgi?id=152481
 
index ed2b224..469aba7 100644 (file)
@@ -99,10 +99,6 @@ storage/indexeddb/unblocked-version-changes.html [ Skip ]
 storage/indexeddb/database-deletepending-flag.html [ Failure ]
 storage/indexeddb/delete-closed-database-object.html [ Failure ]
 storage/indexeddb/intversion-gated-on-delete.html [ Failure ]
-storage/indexeddb/intversion-open-in-upgradeneeded.html [ Failure ]
-storage/indexeddb/intversion-pending-version-changes-descending.html [ Failure ]
-storage/indexeddb/intversion-pending-version-changes-same.html [ Failure ]
-storage/indexeddb/intversion-two-opens-no-versions.html [ Failure ]
 storage/indexeddb/odd-strings.html [ Failure ]
 storage/indexeddb/open-db-private-browsing.html [ Failure ]
 storage/indexeddb/properties-disabled-at-runtime.html [ Failure ]
index 65677cd..e3edc8a 100644 (file)
@@ -1,5 +1,55 @@
 2015-12-23  Brady Eidson  <beidson@apple.com>
 
+        Modern IDB: Don't open any new connections until after version change transactions are completely handled.
+        https://bugs.webkit.org/show_bug.cgi?id=152441
+
+        Reviewed by Alex Christensen.
+
+        No new tests (At least 4 failing tests now pass).
+        
+        When a version change transaction is in progress for a database, the server should not open any new connections
+        to that database until the version change transaction has been 100% completed.
+        
+        This means *all* events related to finishing the transaction must fire.
+        
+        To support this, a new message from client -> server is added.
+
+        * Modules/indexeddb/client/IDBConnectionToServer.cpp:
+        (WebCore::IDBClient::IDBConnectionToServer::didFinishHandlingVersionChangeTransaction):
+        * Modules/indexeddb/client/IDBConnectionToServer.h:
+        * Modules/indexeddb/client/IDBConnectionToServerDelegate.h:
+        
+        * Modules/indexeddb/client/IDBOpenDBRequestImpl.cpp:
+        (WebCore::IDBClient::IDBOpenDBRequest::dispatchEvent):
+        * Modules/indexeddb/client/IDBOpenDBRequestImpl.h:
+        * Modules/indexeddb/client/IDBRequestImpl.h:
+        
+        * Modules/indexeddb/server/IDBServer.cpp:
+        (WebCore::IDBServer::IDBServer::didFinishHandlingVersionChangeTransaction):
+        * Modules/indexeddb/server/IDBServer.h:
+        
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::handleDatabaseOperations):
+        (WebCore::IDBServer::UniqueIDBDatabase::didFinishHandlingVersionChange):
+        (WebCore::IDBServer::UniqueIDBDatabase::commitTransaction): Deleted.
+        (WebCore::IDBServer::UniqueIDBDatabase::didPerformAbortTransaction): Deleted.
+        (WebCore::IDBServer::UniqueIDBDatabase::inProgressTransactionCompleted): Deleted.
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+        
+        * Modules/indexeddb/server/UniqueIDBDatabaseTransaction.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabaseTransaction::didFinishHandlingVersionChange):
+        * Modules/indexeddb/server/UniqueIDBDatabaseTransaction.h:
+        
+        * Modules/indexeddb/shared/IDBTransactionInfo.cpp:
+        (WebCore::IDBTransactionInfo::loggingString):
+        * Modules/indexeddb/shared/IDBTransactionInfo.h:
+        
+        * Modules/indexeddb/shared/InProcessIDBServer.cpp:
+        (WebCore::InProcessIDBServer::didFinishHandlingVersionChangeTransaction):
+        * Modules/indexeddb/shared/InProcessIDBServer.h:
+
+2015-12-23  Brady Eidson  <beidson@apple.com>
+
         Modern IDB: storage/indexeddb/transaction-basics.html fails.
         https://bugs.webkit.org/show_bug.cgi?id=152481
 
index dc133bd..ff1ba86 100644 (file)
@@ -290,6 +290,13 @@ void IDBConnectionToServer::didCommitTransaction(const IDBResourceIdentifier& tr
     transaction->didCommit(error);
 }
 
+void IDBConnectionToServer::didFinishHandlingVersionChangeTransaction(IDBTransaction& transaction)
+{
+    LOG(IndexedDB, "IDBConnectionToServer::didFinishHandlingVersionChangeTransaction");
+    auto identifier = transaction.info().identifier();
+    m_delegate->didFinishHandlingVersionChangeTransaction(identifier);
+}
+
 void IDBConnectionToServer::abortTransaction(IDBTransaction& transaction)
 {
     LOG(IndexedDB, "IDBConnectionToServer::abortTransaction");
index dcc148e..b14e50b 100644 (file)
@@ -98,6 +98,8 @@ public:
     void commitTransaction(IDBTransaction&);
     void didCommitTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError&);
 
+    void didFinishHandlingVersionChangeTransaction(IDBTransaction&);
+
     void abortTransaction(IDBTransaction&);
     void didAbortTransaction(const IDBResourceIdentifier& transactionIdentifier, const IDBError&);
 
index 0b9090e..75b554c 100644 (file)
@@ -59,6 +59,7 @@ public:
     virtual void openDatabase(IDBRequestData&) = 0;
     virtual void abortTransaction(IDBResourceIdentifier&) = 0;
     virtual void commitTransaction(IDBResourceIdentifier&) = 0;
+    virtual void didFinishHandlingVersionChangeTransaction(IDBResourceIdentifier&) = 0;
     virtual void createObjectStore(const IDBRequestData&, const IDBObjectStoreInfo&) = 0;
     virtual void deleteObjectStore(const IDBRequestData&, const String& objectStoreName) = 0;
     virtual void clearObjectStore(const IDBRequestData&, uint64_t objectStoreIdentifier) = 0;
index bd1527c..af0a3ca 100644 (file)
@@ -101,6 +101,16 @@ void IDBOpenDBRequest::fireErrorAfterVersionChangeCompletion()
     enqueueEvent(Event::create(eventNames().errorEvent, true, true));
 }
 
+bool IDBOpenDBRequest::dispatchEvent(Event& event)
+{
+    bool result = IDBRequest::dispatchEvent(event);
+
+    if (m_transaction && m_transaction->isVersionChange() && (event.type() == eventNames().errorEvent || event.type() == eventNames().successEvent))
+        m_transaction->database().serverConnection().didFinishHandlingVersionChangeTransaction(*m_transaction);
+
+    return result;
+}
+
 void IDBOpenDBRequest::onSuccess(const IDBResultData& resultData)
 {
     LOG(IndexedDB, "IDBOpenDBRequest::onSuccess()");
index daa9f6f..f242a79 100644 (file)
@@ -57,6 +57,8 @@ public:
     void fireSuccessAfterVersionChangeCommit();
     void fireErrorAfterVersionChangeCompletion();
 
+    virtual bool dispatchEvent(Event&) override final;
+
 private:
     IDBOpenDBRequest(IDBConnectionToServer&, ScriptExecutionContext*, const IDBDatabaseIdentifier&, uint64_t version, IndexedDB::RequestType);
 
index 7ddc3a0..16c5172 100644 (file)
@@ -79,7 +79,7 @@ public:
     using RefCounted<IDBRequest>::deref;
 
     void enqueueEvent(Ref<Event>&&);
-    virtual bool dispatchEvent(Event&) override final;
+    virtual bool dispatchEvent(Event&) override;
 
     IDBConnectionToServer& connection() { return m_connection; }
 
index 0734376..7fa57c0 100644 (file)
@@ -317,6 +317,17 @@ void IDBServer::commitTransaction(const IDBResourceIdentifier& transactionIdenti
     transaction->commit();
 }
 
+void IDBServer::didFinishHandlingVersionChangeTransaction(const IDBResourceIdentifier& transactionIdentifier)
+{
+    LOG(IndexedDB, "IDBServer::didFinishHandlingVersionChangeTransaction");
+
+    auto transaction = m_transactions.get(transactionIdentifier);
+    if (!transaction)
+        return;
+
+    transaction->didFinishHandlingVersionChange();
+}
+
 void IDBServer::databaseConnectionClosed(uint64_t databaseConnectionIdentifier)
 {
     LOG(IndexedDB, "IDBServer::databaseConnectionClosed");
index b2243b7..be33ab0 100644 (file)
@@ -60,6 +60,7 @@ public:
     void deleteDatabase(const IDBRequestData&);
     void abortTransaction(const IDBResourceIdentifier&);
     void commitTransaction(const IDBResourceIdentifier&);
+    void didFinishHandlingVersionChangeTransaction(const IDBResourceIdentifier&);
     void createObjectStore(const IDBRequestData&, const IDBObjectStoreInfo&);
     void deleteObjectStore(const IDBRequestData&, const String& objectStoreName);
     void clearObjectStore(const IDBRequestData&, uint64_t objectStoreIdentifier);
index 71f65f8..a00a0e2 100644 (file)
@@ -206,13 +206,10 @@ void UniqueIDBDatabase::handleDatabaseOperations()
     ASSERT(isMainThread());
     LOG(IndexedDB, "(main) UniqueIDBDatabase::handleDatabaseOperations - There are %zu pending", m_pendingDatabaseOperations.size());
 
-    if (m_versionChangeDatabaseConnection || m_currentOperation) {
-        // We can't start the next database operation quite yet, but we might need to notify all open connections
-        // about a pending delete.
-        if (!m_pendingDatabaseOperations.isEmpty() && m_pendingDatabaseOperations.first()->isDeleteRequest() && !m_hasNotifiedConnectionsOfDelete) {
-            m_hasNotifiedConnectionsOfDelete = true;
-            notifyConnectionsOfVersionChange(0);
-        }
+    if (m_versionChangeDatabaseConnection || m_versionChangeTransaction || m_currentOperation) {
+        // We can't start any new open-database operations right now, but we might be able to start handling a delete operation.
+        if (!m_currentOperation && !m_pendingDatabaseOperations.isEmpty() && m_pendingDatabaseOperations.first()->isDeleteRequest())
+            m_currentOperation = m_pendingDatabaseOperations.takeFirst();
 
         // Some operations (such as the first open operation after a delete) require multiple passes to completely handle
         if (m_currentOperation)
@@ -849,9 +846,6 @@ void UniqueIDBDatabase::commitTransaction(UniqueIDBDatabaseTransaction& transact
         ASSERT(&m_versionChangeTransaction->databaseConnection() == m_versionChangeDatabaseConnection);
         ASSERT(m_databaseInfo->version() == transaction.info().newVersion());
 
-        m_versionChangeTransaction = nullptr;
-        m_versionChangeDatabaseConnection = nullptr;
-
         invokeOperationAndTransactionTimer();
     }
 
@@ -889,6 +883,20 @@ void UniqueIDBDatabase::abortTransaction(UniqueIDBDatabaseTransaction& transacti
     m_server.postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performAbortTransaction, callbackID, transaction.info().identifier()));
 }
 
+void UniqueIDBDatabase::didFinishHandlingVersionChange(UniqueIDBDatabaseTransaction& transaction)
+{
+    ASSERT(isMainThread());
+    LOG(IndexedDB, "(main) UniqueIDBDatabase::didFinishHandlingVersionChange");
+
+    ASSERT(m_versionChangeTransaction);
+    ASSERT_UNUSED(transaction, m_versionChangeTransaction == &transaction);
+
+    m_versionChangeTransaction = nullptr;
+    m_versionChangeDatabaseConnection = nullptr;
+
+    invokeOperationAndTransactionTimer();
+}
+
 void UniqueIDBDatabase::performAbortTransaction(uint64_t callbackIdentifier, const IDBResourceIdentifier& transactionIdentifier)
 {
     ASSERT(!isMainThread());
@@ -907,9 +915,6 @@ void UniqueIDBDatabase::didPerformAbortTransaction(uint64_t callbackIdentifier,
         ASSERT(&m_versionChangeTransaction->databaseConnection() == m_versionChangeDatabaseConnection);
         ASSERT(m_versionChangeTransaction->originalDatabaseInfo());
         m_databaseInfo = std::make_unique<IDBDatabaseInfo>(*m_versionChangeTransaction->originalDatabaseInfo());
-
-        m_versionChangeTransaction = nullptr;
-        m_versionChangeDatabaseConnection = nullptr;
     }
 
     inProgressTransactionCompleted(transactionIdentifier);
@@ -1094,9 +1099,6 @@ void UniqueIDBDatabase::inProgressTransactionCompleted(const IDBResourceIdentifi
     auto transaction = m_inProgressTransactions.take(transactionIdentifier);
     ASSERT(transaction);
 
-    if (m_versionChangeTransaction == transaction)
-        m_versionChangeTransaction = nullptr;
-
     for (auto objectStore : transaction->objectStoreIdentifiers())
         m_objectStoreTransactionCounts.remove(objectStore);
 
index 2d8edfe..12681de 100644 (file)
@@ -92,6 +92,7 @@ public:
     void iterateCursor(const IDBRequestData&, const IDBKeyData&, unsigned long count, GetResultCallback);
     void commitTransaction(UniqueIDBDatabaseTransaction&, ErrorCallback);
     void abortTransaction(UniqueIDBDatabaseTransaction&, ErrorCallback);
+    void didFinishHandlingVersionChange(UniqueIDBDatabaseTransaction&);
     void transactionDestroyed(UniqueIDBDatabaseTransaction&);
     void connectionClosedFromClient(UniqueIDBDatabaseConnection&);
 
@@ -183,7 +184,7 @@ private:
     HashSet<RefPtr<UniqueIDBDatabaseConnection>> m_closePendingDatabaseConnections;
 
     RefPtr<UniqueIDBDatabaseConnection> m_versionChangeDatabaseConnection;
-    UniqueIDBDatabaseTransaction* m_versionChangeTransaction { nullptr };
+    RefPtr<UniqueIDBDatabaseTransaction> m_versionChangeTransaction;
 
     bool m_isOpeningBackingStore { false };
     std::unique_ptr<IDBBackingStore> m_backingStore;
index b0b51e0..7e991ff 100644 (file)
@@ -309,6 +309,14 @@ void UniqueIDBDatabaseTransaction::didActivateInBackingStore(const IDBError& err
     m_databaseConnection->connectionToClient().didStartTransaction(m_transactionInfo.identifier(), error);
 }
 
+void UniqueIDBDatabaseTransaction::didFinishHandlingVersionChange()
+{
+    LOG(IndexedDB, "UniqueIDBDatabaseTransaction::didFinishHandlingVersionChange");
+    ASSERT(isVersionChange());
+
+    m_databaseConnection->database().didFinishHandlingVersionChange(*this);
+}
+
 } // namespace IDBServer
 } // namespace WebCore
 
index 803929a..5005f07 100644 (file)
@@ -79,6 +79,7 @@ public:
     void iterateCursor(const IDBRequestData&, const IDBKeyData&, unsigned long count);
 
     void didActivateInBackingStore(const IDBError&);
+    void didFinishHandlingVersionChange();
 
     const Vector<uint64_t>& objectStoreIdentifiers();
 
index 1c883ec..7b89ed7 100644 (file)
@@ -28,6 +28,8 @@
 
 #if ENABLE(INDEXED_DATABASE)
 
+#include "IDBTransactionImpl.h"
+
 namespace WebCore {
 
 IDBTransactionInfo::IDBTransactionInfo(const IDBResourceIdentifier& identifier)
@@ -80,6 +82,28 @@ IDBTransactionInfo IDBTransactionInfo::isolatedCopy() const
     return WTF::move(result);
 }
 
+#ifndef NDEBUG
+String IDBTransactionInfo::loggingString() const
+{
+    String modeString;
+    switch (m_mode) {
+    case IndexedDB::TransactionMode::ReadOnly:
+        modeString = IDBTransaction::modeReadOnly();
+        break;
+    case IndexedDB::TransactionMode::ReadWrite:
+        modeString = IDBTransaction::modeReadWrite();
+        break;
+    case IndexedDB::TransactionMode::VersionChange:
+        modeString = IDBTransaction::modeVersionChange();
+        break;
+    default:
+        ASSERT_NOT_REACHED();
+    }
+    
+    return makeString("Transaction: ", m_identifier.loggingString(), " mode ", modeString, " newVersion ", String::number(m_newVersion));
+}
+#endif
+
 } // namespace WebCore
 
 #endif // ENABLE(INDEXED_DATABASE)
index b17280e..bbec74b 100644 (file)
@@ -61,6 +61,10 @@ public:
 
     IDBDatabaseInfo* originalDatabaseInfo() const { return m_originalDatabaseInfo.get(); }
 
+#ifndef NDEBUG
+    String loggingString() const;
+#endif
+
 private:
     IDBTransactionInfo(const IDBResourceIdentifier&);
 
index f8e0f8f..1ef4054 100644 (file)
@@ -224,6 +224,14 @@ void InProcessIDBServer::commitTransaction(IDBResourceIdentifier& resourceIdenti
     });
 }
 
+void InProcessIDBServer::didFinishHandlingVersionChangeTransaction(IDBResourceIdentifier& transactionIdentifier)
+{
+    RefPtr<InProcessIDBServer> self(this);
+    RunLoop::current().dispatch([this, self, transactionIdentifier] {
+        m_server->didFinishHandlingVersionChangeTransaction(transactionIdentifier);
+    });
+}
+
 void InProcessIDBServer::createObjectStore(const IDBRequestData& resultData, const IDBObjectStoreInfo& info)
 {
     RefPtr<InProcessIDBServer> self(this);
index 38ea393..41d43dc 100644 (file)
@@ -58,6 +58,7 @@ public:
     virtual void openDatabase(IDBRequestData&) override final;
     virtual void abortTransaction(IDBResourceIdentifier&) override final;
     virtual void commitTransaction(IDBResourceIdentifier&) override final;
+    virtual void didFinishHandlingVersionChangeTransaction(IDBResourceIdentifier&) override final;
     virtual void createObjectStore(const IDBRequestData&, const IDBObjectStoreInfo&) override final;
     virtual void deleteObjectStore(const IDBRequestData&, const String& objectStoreName) override final;
     virtual void clearObjectStore(const IDBRequestData&, uint64_t objectStoreIdentifier) override final;