Fix flaky tests in http/tests/cache/disk-cache

https://bugs.webkit.org/show_bug.cgi?id=203822

Reviewed by Youenn Fablet.

Source/WebKit:

Right now tests that set the cache model do so using a race condition.  I think this will fix the flakyness we've observed.

* NetworkProcess/NetworkProcess.cpp:
(WebKit::NetworkProcess::setCacheModelSynchronouslyForTesting):
* NetworkProcess/NetworkProcess.h:
* NetworkProcess/NetworkProcess.messages.in:
* UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
(WKWebsiteDataStoreSetCacheModelSynchronouslyForTesting):
* UIProcess/API/C/WKWebsiteDataStoreRef.h:
* UIProcess/WebProcessPool.cpp:
(WebKit::WebProcessPool::setCacheModelSynchronouslyForTesting):
* UIProcess/WebProcessPool.h:
* UIProcess/WebsiteData/WebsiteDataStore.cpp:
(WebKit::WebsiteDataStore::setCacheModelSynchronouslyForTesting):
* UIProcess/WebsiteData/WebsiteDataStore.h:

Tools:

* WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
(WTR::InjectedBundle::setCacheModel):
* WebKitTestRunner/TestInvocation.cpp:
(WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
(WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):

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

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index 5e63418..1e7bc7b 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,5 +1,28 @@
 2019-11-08  Alex Christensen  <achristensen@webkit.org>
 
+        Fix flaky tests in http/tests/cache/disk-cache
+        https://bugs.webkit.org/show_bug.cgi?id=203822
+
+        Reviewed by Youenn Fablet.
+
+        Right now tests that set the cache model do so using a race condition.  I think this will fix the flakyness we've observed.
+
+        * NetworkProcess/NetworkProcess.cpp:
+        (WebKit::NetworkProcess::setCacheModelSynchronouslyForTesting):
+        * NetworkProcess/NetworkProcess.h:
+        * NetworkProcess/NetworkProcess.messages.in:
+        * UIProcess/API/C/WKWebsiteDataStoreRef.cpp:
+        (WKWebsiteDataStoreSetCacheModelSynchronouslyForTesting):
+        * UIProcess/API/C/WKWebsiteDataStoreRef.h:
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::setCacheModelSynchronouslyForTesting):
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebsiteData/WebsiteDataStore.cpp:
+        (WebKit::WebsiteDataStore::setCacheModelSynchronouslyForTesting):
+        * UIProcess/WebsiteData/WebsiteDataStore.h:
+
+2019-11-08  Alex Christensen  <achristensen@webkit.org>
+
         Revert some changes accidentally committed with r252257
         https://bugs.webkit.org/show_bug.cgi?id=202178
 

diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
index 4f8f0aa..5f23cc6 100644 (file)
--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp
+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
@@ -1993,6 +1993,12 @@ void NetworkProcess::continueDecidePendingDownloadDestination(DownloadID downloa
         downloadManager().continueDecidePendingDownloadDestination(downloadID, destination, WTFMove(sandboxExtensionHandle), allowOverwrite);
 }
 
+void NetworkProcess::setCacheModelSynchronouslyForTesting(CacheModel cacheModel, CompletionHandler<void()>&& completionHandler)
+{
+    setCacheModel(cacheModel, { });
+    completionHandler();
+}
+
 void NetworkProcess::setCacheModel(CacheModel cacheModel, String cacheStorageDirectory)
 {
     if (m_hasSetCacheModel && (cacheModel == m_cacheModel))

diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.h b/Source/WebKit/NetworkProcess/NetworkProcess.h
index adb5c97..7c48b18 100644 (file)
--- a/Source/WebKit/NetworkProcess/NetworkProcess.h
+++ b/Source/WebKit/NetworkProcess/NetworkProcess.h
@@ -406,6 +406,7 @@ private:
     void applicationWillEnterForeground();
 
     void setCacheModel(CacheModel, String overrideCacheStorageDirectory);
+    void setCacheModelSynchronouslyForTesting(CacheModel, CompletionHandler<void()>&&);
     void allowSpecificHTTPSCertificateForHost(const WebCore::CertificateInfo&, const String& host);
     void clearCacheForAllOrigins(uint32_t cachesToClear);
     void setAllowsAnySSLCertificateForWebSocket(bool, CompletionHandler<void()>&&);

diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.messages.in b/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
index 8895214..3677677 100644 (file)
--- a/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
+++ b/Source/WebKit/NetworkProcess/NetworkProcess.messages.in
@@ -69,7 +69,8 @@ messages -> NetworkProcess LegacyReceiver {
     AllowSpecificHTTPSCertificateForHost(WebCore::CertificateInfo certificate, String host)
     
     ClearCacheForAllOrigins(uint32_t cachesToClear)
-    SetCacheModel(enum:uint8_t WebKit::CacheModel cacheModel, String overrideCacheStorageDirectory);
+    SetCacheModel(enum:uint8_t WebKit::CacheModel cacheModel, String overrideCacheStorageDirectory)
+    SetCacheModelSynchronouslyForTesting(enum:uint8_t WebKit::CacheModel cacheModel) -> () Synchronous
 
     ProcessDidTransitionToBackground()
     ProcessDidTransitionToForeground()

diff --git a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp
index 7089f7e..7eb7577 100644 (file)
--- a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp
+++ b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp
@@ -682,3 +682,7 @@ void WKWebsiteDataStoreClearAdClickAttributionsThroughWebsiteDataRemoval(WKWebsi
     });
 }
 
