Implement testing mode for disk cache
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Apr 2017 18:32:47 +0000 (18:32 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 6 Apr 2017 18:32:47 +0000 (18:32 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170547

Reviewed by Andreas Kling.

Source/WebKit2:

Disable read timeouts and cache shrinking in TestRunner to eliminate potential sources of randomness.

Cache directories are deleted by TestRunner so lack of shrinking does not consume the disk.

This is enabled by the existing WKContextUseTestingNetworkSession SPI.

* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::Cache::initialize):
* NetworkProcess/cache/NetworkCache.h:
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::open):
(WebKit::NetworkCache::Storage::Storage):
(WebKit::NetworkCache::Storage::dispatchReadOperation):
(WebKit::NetworkCache::Storage::shrinkIfNeeded):
* NetworkProcess/cache/NetworkCacheStorage.h:
* NetworkProcess/cocoa/NetworkProcessCocoa.mm:
(WebKit::NetworkProcess::platformInitializeNetworkProcessCocoa):

LayoutTests:

Enable a few disabled tests to see how it goes.

* platform/mac-wk2/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCache.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h
Source/WebKit2/NetworkProcess/cocoa/NetworkProcessCocoa.mm
Source/WebKit2/NetworkProcess/soup/NetworkProcessSoup.cpp

index 5a8c1e3..bf28b49 100644 (file)
@@ -1,3 +1,14 @@
+2017-04-06  Antti Koivisto  <antti@apple.com>
+
+        Implement testing mode for disk cache
+        https://bugs.webkit.org/show_bug.cgi?id=170547
+
+        Reviewed by Andreas Kling.
+
+        Enable a few disabled tests to see how it goes.
+
+        * platform/mac-wk2/TestExpectations:
+
 2017-04-06  Romain Bellessort  <romain.bellessort@crf.canon.fr>
 
         [Readable Streams API] Implement ReadableStreamBYOBRequest respondWithNewView()
index 3d9a066..6332916 100644 (file)
@@ -308,10 +308,6 @@ webkit.org/b/151326 [ Yosemite+ ] webarchive/loading/missing-data.html [ Pass Cr
 
 webkit.org/b/151455 [ Release ] http/tests/xmlhttprequest/workers/methods-async.html [ Pass Timeout ]
 
-webkit.org/b/162945 http/tests/cache/disk-cache/disk-cache-request-max-stale.html [ Pass Failure ]
-
-webkit.org/b/162946 http/tests/cache/disk-cache/disk-cache-media.html [ Pass Failure ]
-
 webkit.org/b/150542 fast/forms/state-restore-per-form.html [ Pass Timeout ]
 
 webkit.org/b/151709 [ Release ] http/tests/xmlhttprequest/workers/methods.html [ Pass Timeout ]
index b2caa79..087163e 100644 (file)
@@ -1,3 +1,28 @@
+2017-04-06  Antti Koivisto  <antti@apple.com>
+
+        Implement testing mode for disk cache
+        https://bugs.webkit.org/show_bug.cgi?id=170547
+
+        Reviewed by Andreas Kling.
+
+        Disable read timeouts and cache shrinking in TestRunner to eliminate potential sources of randomness.
+
+        Cache directories are deleted by TestRunner so lack of shrinking does not consume the disk.
+
+        This is enabled by the existing WKContextUseTestingNetworkSession SPI.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::initialize):
+        * NetworkProcess/cache/NetworkCache.h:
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::open):
+        (WebKit::NetworkCache::Storage::Storage):
+        (WebKit::NetworkCache::Storage::dispatchReadOperation):
+        (WebKit::NetworkCache::Storage::shrinkIfNeeded):
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+        * NetworkProcess/cocoa/NetworkProcessCocoa.mm:
+        (WebKit::NetworkProcess::platformInitializeNetworkProcessCocoa):
+
 2017-04-06  Chris Dumez  <cdumez@apple.com>
 
         [WK2] Add C private API to toggle invisibleAutoplayNotPermitted setting
index e31bc04..0f617cf 100644 (file)
@@ -73,12 +73,12 @@ static void dumpFileChanged(Cache* cache)
 }
 #endif
 
