Attachment 357586 Details for Bug 192796 – Patch

[patch] Patch

bug-192796-20181218111744.patch (text/plain), 13.73 KB, created by Sihui Liu on 2018-12-18 11:17:44 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Sihui Liu

Created: 2018-12-18 11:17:44 PST

Size: 13.73 KB

patch

obsolete

>Subversion Revision: 239338
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 979627908835a511fa6c904d8b2300da8c26221c..d0eef7ff26d6ae728e23c16ba855a44da2d39231 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,44 @@
>+2018-12-18  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Clean up IndexedDB files between tests
>+        https://bugs.webkit.org/show_bug.cgi?id=192796
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        We should clean up the IndexedDB files between tests to make sure each test is independent of others.
>+
>+        This patch also fixes some issues in IDB.
>+
>+        Covered by existing tests.
>+
>+        * Modules/indexeddb/server/IDBServer.cpp:
>+        (WebCore::IDBServer::IDBServer::closeAndDeleteDatabasesModifiedSince):
>+        We should shut down all open databases instead of databases from open database connections before deleting 
>+        files, because database starts accessing files before connection to database is established. Deleting a file in 
>+        use could cause simulated crash.
>+
>+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
>+        (WebCore::IDBServer::UniqueIDBDatabase::shutdownForClose):
>+        We should shutdown database after tasks in queue are completed, because tasks have pointer of UniqueIDBDatabase 
>+        and UniqueIDBDatabase can be destructed after shutdown.
>+
>+        (WebCore::IDBServer::UniqueIDBDatabase::didDeleteBackingStore):
>+        didDeleteBackingStore can be posted to main thread after immediateCloseForUserDelete, and timer are not 
>+        suppposed be invoked during the hard close.
>+
>+        (WebCore::IDBServer::UniqueIDBDatabase::handleDatabaseOperations):
>+        Tasks like didOpenBackingStore could be posted to main thread after immediateCloseForUserDelete, but we know the
>+        backing store will be deleted soon, so no need to hanlde any database operation. 
>+
>+        (WebCore::IDBServer::UniqueIDBDatabase::performPrefetchCursor):
>+        performPrefetchCursor was not aware of the state of UniqueIDBDatabase, so it might keep posting prefetch task 
>+        to queue, which should be stopped when the database is being deleted.
>+
>+        (WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
>+        immediateCloseForUserDelete does not handle transactions that are in the process of commit or abort, those
>+        transactions may need m_objectStoreTransactionCounts and m_objectStoreWriteTransactions in transactionCompleted,
>+        so they do not need to be cleared here.
>+
> 2018-12-18  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Calling setValue() while typing should invoke -textDidChangeInTextField in the injected bundle
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 6c8822de09151b025fc2382e5a810e45b84c0904..596a8a72cd86d526a19c41604fa62e2f80f8c9d2 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,14 @@
>+2018-12-18  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Clean up IndexedDB files between tests
>+        https://bugs.webkit.org/show_bug.cgi?id=192796
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
>+        (WKWebsiteDataStoreRemoveAllIndexedDatabasesSync):
>+        * UIProcess/API/C/WKWebsiteDataStoreRef.h:
>+
> 2018-12-18  Chris Dumez  <cdumez@apple.com>
> 
>         Regression(r239182) SuspendedPage's process reuse for link navigation optimization sometimes broken
>diff --git a/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp b/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
>index cb17ba1d579b6d2599ff4c5bcefdfe2faef4bea0..a2cfa99c3636be82f085c4b71525e0e5997df9ed 100644
>--- a/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
>+++ b/Source/WebCore/Modules/indexeddb/server/IDBServer.cpp
>@@ -509,8 +509,8 @@ void IDBServer::closeAndDeleteDatabasesModifiedSince(WallTime modificationTime,
>     }
> 
>     HashSet<UniqueIDBDatabase*> openDatabases;
>-    for (auto* connection : m_databaseConnections.values())
>-        openDatabases.add(connection->database());
>+    for (auto& database : m_uniqueIDBDatabaseMap.values())
>+        openDatabases.add(database.get());
> 
>     for (auto& database : openDatabases)
>         database->immediateCloseForUserDelete();
>diff --git a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
>index 4e2a2df6b7a37777229cc8f2d5a44985b668c673..cf20e9c1fd7a98dd3855dbbc7d2d87e604f8d804 100644
>--- a/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
>+++ b/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
>@@ -294,7 +294,10 @@ void UniqueIDBDatabase::shutdownForClose()
>     m_backingStoreSupportsSimultaneousTransactions = false;
>     m_backingStoreIsEphemeral = false;
> 
>-    ASSERT(m_databaseQueue.isEmpty());
>+    if (!m_databaseQueue.isEmpty()) {
>+        postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::shutdownForClose));
>+        return;
>+    }
>     m_databaseQueue.kill();
> 
>     postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didShutdownForClose));
>@@ -336,14 +339,18 @@ void UniqueIDBDatabase::didDeleteBackingStore(uint64_t deletedVersion)
>     }
> 
>     m_deleteBackingStoreInProgress = false;
>-    invokeOperationAndTransactionTimer();
>+
>+    if (!m_hardClosedForUserDelete)
>+        invokeOperationAndTransactionTimer();
> }
> 
> void UniqueIDBDatabase::handleDatabaseOperations()
> {
>     ASSERT(isMainThread());
>     LOG(IndexedDB, "(main) UniqueIDBDatabase::handleDatabaseOperations - There are %u pending", m_pendingOpenDBRequests.size());
>-    ASSERT(!m_hardClosedForUserDelete);
>+
>+    if (m_hardClosedForUserDelete)
>+        return;
> 
>     if (m_deleteBackingStoreInProgress)
>         return;
>@@ -1278,6 +1285,9 @@ void UniqueIDBDatabase::performPrefetchCursor(const IDBResourceIdentifier& trans
>     ASSERT(m_cursorPrefetches.contains(cursorIdentifier));
>     LOG(IndexedDB, "(db) UniqueIDBDatabase::performPrefetchCursor");
> 
>+    if (m_owningPointerForClose)
>+        return;
>+
>     if (m_backingStore->prefetchCursor(transactionIdentifier, cursorIdentifier))
>         postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performPrefetchCursor, transactionIdentifier, cursorIdentifier));
>     else
>@@ -1818,8 +1828,6 @@ void UniqueIDBDatabase::immediateCloseForUserDelete()
>     for (auto& transaction : m_pendingTransactions)
>         transaction->databaseConnection().deleteTransaction(*transaction);
>     m_pendingTransactions.clear();
>-    m_objectStoreTransactionCounts.clear();
>-    m_objectStoreWriteTransactions.clear();
> 
>     // Error out all pending callbacks
>     IDBError error = IDBError::userDeleteError();
>diff --git a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp
>index a3e3468256bb67008295fc13f072df379b70012c..c6be6f51838741000e71382ada6d48ecd73fcd91 100644
>--- a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp
>+++ b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp
>@@ -511,10 +511,13 @@ void WKWebsiteDataStoreRemoveFetchCacheForOrigin(WKWebsiteDataStoreRef dataStore
>     });
> }
> 
>-void WKWebsiteDataStoreRemoveAllIndexedDatabases(WKWebsiteDataStoreRef dataStoreRef)
>+void WKWebsiteDataStoreRemoveAllIndexedDatabases(WKWebsiteDataStoreRef dataStoreRef, void* context, WKWebsiteDataStoreRemoveAllIndexedDatabasesCallback callback)
> {
>     OptionSet<WebKit::WebsiteDataType> dataTypes = WebKit::WebsiteDataType::IndexedDBDatabases;
>-    WebKit::toImpl(dataStoreRef)->websiteDataStore().removeData(dataTypes, -WallTime::infinity(), [] { });
>+    WebKit::toImpl(dataStoreRef)->websiteDataStore().removeData(dataTypes, -WallTime::infinity(), [context, callback] {
>+        if (callback)
>+            callback(context);
>+    });
> }
> 
> void WKWebsiteDataStoreRemoveAllServiceWorkerRegistrations(WKWebsiteDataStoreRef dataStoreRef, void* context, WKWebsiteDataStoreRemoveAllServiceWorkerRegistrationsCallback callback)
>diff --git a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h
>index 01b98e19328b140ed7220d3c58aef3dcb7ed9e68..93e52185b5bd7645216522461e58874c31e8971e 100644
>--- a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h
>+++ b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h
>@@ -103,7 +103,8 @@ WK_EXPORT void WKWebsiteDataStoreRemoveAllFetchCaches(WKWebsiteDataStoreRef data
> typedef void (*WKWebsiteDataStoreRemoveAllServiceWorkerRegistrationsCallback)(void* functionContext);
> WK_EXPORT void WKWebsiteDataStoreRemoveAllServiceWorkerRegistrations(WKWebsiteDataStoreRef dataStoreRef, void* context, WKWebsiteDataStoreRemoveAllServiceWorkerRegistrationsCallback callback);
> 
>-WK_EXPORT void WKWebsiteDataStoreRemoveAllIndexedDatabases(WKWebsiteDataStoreRef dataStoreRef);
>+typedef void (*WKWebsiteDataStoreRemoveAllIndexedDatabasesCallback)(void* functionContext);
>+WK_EXPORT void WKWebsiteDataStoreRemoveAllIndexedDatabases(WKWebsiteDataStoreRef dataStoreRef, void* context, WKWebsiteDataStoreRemoveAllIndexedDatabasesCallback callback);
> 
> typedef void (*WKWebsiteDataStoreGetFetchCacheOriginsFunction)(WKArrayRef, void*);
> WK_EXPORT void WKWebsiteDataStoreGetFetchCacheOrigins(WKWebsiteDataStoreRef dataStoreRef, void* context, WKWebsiteDataStoreGetFetchCacheOriginsFunction function);
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 1525bb6ddb270f8ff6c5ee2b183c41a66e143593..af6ef441ca36e20ccb33eede4f849602a67936d8 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-12-18  Sihui Liu  <sihui_liu@apple.com>
>+
>+        Clean up IndexedDB files between tests
>+        https://bugs.webkit.org/show_bug.cgi?id=192796
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * DumpRenderTree/mac/DumpRenderTree.mm:
>+        (runTest):
>+        * WebKitTestRunner/TestController.cpp:
>+        (WTR::TestController::resetStateToConsistentValues):
>+        (WTR::RemoveAllIndexedDatabasesCallbackContext::RemoveAllIndexedDatabasesCallbackContext):
>+        (WTR::RemoveAllIndexedDatabasesCallback):
>+        (WTR::TestController::ClearIndexedDatabases):
>+        * WebKitTestRunner/TestController.h:
>+
> 2018-12-18  Chris Dumez  <cdumez@apple.com>
> 
>         Regression(r239182) SuspendedPage's process reuse for link navigation optimization sometimes broken
>diff --git a/Tools/DumpRenderTree/mac/DumpRenderTree.mm b/Tools/DumpRenderTree/mac/DumpRenderTree.mm
>index 5de612c59f367c4d094c8fb3178286ed3bdcbff0..3cb77ff6909e86abfd02e78f5517ef683945e205 100644
>--- a/Tools/DumpRenderTree/mac/DumpRenderTree.mm
>+++ b/Tools/DumpRenderTree/mac/DumpRenderTree.mm
>@@ -2012,6 +2012,8 @@ static void runTest(const string& inputLine)
>     sizeWebViewForCurrentTest();
>     gTestRunner->setIconDatabaseEnabled(false);
>     gTestRunner->clearAllApplicationCaches();
>+    gTestRunner->clearAllDatabases();
>+    gTestRunner->setIDBPerOriginQuota(50 * MB);
> 
>     if (disallowedURLs)
>         CFSetRemoveAllValues(disallowedURLs);
>diff --git a/Tools/WebKitTestRunner/TestController.cpp b/Tools/WebKitTestRunner/TestController.cpp
>index 2f98ed32396da759f6ffdeddeadc935d11e558d2..d4ded0075072dbb5e8d284b5fc7e7813a366db3c 100644
>--- a/Tools/WebKitTestRunner/TestController.cpp
>+++ b/Tools/WebKitTestRunner/TestController.cpp
>@@ -867,6 +867,9 @@ bool TestController::resetStateToConsistentValues(const TestOptions& options, Re
> 
>     WKContextClearCachedCredentials(TestController::singleton().context());
> 
>+    ClearIndexedDatabases();
>+    setIDBPerOriginQuota(50 * MB);
>+
>     clearServiceWorkerRegistrations();
>     clearDOMCaches();
> 
>@@ -2759,6 +2762,28 @@ void TestController::setIDBPerOriginQuota(uint64_t quota)
>     WKContextSetIDBPerOriginQuota(platformContext(), quota);
> }
> 
>+struct RemoveAllIndexedDatabasesCallbackContext {
>+    explicit RemoveAllIndexedDatabasesCallbackContext(TestController& controller)
>+        : testController(controller)
>+        {
>+        }
>+    TestController& testController;
>+    bool done { false };
>+};
>+static void RemoveAllIndexedDatabasesCallback(void* userData)
>+{
>+    auto* context = static_cast<RemoveAllIndexedDatabasesCallbackContext*>(userData);
>+    context->done = true;
>+    context->testController.notifyDone();
>+}
>+void TestController::ClearIndexedDatabases()
>+{
>+    auto websiteDataStore = WKContextGetWebsiteDataStore(platformContext());
>+    RemoveAllIndexedDatabasesCallbackContext context(*this);
>+    WKWebsiteDataStoreRemoveAllIndexedDatabases(websiteDataStore, &context, RemoveAllIndexedDatabasesCallback);
>+    runUntil(context.done, noTimeout);
>+}
>+
> struct FetchCacheOriginsCallbackContext {
>     FetchCacheOriginsCallbackContext(TestController& controller, WKStringRef origin)
>         : testController(controller)
>diff --git a/Tools/WebKitTestRunner/TestController.h b/Tools/WebKitTestRunner/TestController.h
>index 13a39c6905f0d87ba87b9db57f8dad5c17ac71bc..acb4b98876d2d5afb5845f031b8ae816c49530d2 100644
>--- a/Tools/WebKitTestRunner/TestController.h
>+++ b/Tools/WebKitTestRunner/TestController.h
>@@ -242,6 +242,8 @@ public:
> 
>     void removeAllSessionCredentials();
> 
>+    void ClearIndexedDatabases();
>+
>     void clearServiceWorkerRegistrations();
> 
>     void clearDOMCache(WKStringRef origin);
>diff --git a/Tools/WebKitTestRunner/TestInvocation.cpp b/Tools/WebKitTestRunner/TestInvocation.cpp
>index 1a91eb28434d4ed7d1b961220063850540ad782b..c037301583f33b05c8a4115e73c5ced94ba37563 100644
>--- a/Tools/WebKitTestRunner/TestInvocation.cpp
>+++ b/Tools/WebKitTestRunner/TestInvocation.cpp
>@@ -903,7 +903,7 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
>     }
>     
>     if (WKStringIsEqualToUTF8CString(messageName, "DeleteAllIndexedDatabases")) {
>-        WKWebsiteDataStoreRemoveAllIndexedDatabases(WKContextGetWebsiteDataStore(TestController::singleton().context()));
>+        WKWebsiteDataStoreRemoveAllIndexedDatabases(WKContextGetWebsiteDataStore(TestController::singleton().context()), nullptr, { });
>         return nullptr;
>     }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 192796: 357526 | 357586 | 357620 | 359785 | 359820 | 359821