Attachment 361661 Details for Bug 194413 – Patch

[patch] Patch

bug-194413-20190210230946.patch (text/plain), 8.91 KB, created by Sihui Liu on 2019-02-10 23:09:47 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2019-02-10 23:09:47 PST

Size: 8.91 KB

patch

obsolete

>Subversion Revision: 241247
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 50320c2098d5ae6c2b825ce142d640a5d73fa98c..a037dc703c691446d8a667b2bf2b69e8afdf4cce 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,27 @@
>+2019-02-10  Sihui Liu  <sihui_liu@apple.com>
>+
>+        REGRESSION: [ Mac Debug WK2 ] Layout Test storage/indexeddb/key-type-infinity-private.html is a flaky crash
>+        https://bugs.webkit.org/show_bug.cgi?id=194413
>+        <rdar://problem/47897254>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        IDB clients expected transaction operations to be executed in order, but in UniqueIDBDatabase::immediateCloseForUserDelete, callbacks in callback map were errored out randomly.
>+        This patch added a callback queue to UniqueIDBDatabase to make sure callbacks will be called in the same order
>+        as IDB Server receives the request.
>+
>+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
>+        (WebCore::IDBServer::UniqueIDBDatabase::storeCallbackOrFireError):
>+        (WebCore::IDBServer::UniqueIDBDatabase::didPerformPutOrAdd):
>+        (WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
>+        (WebCore::IDBServer::UniqueIDBDatabase::performErrorCallback):
>+        (WebCore::IDBServer::UniqueIDBDatabase::performKeyDataCallback):
>+        (WebCore::IDBServer::UniqueIDBDatabase::performGetResultCallback):
>+        (WebCore::IDBServer::UniqueIDBDatabase::performGetAllResultsCallback):
>+        (WebCore::IDBServer::UniqueIDBDatabase::performCountCallback):
>+        (WebCore::IDBServer::UniqueIDBDatabase::forgetErrorCallback):
>+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
>+
> 2019-02-09  Darin Adler  <darin@apple.com>
> 
>         Eliminate unnecessary String temporaries by using StringConcatenateNumbers
>diff --git a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
>index dfad78734b46e410698e277c8ead27b7953fcf2a..32e486cff1ea5b1cf5d962a8162f3dfa99edff24 100644
>--- a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
>+++ b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
>@@ -423,6 +423,7 @@ uint64_t UniqueIDBDatabase::storeCallbackOrFireError(ErrorCallback&& callback)
>     uint64_t identifier = generateUniqueCallbackIdentifier();
>     ASSERT(!m_errorCallbacks.contains(identifier));
>     m_errorCallbacks.add(identifier, WTFMove(callback));
>+    m_callbackQueue.append(identifier);
>     return identifier;
> }
> 
>@@ -436,6 +437,7 @@ uint64_t UniqueIDBDatabase::storeCallbackOrFireError(KeyDataCallback&& callback)
>     uint64_t identifier = generateUniqueCallbackIdentifier();
>     ASSERT(!m_keyDataCallbacks.contains(identifier));
>     m_keyDataCallbacks.add(identifier, WTFMove(callback));
>+    m_callbackQueue.append(identifier);
>     return identifier;
> }
> 
>@@ -449,6 +451,7 @@ uint64_t UniqueIDBDatabase::storeCallbackOrFireError(GetResultCallback&& callbac
>     uint64_t identifier = generateUniqueCallbackIdentifier();
>     ASSERT(!m_getResultCallbacks.contains(identifier));
>     m_getResultCallbacks.add(identifier, WTFMove(callback));
>+    m_callbackQueue.append(identifier);
>     return identifier;
> }
> 
>@@ -462,6 +465,7 @@ uint64_t UniqueIDBDatabase::storeCallbackOrFireError(GetAllResultsCallback&& cal
>     uint64_t identifier = generateUniqueCallbackIdentifier();
>     ASSERT(!m_getAllResultsCallbacks.contains(identifier));
>     m_getAllResultsCallbacks.add(identifier, WTFMove(callback));
>+    m_callbackQueue.append(identifier);
>     return identifier;
> }
> 
>@@ -475,6 +479,7 @@ uint64_t UniqueIDBDatabase::storeCallbackOrFireError(CountCallback&& callback)
>     uint64_t identifier = generateUniqueCallbackIdentifier();
>     ASSERT(!m_countCallbacks.contains(identifier));
>     m_countCallbacks.add(identifier, WTFMove(callback));
>+    m_callbackQueue.append(identifier);
>     return identifier;
> }
> 
>@@ -1067,6 +1072,7 @@ void UniqueIDBDatabase::performPutOrAdd(uint64_t callbackIdentifier, const IDBRe
> 
> void UniqueIDBDatabase::didPerformPutOrAdd(uint64_t callbackIdentifier, const IDBError& error, const IDBKeyData& resultKey)
> {
>+    WTFLogAlways("sihuil: (server)[%p]UniqueIDBDatabase::didPerformPutOrAdd with callback %llu", this, callbackIdentifier);
>     ASSERT(isMainThread());
>     LOG(IndexedDB, "(main) UniqueIDBDatabase::didPerformPutOrAdd");
> 
>@@ -1828,18 +1834,23 @@ void UniqueIDBDatabase::immediateCloseForUserDelete()
>     IDBError error = IDBError::userDeleteError();
>     IDBKeyData keyData;
>     IDBGetResult getResult;
>-
>-    for (auto identifier : copyToVector(m_errorCallbacks.keys()))
>-        performErrorCallback(identifier, error);
>-
>-    for (auto identifier : copyToVector(m_keyDataCallbacks.keys()))
>-        performKeyDataCallback(identifier, error, keyData);
>-
>-    for (auto identifier : copyToVector(m_getResultCallbacks.keys()))
>-        performGetResultCallback(identifier, error, getResult);
>-
>-    for (auto identifier : copyToVector(m_countCallbacks.keys()))
>-        performCountCallback(identifier, error, 0);
>+    IDBGetAllResult getAllResult;
>+
>+    while (!m_callbackQueue.isEmpty()) {
>+        auto identifier = m_callbackQueue.first();
>+        if (m_errorCallbacks.contains(identifier))
>+            performErrorCallback(identifier, error);
>+        else if (m_keyDataCallbacks.contains(identifier))
>+            performKeyDataCallback(identifier, error, keyData);
>+        else if (m_getResultCallbacks.contains(identifier))
>+            performGetResultCallback(identifier, error, getResult);
>+        else if (m_countCallbacks.contains(identifier))
>+            performCountCallback(identifier, error, 0);
>+        else if (m_getAllResultsCallbacks.contains(identifier))
>+            performGetAllResultsCallback(identifier, error, getAllResult);
>+        else
>+            ASSERT_NOT_REACHED();
>+    }
> 
>     // Error out all IDBOpenDBRequests
>     if (m_currentOpenDBRequest) {
>@@ -1880,45 +1891,62 @@ void UniqueIDBDatabase::performErrorCallback(uint64_t callbackIdentifier, const
> {
>     auto callback = m_errorCallbacks.take(callbackIdentifier);
>     ASSERT(callback || m_hardClosedForUserDelete);
>-    if (callback)
>+    if (callback) {
>         callback(error);
>+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
>+        m_callbackQueue.removeFirst();
>+    }
> }
> 
> void UniqueIDBDatabase::performKeyDataCallback(uint64_t callbackIdentifier, const IDBError& error, const IDBKeyData& resultKey)
> {
>     auto callback = m_keyDataCallbacks.take(callbackIdentifier);
>     ASSERT(callback || m_hardClosedForUserDelete);
>-    if (callback)
>+    if (callback) {
>         callback(error, resultKey);
>+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
>+        m_callbackQueue.removeFirst();
>+    }
> }
> 
> void UniqueIDBDatabase::performGetResultCallback(uint64_t callbackIdentifier, const IDBError& error, const IDBGetResult& resultData)
> {
>     auto callback = m_getResultCallbacks.take(callbackIdentifier);
>     ASSERT(callback || m_hardClosedForUserDelete);
>-    if (callback)
>+    if (callback) {
>         callback(error, resultData);
>+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
>+        m_callbackQueue.removeFirst();
>+    }
> }
> 
> void UniqueIDBDatabase::performGetAllResultsCallback(uint64_t callbackIdentifier, const IDBError& error, const IDBGetAllResult& resultData)
> {
>     auto callback = m_getAllResultsCallbacks.take(callbackIdentifier);
>     ASSERT(callback || m_hardClosedForUserDelete);
>-    if (callback)
>+    if (callback) {
>         callback(error, resultData);
>+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
>+        m_callbackQueue.removeFirst();
>+    }
> }
> 
> void UniqueIDBDatabase::performCountCallback(uint64_t callbackIdentifier, const IDBError& error, uint64_t count)
> {
>     auto callback = m_countCallbacks.take(callbackIdentifier);
>     ASSERT(callback || m_hardClosedForUserDelete);
>-    if (callback)
>+    if (callback) {
>         callback(error, count);
>+        ASSERT(m_callbackQueue.first() == callbackIdentifier);
>+        m_callbackQueue.removeFirst();
>+    }
> }
> 
> void UniqueIDBDatabase::forgetErrorCallback(uint64_t callbackIdentifier)
> {
>     ASSERT(m_errorCallbacks.contains(callbackIdentifier));
>+    ASSERT(m_callbackQueue.last() == callbackIdentifier);
>+    m_callbackQueue.removeLast();
>     m_errorCallbacks.remove(callbackIdentifier);
> }
> 
>diff --git a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h
>index 5e193a231881fa1c5edf9c8bdffdc094c2b584ae..83c0c908a54e59b463a0f670da9e192554e29977 100644
>--- a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h
>+++ b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h
>@@ -250,6 +250,7 @@ private:
>     HashMap<uint64_t, GetResultCallback> m_getResultCallbacks;
>     HashMap<uint64_t, GetAllResultsCallback> m_getAllResultsCallbacks;
>     HashMap<uint64_t, CountCallback> m_countCallbacks;
>+    Deque<uint64_t> m_callbackQueue;
> 
>     Timer m_operationAndTransactionTimer;
>

Actions: View | Formatted Diff | Diff

Attachments on bug 194413: 361444 | 361661 | 361667 | 361690