-bool Cache::initialize(const String& cachePath, const Parameters& parameters)
+bool Cache::initialize(const String& cachePath, OptionSet<Option> options)
 {
-    m_storage = Storage::open(cachePath);
+    m_storage = Storage::open(cachePath, options.contains(Option::TestingMode) ? Storage::Mode::Testing : Storage::Mode::Normal);
 
 #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
-    if (parameters.enableNetworkCacheSpeculativeRevalidation) {
+    if (options.contains(Option::SpeculativeRevalidation)) {
         m_lowPowerModeNotifier = std::make_unique<WebCore::LowPowerModeNotifier>([this](bool isLowPowerModeEnabled) {
             ASSERT(WTF::isMainThread());
             if (isLowPowerModeEnabled)
@@ -93,7 +93,7 @@ bool Cache::initialize(const String& cachePath, const Parameters& parameters)
     }
 #endif
 
-    if (parameters.enableEfficacyLogging)
+    if (options.contains(Option::EfficacyLogging))
         m_statistics = Statistics::open(cachePath);
 
 #if PLATFORM(COCOA)
index 63bbf24..ed0f3d4 100644 (file)
@@ -33,6 +33,7 @@
 #include "ShareableResource.h"
 #include <WebCore/ResourceResponse.h>
 #include <wtf/Function.h>
+#include <wtf/OptionSet.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebCore {
@@ -94,13 +95,15 @@ class Cache {
     WTF_MAKE_NONCOPYABLE(Cache);
     friend class WTF::NeverDestroyed<Cache>;
 public:
-    struct Parameters {
-        bool enableEfficacyLogging;
+    enum class Option {
+        EfficacyLogging,
+        // In testing mode we try to eliminate sources of randomness. Cache does not shrink and there are no read timeouts.
+        TestingMode,
 #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
-        bool enableNetworkCacheSpeculativeRevalidation;
+        SpeculativeRevalidation,
 #endif
     };
-    bool initialize(const String& cachePath, const Parameters&);
+    bool initialize(const String& cachePath, OptionSet<Option>);
     void setCapacity(size_t);
 
     bool isEnabled() const { return !!m_storage; }
index 0770b4a..26a57a2 100644 (file)
@@ -149,7 +149,7 @@ static String makeSaltFilePath(const String& baseDirectoryPath)
     return WebCore::pathByAppendingComponent(makeVersionedDirectoryPath(baseDirectoryPath), saltFileName);
 }
 
-std::unique_ptr<Storage> Storage::open(const String& cachePath)
+std::unique_ptr<Storage> Storage::open(const String& cachePath, Mode mode)
 {
     ASSERT(RunLoop::isMain());
 
@@ -158,7 +158,7 @@ std::unique_ptr<Storage> Storage::open(const String& cachePath)
     auto salt = readOrMakeSalt(makeSaltFilePath(cachePath));
     if (!salt)
         return nullptr;
-    return std::unique_ptr<Storage>(new Storage(cachePath, *salt));
+    return std::unique_ptr<Storage>(new Storage(cachePath, mode, *salt));
 }
 
 void traverseRecordsFiles(const String& recordsPath, const String& expectedType, const RecordFileTraverseFunction& function)
@@ -207,9 +207,10 @@ static void deleteEmptyRecordsDirectories(const String& recordsPath)
     });
 }
 
-Storage::Storage(const String& baseDirectoryPath, Salt salt)
+Storage::Storage(const String& baseDirectoryPath, Mode mode, Salt salt)
     : m_basePath(baseDirectoryPath)
     , m_recordsPath(makeRecordsDirectoryPath(baseDirectoryPath))
+    , m_mode(mode)
     , m_salt(salt)
     , m_canUseSharedMemoryForBodyData(canUseSharedMemoryForPath(baseDirectoryPath))
     , m_readOperationTimeoutTimer(*this, &Storage::cancelAllReadOperations)
@@ -573,9 +574,12 @@ void Storage::dispatchReadOperation(std::unique_ptr<ReadOperation> readOperation
     auto& readOperation = *readOperationPtr;
     m_activeReadOperations.add(WTFMove(readOperationPtr));
 
-    // I/O pressure may make disk operations slow. If they start taking very long time we rather go to network.
-    const auto readTimeout = 1500ms;
-    m_readOperationTimeoutTimer.startOneShot(readTimeout);
+    // Avoid randomness during testing.
+    if (m_mode != Mode::Testing) {
+        // I/O pressure may make disk operations slow. If they start taking very long time we rather go to network.
+        const auto readTimeout = 1500ms;
+        m_readOperationTimeoutTimer.startOneShot(readTimeout);
+    }
 
     bool shouldGetBodyBlob = mayContainBlob(readOperation.key);
 
@@ -965,6 +969,10 @@ void Storage::shrinkIfNeeded()
 {
     ASSERT(RunLoop::isMain());
 
+    // Avoid randomness caused by cache shrinks.
+    if (m_mode == Mode::Testing)
+        return;
+
     if (approximateSize() > m_capacity)
         shrink();
 }
index 50b7dd5..af655da 100644 (file)
@@ -48,7 +48,8 @@ class IOChannel;
 class Storage {
     WTF_MAKE_NONCOPYABLE(Storage);
 public:
-    static std::unique_ptr<Storage> open(const String& cachePath);
+    enum class Mode { Normal, Testing };
+    static std::unique_ptr<Storage> open(const String& cachePath, Mode);
 
     struct Record {
         WTF_MAKE_FAST_ALLOCATED;
@@ -105,7 +106,7 @@ public:
     ~Storage();
 
 private:
-    Storage(const String& directoryPath, Salt);
+    Storage(const String& directoryPath, Mode, Salt);
 
     String recordDirectoryPathForKey(const Key&) const;
     String recordPathForKey(const Key&) const;
@@ -145,9 +146,9 @@ private:
 
     const String m_basePath;
     const String m_recordsPath;
-
+    
+    const Mode m_mode;
     const Salt m_salt;
-
     const bool m_canUseSharedMemoryForBodyData;
 
     size_t m_capacity { std::numeric_limits<size_t>::max() };
index e5497ba..ef0f590 100644 (file)
@@ -112,13 +112,16 @@ void NetworkProcess::platformInitializeNetworkProcessCocoa(const NetworkProcessC
         SandboxExtension::consumePermanently(parameters.diskCacheDirectoryExtensionHandle);
 #if ENABLE(NETWORK_CACHE)
         if (parameters.shouldEnableNetworkCache) {
-            NetworkCache::Cache::Parameters cacheParameters = {
-                parameters.shouldEnableNetworkCacheEfficacyLogging
+            OptionSet<NetworkCache::Cache::Option> cacheOptions;
+            if (parameters.shouldEnableNetworkCacheEfficacyLogging)
+                cacheOptions |= NetworkCache::Cache::Option::EfficacyLogging;
+            if (parameters.shouldUseTestingNetworkSession)
+                cacheOptions |= NetworkCache::Cache::Option::TestingMode;
 #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
-                , parameters.shouldEnableNetworkCacheSpeculativeRevalidation
+            if (parameters.shouldEnableNetworkCacheSpeculativeRevalidation)
+                cacheOptions |= NetworkCache::Cache::Option::SpeculativeRevalidation;
 #endif
-            };
-            if (NetworkCache::singleton().initialize(m_diskCacheDirectory, cacheParameters)) {
+            if (NetworkCache::singleton().initialize(m_diskCacheDirectory, cacheOptions)) {
                 auto urlCache(adoptNS([[NSURLCache alloc] initWithMemoryCapacity:0 diskCapacity:0 diskPath:nil]));
                 [NSURLCache setSharedURLCache:urlCache.get()];
                 return;
index 26070f0..eb48e1f 100644 (file)
@@ -112,13 +112,15 @@ void NetworkProcess::platformInitializeNetworkProcess(const NetworkProcessCreati
 
     SoupNetworkSession::clearOldSoupCache(WebCore::directoryName(m_diskCacheDirectory));
 
-    NetworkCache::Cache::Parameters cacheParameters {
-        parameters.shouldEnableNetworkCacheEfficacyLogging
+    OptionSet<NetworkCache::Cache::Option> cacheOptions;
+    if (parameters.shouldEnableNetworkCacheEfficacyLogging)
+        cacheOptions |= NetworkCache::Cache::Option::EfficacyLogging;
 #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
-        , parameters.shouldEnableNetworkCacheSpeculativeRevalidation
+    if (parameters.shouldEnableNetworkCacheSpeculativeRevalidation)
+        cacheOptions |= NetworkCache::Cache::Option::SpeculativeRevalidation;
 #endif
-    };
-    NetworkCache::singleton().initialize(m_diskCacheDirectory, cacheParameters);
+
+    NetworkCache::singleton().initialize(m_diskCacheDirectory, cacheOptions);
 
     if (!parameters.cookiePersistentStoragePath.isEmpty()) {
         supplement<WebCookieManager>()->setCookiePersistentStorage(parameters.cookiePersistentStoragePath,