+void WKWebsiteDataStoreSetCacheModelSynchronouslyForTesting(WKWebsiteDataStoreRef dataStoreRef, WKCacheModel cacheModel)
+{
+    WebKit::toImpl(dataStoreRef)->setCacheModelSynchronouslyForTesting(WebKit::toCacheModel(cacheModel));
+}

diff --git a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h
index 65385f8..95239a3 100644 (file)
--- a/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h
+++ b/Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.h
@@ -33,6 +33,8 @@
 extern "C" {
 #endif
 
+typedef uint32_t WKCacheModel;
+
 WK_EXPORT WKTypeID WKWebsiteDataStoreGetTypeID();
 
 WK_EXPORT WKWebsiteDataStoreRef WKWebsiteDataStoreGetDefaultDataStore();
@@ -151,6 +153,8 @@ WK_EXPORT void WKWebsiteDataStoreClearAllDeviceOrientationPermissions(WKWebsiteD
 typedef void (*WKWebsiteDataStoreClearAdClickAttributionsThroughWebsiteDataRemovalFunction)(void* functionContext);
 WK_EXPORT void WKWebsiteDataStoreClearAdClickAttributionsThroughWebsiteDataRemoval(WKWebsiteDataStoreRef dataStoreRef, void* context, WKWebsiteDataStoreClearAdClickAttributionsThroughWebsiteDataRemovalFunction callback);
 
+WK_EXPORT void WKWebsiteDataStoreSetCacheModelSynchronouslyForTesting(WKWebsiteDataStoreRef dataStoreRef, WKCacheModel cacheModel);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/Source/WebKit/UIProcess/WebProcessPool.cpp b/Source/WebKit/UIProcess/WebProcessPool.cpp
index ed950f4..0e37889 100644 (file)
--- a/Source/WebKit/UIProcess/WebProcessPool.cpp
+++ b/Source/WebKit/UIProcess/WebProcessPool.cpp
@@ -1574,6 +1574,14 @@ void WebProcessPool::setCacheModel(CacheModel cacheModel)
         m_networkProcess->send(Messages::NetworkProcess::SetCacheModel(cacheModel, { }), 0);
 }
 
+void WebProcessPool::setCacheModelSynchronouslyForTesting(CacheModel cacheModel)
+{
+    updateBackForwardCacheCapacity();
+
+    if (m_networkProcess)
+        m_networkProcess->sendSync(Messages::NetworkProcess::SetCacheModelSynchronouslyForTesting(cacheModel), { }, { });
+}
+
 void WebProcessPool::setDefaultRequestTimeoutInterval(double timeoutInterval)
 {
     sendToAllProcesses(Messages::WebProcess::SetDefaultRequestTimeoutInterval(timeoutInterval));

diff --git a/Source/WebKit/UIProcess/WebProcessPool.h b/Source/WebKit/UIProcess/WebProcessPool.h
index 8aab5df..9de2172 100644 (file)
--- a/Source/WebKit/UIProcess/WebProcessPool.h
+++ b/Source/WebKit/UIProcess/WebProcessPool.h
@@ -283,6 +283,8 @@ public:
     VisitedLinkStore& visitedLinkStore() { return m_visitedLinkStore.get(); }
 
     void setCacheModel(CacheModel);
+    void setCacheModelSynchronouslyForTesting(CacheModel);
+
 
     void setDefaultRequestTimeoutInterval(double);
 

diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
index 2df9d7f..bb98416 100644 (file)
--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp
@@ -2063,6 +2063,12 @@ uint64_t WebsiteDataStore::perThirdPartyOriginStorageQuota() const
     return WebCore::StorageQuotaManager::defaultThirdPartyQuotaFromPerOriginQuota(perOriginStorageQuota());
 }
 
+void WebsiteDataStore::setCacheModelSynchronouslyForTesting(CacheModel cacheModel)
+{
+    for (auto processPool : WebProcessPool::allProcessPools())
+        processPool->setCacheModelSynchronouslyForTesting(cacheModel);
+}
+
 #if !PLATFORM(COCOA)
 WebsiteDataStoreParameters WebsiteDataStore::parameters()
 {

diff --git a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
index f9a21db..2784fe3 100644 (file)
--- a/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
+++ b/Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.h
@@ -78,6 +78,7 @@ class WebPageProxy;
 class WebProcessPool;
 class WebProcessProxy;
 class WebResourceLoadStatisticsStore;
+enum class CacheModel : uint8_t;
 enum class WebsiteDataFetchOption;
 enum class WebsiteDataType;
 struct WebsiteDataRecord;
@@ -138,6 +139,7 @@ public:
     void removeData(OptionSet<WebsiteDataType>, const Vector<WebsiteDataRecord>&, Function<void()>&& completionHandler);
 
     void getLocalStorageDetails(Function<void(Vector<LocalStorageDatabaseTracker::OriginDetails>&&)>&&);
+    void setCacheModelSynchronouslyForTesting(CacheModel);
 
 #if ENABLE(RESOURCE_LOAD_STATISTICS)
     void fetchDataForRegistrableDomains(OptionSet<WebsiteDataType>, OptionSet<WebsiteDataFetchOption>, const Vector<WebCore::RegistrableDomain>&, CompletionHandler<void(Vector<WebsiteDataRecord>&&, HashSet<WebCore::RegistrableDomain>&&)>&&);

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
index 6db7461..5304e14 100644 (file)
--- a/Tools/ChangeLog
+++ b/Tools/ChangeLog
@@ -1,3 +1,16 @@
+2019-11-08  Alex Christensen  <achristensen@webkit.org>
+
+        Fix flaky tests in http/tests/cache/disk-cache
+        https://bugs.webkit.org/show_bug.cgi?id=203822
+
+        Reviewed by Youenn Fablet.
+
+        * WebKitTestRunner/InjectedBundle/InjectedBundle.cpp:
+        (WTR::InjectedBundle::setCacheModel):
+        * WebKitTestRunner/TestInvocation.cpp:
+        (WTR::TestInvocation::didReceiveMessageFromInjectedBundle):
+        (WTR::TestInvocation::didReceiveSynchronousMessageFromInjectedBundle):
+
 2019-11-08  Aakash Jain  <aakash_jain@apple.com>
 
         Clicking on EWS status-bubble should open the results in new tab

diff --git a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
index 21e571e..7ea5358 100644 (file)
--- a/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
+++ b/Tools/WebKitTestRunner/InjectedBundle/InjectedBundle.cpp
@@ -925,7 +925,7 @@ void InjectedBundle::setCacheModel(int model)
 {
     WKRetainPtr<WKStringRef> messageName = adoptWK(WKStringCreateWithUTF8CString("SetCacheModel"));
     WKRetainPtr<WKUInt64Ref> messageBody = adoptWK(WKUInt64Create(model));
-    WKBundlePagePostMessage(page()->page(), messageName.get(), messageBody.get());
+    WKBundlePagePostSynchronousMessageForTesting(page()->page(), messageName.get(), messageBody.get(), nullptr);
 }
 
 bool InjectedBundle::shouldProcessWorkQueue() const

diff --git a/Tools/WebKitTestRunner/TestInvocation.cpp b/Tools/WebKitTestRunner/TestInvocation.cpp
index 3c6269c..87ae56b 100644 (file)
--- a/Tools/WebKitTestRunner/TestInvocation.cpp
+++ b/Tools/WebKitTestRunner/TestInvocation.cpp
@@ -546,13 +546,6 @@ void TestInvocation::didReceiveMessageFromInjectedBundle(WKStringRef messageName
         return;
     }
 
-    if (WKStringIsEqualToUTF8CString(messageName, "SetCacheModel")) {
-        ASSERT(WKGetTypeID(messageBody) == WKUInt64GetTypeID());
-        uint64_t model = WKUInt64GetValue(static_cast<WKUInt64Ref>(messageBody));
-        WKContextSetCacheModel(TestController::singleton().context(), model);
-        return;
-    }
-
     if (WKStringIsEqualToUTF8CString(messageName, "SetCustomPolicyDelegate")) {
         ASSERT(WKGetTypeID(messageBody) == WKDictionaryGetTypeID());
         WKDictionaryRef messageBodyDictionary = static_cast<WKDictionaryRef>(messageBody);
@@ -879,6 +872,13 @@ WKRetainPtr<WKTypeRef> TestInvocation::didReceiveSynchronousMessageFromInjectedB
         return result;
     }
 
+    if (WKStringIsEqualToUTF8CString(messageName, "SetCacheModel")) {
+        ASSERT(WKGetTypeID(messageBody) == WKUInt64GetTypeID());
+        uint64_t model = WKUInt64GetValue(static_cast<WKUInt64Ref>(messageBody));
+        WKWebsiteDataStoreSetCacheModelSynchronouslyForTesting(TestController::websiteDataStore(), model);
+        return nullptr;
+    }
+
     if (WKStringIsEqualToUTF8CString(messageName, "IsWorkQueueEmpty")) {
         bool isEmpty = TestController::singleton().workQueueManager().isWorkQueueEmpty();
         WKRetainPtr<WKTypeRef> result = adoptWK(WKBooleanCreate(isEmpty));