Modified: trunk/Source/WebCore/ChangeLog (194240 => 194241)
--- trunk/Source/WebCore/ChangeLog 2015-12-17 23:02:25 UTC (rev 194240)
+++ trunk/Source/WebCore/ChangeLog 2015-12-17 23:29:53 UTC (rev 194241)
@@ -1,3 +1,38 @@
+2015-12-17 Brady Eidson <[email protected]>
+
+ Modern IDB: Refactor open/delete requests to exist in the same queue.
+ https://bugs.webkit.org/show_bug.cgi?id=152397
+
+ Reviewed by Alex Christensen.
+
+ No new tests (Refactor, all existing tests continue to pass).
+
+ The order between incoming open and delete requests matters, and each request
+ needs to be handled individually.
+
+ This patch does the above without changing behavior on existing passing tests,
+ while moving many currently skipped tests closer to passing.
+
+ * Modules/indexeddb/server/IDBServerOperation.cpp:
+ (WebCore::IDBServer::IDBServerOperation::notifyDeleteRequestBlocked):
+ (WebCore::IDBServer::IDBServerOperation::notifyDidDeleteDatabase):
+ * Modules/indexeddb/server/IDBServerOperation.h:
+ (WebCore::IDBServer::IDBServerOperation::hasNotifiedDeleteRequestBlocked):
+
+ * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+ (WebCore::IDBServer::UniqueIDBDatabase::~UniqueIDBDatabase):
+ (WebCore::IDBServer::UniqueIDBDatabase::openDatabaseConnection):
+ (WebCore::IDBServer::UniqueIDBDatabase::isVersionChangeInProgress):
+ (WebCore::IDBServer::UniqueIDBDatabase::performCurrentOpenOperation):
+ (WebCore::IDBServer::UniqueIDBDatabase::performCurrentDeleteOperation):
+ (WebCore::IDBServer::UniqueIDBDatabase::handleDatabaseOperations):
+ (WebCore::IDBServer::UniqueIDBDatabase::handleCurrentOperation):
+ (WebCore::IDBServer::UniqueIDBDatabase::handleDelete):
+ (WebCore::IDBServer::UniqueIDBDatabase::invokeOperationAndTransactionTimer):
+ (WebCore::IDBServer::UniqueIDBDatabase::operationAndTransactionTimerFired):
+ (WebCore::IDBServer::UniqueIDBDatabase::maybeDeleteDatabase): Deleted.
+ * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
2015-12-17 Brent Fulgham <[email protected]>
[Win] Prevent flashing/strobing repaints on certain hardware
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBServerOperation.cpp (194240 => 194241)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBServerOperation.cpp 2015-12-17 23:02:25 UTC (rev 194240)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBServerOperation.cpp 2015-12-17 23:29:53 UTC (rev 194241)
@@ -28,6 +28,7 @@
#if ENABLE(INDEXED_DATABASE)
+#include "IDBResultData.h"
#include <wtf/RefCounted.h>
namespace WebCore {
@@ -54,6 +55,22 @@
return m_requestData.isDeleteRequest();
}
+void IDBServerOperation::notifyDeleteRequestBlocked(uint64_t currentVersion)
+{
+ ASSERT(isDeleteRequest());
+ ASSERT(!m_notifiedDeleteRequestBlocked);
+
+ m_connection.notifyOpenDBRequestBlocked(m_requestData.requestIdentifier(), currentVersion, 0);
+ m_notifiedDeleteRequestBlocked = true;
+}
+
+void IDBServerOperation::notifyDidDeleteDatabase(const IDBDatabaseInfo& info)
+{
+ ASSERT(isDeleteRequest());
+
+ m_connection.didDeleteDatabase(IDBResultData::deleteDatabaseSuccess(m_requestData.requestIdentifier(), info));
+}
+
} // namespace IDBServer
} // namespace WebCore
Modified: trunk/Source/WebCore/Modules/indexeddb/server/IDBServerOperation.h (194240 => 194241)
--- trunk/Source/WebCore/Modules/indexeddb/server/IDBServerOperation.h 2015-12-17 23:02:25 UTC (rev 194240)
+++ trunk/Source/WebCore/Modules/indexeddb/server/IDBServerOperation.h 2015-12-17 23:29:53 UTC (rev 194241)
@@ -34,6 +34,9 @@
#include <wtf/RefCounted.h>
namespace WebCore {
+
+class IDBDatabaseInfo;
+
namespace IDBServer {
class IDBServerOperation : public RefCounted<IDBServerOperation> {
@@ -46,11 +49,17 @@
bool isOpenRequest() const;
bool isDeleteRequest() const;
+ void notifyDeleteRequestBlocked(uint64_t currentVersion);
+ void notifyDidDeleteDatabase(const IDBDatabaseInfo&);
+ bool hasNotifiedDeleteRequestBlocked() const { return m_notifiedDeleteRequestBlocked; }
+
private:
IDBServerOperation(IDBConnectionToClient&, const IDBRequestData&);
IDBConnectionToClient& m_connection;
IDBRequestData m_requestData;
+
+ bool m_notifiedDeleteRequestBlocked { false };
};
} // namespace IDBServer
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (194240 => 194241)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2015-12-17 23:02:25 UTC (rev 194240)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp 2015-12-17 23:29:53 UTC (rev 194241)
@@ -51,6 +51,15 @@
{
}
+UniqueIDBDatabase::~UniqueIDBDatabase()
+{
+ LOG(IndexedDB, "UniqueIDBDatabase::~UniqueIDBDatabase()");
+ ASSERT(!hasAnyPendingCallbacks());
+ ASSERT(m_inProgressTransactions.isEmpty());
+ ASSERT(m_pendingTransactions.isEmpty());
+ ASSERT(m_openDatabaseConnections.isEmpty());
+}
+
const IDBDatabaseInfo& UniqueIDBDatabase::info() const
{
RELEASE_ASSERT(m_databaseInfo);
@@ -60,9 +69,9 @@
void UniqueIDBDatabase::openDatabaseConnection(IDBConnectionToClient& connection, const IDBRequestData& requestData)
{
auto operation = IDBServerOperation::create(connection, requestData);
- m_pendingOpenDatabaseOperations.append(WTF::move(operation));
+ m_pendingDatabaseOperations.append(WTF::move(operation));
- // An open operation is already in progress, so this one has to wait.
+ // An open operation is already in progress, so we can't possibly handle this one yet.
if (m_isOpeningBackingStore)
return;
@@ -83,39 +92,14 @@
|| !m_countCallbacks.isEmpty();
}
-bool UniqueIDBDatabase::maybeDeleteDatabase(IDBServerOperation* newestDeleteOperation)
+bool UniqueIDBDatabase::isVersionChangeInProgress()
{
- ASSERT(isMainThread());
- LOG(IndexedDB, "(main) UniqueIDBDatabase::maybeDeleteDatabase");
+#ifndef NDEBUG
+ if (m_versionChangeTransaction)
+ ASSERT(m_versionChangeDatabaseConnection);
+#endif
- if (hasAnyOpenConnections() || !m_closePendingDatabaseConnections.isEmpty()) {
- // Exactly once, notify all open connections of the pending deletion.
- if (!m_hasNotifiedConnectionsOfDelete) {
- notifyConnectionsOfVersionChange(0);
- m_hasNotifiedConnectionsOfDelete = true;
- }
-
- if (newestDeleteOperation)
- newestDeleteOperation->connection().notifyOpenDBRequestBlocked(newestDeleteOperation->requestData().requestIdentifier(), m_databaseInfo->version(), 0);
-
- return false;
- }
-
- ASSERT(!hasAnyPendingCallbacks());
- ASSERT(m_inProgressTransactions.isEmpty());
- ASSERT(m_pendingTransactions.isEmpty());
- ASSERT(m_openDatabaseConnections.isEmpty());
- ASSERT(m_pendingOpenDatabaseOperations.isEmpty());
-
- for (auto& operation : m_pendingDeleteDatabaseOperations) {
- ASSERT(m_databaseInfo);
- ASSERT(operation->isDeleteRequest());
- operation->connection().didDeleteDatabase(IDBResultData::deleteDatabaseSuccess(operation->requestData().requestIdentifier(), *m_databaseInfo));
- }
-
- m_server.deleteUniqueIDBDatabase(*this);
-
- return true;
+ return m_versionChangeDatabaseConnection;
}
void UniqueIDBDatabase::performCurrentOpenOperation()
@@ -125,6 +109,14 @@
ASSERT(m_currentOperation);
ASSERT(m_currentOperation->isOpenRequest());
+ // If we previously started a version change operation but were blocked by having open connections,
+ // we might now be unblocked.
+ if (m_versionChangeDatabaseConnection) {
+ if (!m_versionChangeTransaction && !hasAnyOpenConnections())
+ startVersionChangeTransaction();
+ return;
+ }
+
// 3.3.1 Opening a database
// If requested version is undefined, then let requested version be 1 if db was created in the previous step,
// or the current version of db otherwise.
@@ -176,36 +168,77 @@
{
LOG(IndexedDB, "(main) UniqueIDBDatabase::performCurrentDeleteOperation");
- // Not used yet.
+ ASSERT(m_databaseInfo);
+ ASSERT(m_currentOperation);
+ ASSERT(m_currentOperation->isDeleteRequest());
+
+ if (hasAnyOpenConnections()) {
+ // Exactly once, notify all open connections of the pending deletion.
+ if (!m_hasNotifiedConnectionsOfDelete) {
+ notifyConnectionsOfVersionChange(0);
+ m_hasNotifiedConnectionsOfDelete = true;
+ }
+
+ if (!m_currentOperation->hasNotifiedDeleteRequestBlocked())
+ m_currentOperation->notifyDeleteRequestBlocked(m_databaseInfo->version());
+
+ return;
+ }
+
+ ASSERT(!hasAnyPendingCallbacks());
+ ASSERT(m_inProgressTransactions.isEmpty());
+ ASSERT(m_pendingTransactions.isEmpty());
+ ASSERT(m_openDatabaseConnections.isEmpty());
+
+ m_currentOperation->notifyDidDeleteDatabase(*m_databaseInfo);
+ m_currentOperation = nullptr;
+ m_hasNotifiedConnectionsOfDelete = false;
+ m_deletePending = false;
+
+ if (m_pendingDatabaseOperations.isEmpty())
+ m_server.deleteUniqueIDBDatabase(*this);
+ else
+ invokeOperationAndTransactionTimer();
}
void UniqueIDBDatabase::handleDatabaseOperations()
{
ASSERT(isMainThread());
- LOG(IndexedDB, "(main) UniqueIDBDatabase::handleDatabaseOperations");
+ LOG(IndexedDB, "(main) UniqueIDBDatabase::handleDatabaseOperations - There are %zu pending", m_pendingDatabaseOperations.size());
- // If a version change transaction is currently in progress, no new connections can be opened right now.
- // We will try again later.
- if (m_versionChangeDatabaseConnection)
+ if (m_pendingDatabaseOperations.isEmpty())
return;
- if (m_pendingOpenDatabaseOperations.isEmpty())
- return;
+ 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.first()->isDeleteRequest() && !m_hasNotifiedConnectionsOfDelete) {
+ m_hasNotifiedConnectionsOfDelete = true;
+ notifyConnectionsOfVersionChange(0);
+ }
+ }
- if (m_currentOperation)
- return;
+ m_currentOperation = m_pendingDatabaseOperations.takeFirst();
+ LOG(IndexedDB, "UniqueIDBDatabase::handleDatabaseOperations - Popped an operation, now there are %zu pending", m_pendingDatabaseOperations.size());
- m_currentOperation = m_pendingOpenDatabaseOperations.takeFirst();
+ handleCurrentOperation();
+}
- // FIXME: Once handleDatabaseOperations also handles delete operations, remove this ASSERT.
- ASSERT(m_currentOperation->isOpenRequest());
+void UniqueIDBDatabase::handleCurrentOperation()
+{
+ ASSERT(m_currentOperation);
+ RefPtr<UniqueIDBDatabase> protector(this);
+
if (m_currentOperation->isOpenRequest())
performCurrentOpenOperation();
else if (m_currentOperation->isDeleteRequest())
performCurrentDeleteOperation();
else
ASSERT_NOT_REACHED();
+
+ if (!m_currentOperation)
+ invokeOperationAndTransactionTimer();
}
bool UniqueIDBDatabase::hasAnyOpenConnections() const
@@ -256,18 +289,8 @@
{
LOG(IndexedDB, "(main) UniqueIDBDatabase::handleDelete");
- auto operation = IDBServerOperation::create(connection, requestData);
- auto* rawOperation = &operation.get();
- m_pendingDeleteDatabaseOperations.append(WTF::move(operation));
-
- // If a different request has already come in to delete this database, there's nothing left to do.
- // A delete is already in progress, and this request will be handled along with all the rest.
- if (m_deletePending)
- return;
-
- m_deletePending = true;
-
- maybeDeleteDatabase(rawOperation);
+ m_pendingDatabaseOperations.append(IDBServerOperation::create(connection, requestData));
+ handleDatabaseOperations();
}
void UniqueIDBDatabase::startVersionChangeTransaction()
@@ -937,6 +960,7 @@
void UniqueIDBDatabase::invokeOperationAndTransactionTimer()
{
+ LOG(IndexedDB, "UniqueIDBDatabase::invokeOperationAndTransactionTimer()");
if (!m_operationAndTransactionTimer.isActive())
m_operationAndTransactionTimer.startOneShot(0);
}
@@ -945,19 +969,14 @@
{
LOG(IndexedDB, "(main) UniqueIDBDatabase::operationAndTransactionTimerFired");
- handleDatabaseOperations();
+ // The current operation might require multiple attempts to handle, so try to
+ // make further progress on it now.
+ if (m_currentOperation)
+ handleCurrentOperation();
- if (m_deletePending && maybeDeleteDatabase(nullptr))
- return;
+ if (!m_currentOperation)
+ handleDatabaseOperations();
- // If the database was not deleted in the previous step, try to run a transaction now.
- if (m_pendingTransactions.isEmpty()) {
- if (!hasAnyOpenConnections() && m_currentOperation) {
- startVersionChangeTransaction();
- return;
- }
- }
-
bool hadDeferredTransactions = false;
auto transaction = takeNextRunnableTransaction(hadDeferredTransactions);
Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h (194240 => 194241)
--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2015-12-17 23:02:25 UTC (rev 194240)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h 2015-12-17 23:29:53 UTC (rev 194241)
@@ -71,6 +71,8 @@
return adoptRef(*new UniqueIDBDatabase(server, identifier));
}
+ ~UniqueIDBDatabase();
+
void openDatabaseConnection(IDBConnectionToClient&, const IDBRequestData&);
const IDBDatabaseInfo& info() const;
@@ -105,15 +107,16 @@
UniqueIDBDatabase(IDBServer&, const IDBDatabaseIdentifier&);
void handleDatabaseOperations();
+ void handleCurrentOperation();
void performCurrentOpenOperation();
void performCurrentDeleteOperation();
void addOpenDatabaseConnection(Ref<UniqueIDBDatabaseConnection>&&);
- bool maybeDeleteDatabase(IDBServerOperation*);
bool hasAnyOpenConnections() const;
void startVersionChangeTransaction();
void notifyConnectionsOfVersionChangeForUpgrade();
void notifyConnectionsOfVersionChange(uint64_t requestedVersion);
+ bool isVersionChangeInProgress();
void activateTransactionInBackingStore(UniqueIDBDatabaseTransaction&);
void inProgressTransactionCompleted(const IDBResourceIdentifier&);
@@ -173,8 +176,7 @@
IDBServer& m_server;
IDBDatabaseIdentifier m_identifier;
- Deque<Ref<IDBServerOperation>> m_pendingOpenDatabaseOperations;
- Deque<Ref<IDBServerOperation>> m_pendingDeleteDatabaseOperations;
+ Deque<Ref<IDBServerOperation>> m_pendingDatabaseOperations;
RefPtr<IDBServerOperation> m_currentOperation;
HashSet<RefPtr<UniqueIDBDatabaseConnection>> m_openDatabaseConnections;
