Use WTF::Function instead of std::function in network cache code
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Jan 2017 19:10:23 +0000 (19:10 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 5 Jan 2017 19:10:23 +0000 (19:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=166721

Reviewed by Andreas Kling.

Use better move-only type. Fix some unnecessary function copies.

* NetworkProcess/cache/NetworkCache.cpp:
(WebKit::NetworkCache::Cache::retrieve):
(WebKit::NetworkCache::Cache::clear):
* NetworkProcess/cache/NetworkCache.h:
* NetworkProcess/cache/NetworkCacheData.h:
* NetworkProcess/cache/NetworkCacheDataCocoa.mm:
(WebKit::NetworkCache::Data::apply):
* NetworkProcess/cache/NetworkCacheDataSoup.cpp:
(WebKit::NetworkCache::Data::apply):
* NetworkProcess/cache/NetworkCacheFileSystem.cpp:
(WebKit::NetworkCache::traverseDirectory):
* NetworkProcess/cache/NetworkCacheFileSystem.h:
* NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
(WebKit::NetworkCache::SpeculativeLoadManager::canRetrieve):
(WebKit::NetworkCache::SpeculativeLoadManager::retrieve):

    Split retrieve() to canRetrieve() and retrieve() functions.
    This avoids the need to copy the completion handler in the caller.

* NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h:
* NetworkProcess/cache/NetworkCacheStorage.cpp:
(WebKit::NetworkCache::Storage::ReadOperation::ReadOperation):
(WebKit::NetworkCache::Storage::traverse):
(WebKit::NetworkCache::Storage::clear):
* NetworkProcess/cache/NetworkCacheStorage.h:

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

13 files changed:
Source/WebKit2/ChangeLog
Source/WebKit2/NetworkProcess/NetworkResourceLoader.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCache.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheData.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheDataCocoa.mm
Source/WebKit2/NetworkProcess/cache/NetworkCacheDataSoup.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheFileSystem.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.cpp
Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h

index 9a100b7..2a81c8b 100644 (file)
@@ -1,3 +1,38 @@
+2017-01-05  Antti Koivisto  <antti@apple.com>
+
+        Use WTF::Function instead of std::function in network cache code
+        https://bugs.webkit.org/show_bug.cgi?id=166721
+
+        Reviewed by Andreas Kling.
+
+        Use better move-only type. Fix some unnecessary function copies.
+
+        * NetworkProcess/cache/NetworkCache.cpp:
+        (WebKit::NetworkCache::Cache::retrieve):
+        (WebKit::NetworkCache::Cache::clear):
+        * NetworkProcess/cache/NetworkCache.h:
+        * NetworkProcess/cache/NetworkCacheData.h:
+        * NetworkProcess/cache/NetworkCacheDataCocoa.mm:
+        (WebKit::NetworkCache::Data::apply):
+        * NetworkProcess/cache/NetworkCacheDataSoup.cpp:
+        (WebKit::NetworkCache::Data::apply):
+        * NetworkProcess/cache/NetworkCacheFileSystem.cpp:
+        (WebKit::NetworkCache::traverseDirectory):
+        * NetworkProcess/cache/NetworkCacheFileSystem.h:
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+        (WebKit::NetworkCache::SpeculativeLoadManager::canRetrieve):
+        (WebKit::NetworkCache::SpeculativeLoadManager::retrieve):
+
+            Split retrieve() to canRetrieve() and retrieve() functions.
+            This avoids the need to copy the completion handler in the caller.
+
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h:
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::ReadOperation::ReadOperation):
+        (WebKit::NetworkCache::Storage::traverse):
+        (WebKit::NetworkCache::Storage::clear):
+        * NetworkProcess/cache/NetworkCacheStorage.h:
+
 2017-01-05  Enrica Casucci  <enrica@apple.com>
 
         Unreviewed build fix after https://trac.webkit.org/changeset/210360
index 9010599..2ffe202 100644 (file)
@@ -177,7 +177,7 @@ void NetworkResourceLoader::retrieveCacheEntry(const ResourceRequest& request)
     ASSERT(canUseCache(request));
 
     RefPtr<NetworkResourceLoader> loader(this);
-    NetworkCache::singleton().retrieve(request, { m_parameters.webPageID, m_parameters.webFrameID }, [loader, request](auto entry) {
+    NetworkCache::singleton().retrieve(request, { m_parameters.webPageID, m_parameters.webFrameID }, [loader = WTFMove(loader), request](auto entry) {
         if (loader->hasOneRef()) {
             // The loader has been aborted and is only held alive by this lambda.
             return;
index 6b99476..a80291f 100644 (file)
@@ -309,7 +309,7 @@ static StoreDecision makeStoreDecision(const WebCore::ResourceRequest& originalR
     return StoreDecision::Yes;
 }
 
-void Cache::retrieve(const WebCore::ResourceRequest& request, const GlobalFrameID& frameID, std::function<void (std::unique_ptr<Entry>)>&& completionHandler)
+void Cache::retrieve(const WebCore::ResourceRequest& request, const GlobalFrameID& frameID, Function<void (std::unique_ptr<Entry>)>&& completionHandler)
 {
     ASSERT(isEnabled());
     ASSERT(request.url().protocolIsInHTTPFamily());
@@ -337,13 +337,15 @@ void Cache::retrieve(const WebCore::ResourceRequest& request, const GlobalFrameI
     }
 
 #if ENABLE(NETWORK_CACHE_SPECULATIVE_REVALIDATION)
-    if (canUseSpeculativeRevalidation && m_speculativeLoadManager->retrieve(frameID, storageKey, request, [request, completionHandler](std::unique_ptr<Entry> entry) {
-        if (entry && WebCore::verifyVaryingRequestHeaders(entry->varyingRequestHeaders(), request))
-            completionHandler(WTFMove(entry));
-        else
-            completionHandler(nullptr);
-    }))
+    if (canUseSpeculativeRevalidation && m_speculativeLoadManager->canRetrieve(storageKey, request, frameID)) {
+        m_speculativeLoadManager->retrieve(storageKey, [request, completionHandler = WTFMove(completionHandler)](std::unique_ptr<Entry> entry) {
+            if (entry && WebCore::verifyVaryingRequestHeaders(entry->varyingRequestHeaders(), request))
+                completionHandler(WTFMove(entry));
+            else
+                completionHandler(nullptr);
+        });
         return;
+    }
 #endif
 
     auto startTime = std::chrono::system_clock::now();
@@ -586,7 +588,7 @@ void Cache::deleteDumpFile()
     });
 }
 
-void Cache::clear(std::chrono::system_clock::time_point modifiedSince, std::function<void ()>&& completionHandler)
+void Cache::clear(std::chrono::system_clock::time_point modifiedSince, Function<void ()>&& completionHandler)
 {
     LOG(NetworkCache, "(NetworkProcess) clearing cache");
 
index a391e7a..4794efa 100644 (file)
@@ -105,7 +105,7 @@ public:
     bool isEnabled() const { return !!m_storage; }
 
     // Completion handler may get called back synchronously on failure.
-    void retrieve(const WebCore::ResourceRequest&, const GlobalFrameID&, std::function<void (std::unique_ptr<Entry>)>&&);
+    void retrieve(const WebCore::ResourceRequest&, const GlobalFrameID&, Function<void (std::unique_ptr<Entry>)>&&);
     std::unique_ptr<Entry> store(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, RefPtr<WebCore::SharedBuffer>&&, Function<void (MappedBody&)>&&);
     std::unique_ptr<Entry> storeRedirect(const WebCore::ResourceRequest&, const WebCore::ResourceResponse&, const WebCore::ResourceRequest& redirectRequest);
     std::unique_ptr<Entry> update(const WebCore::ResourceRequest&, const GlobalFrameID&, const Entry&, const WebCore::ResourceResponse& validatingResponse);
@@ -119,7 +119,7 @@ public:
     void remove(const WebCore::ResourceRequest&);
 
     void clear();
-    void clear(std::chrono::system_clock::time_point modifiedSince, std::function<void ()>&& completionHandler);
+    void clear(std::chrono::system_clock::time_point modifiedSince, Function<void ()>&& completionHandler);
 
     void dumpContentsToFile();
 
index 724608d..29c264b 100644 (file)
@@ -128,7 +128,7 @@ public:
 
     Data subrange(size_t offset, size_t) const;
 
-    bool apply(const std::function<bool (const uint8_t*, size_t)>&&) const;
+    bool apply(const Function<bool (const uint8_t*, size_t)>&) const;
 
     Data mapToFile(const char* path) const;
 
index 7f267c1..3d97c3b 100644 (file)
@@ -71,7 +71,7 @@ bool Data::isNull() const
     return !m_dispatchData;
 }
 
-bool Data::apply(const std::function<bool (const uint8_t*, size_t)>&& applier) const
+bool Data::apply(const Function<bool (const uint8_t*, size_t)>& applier) const
 {
     if (!m_size)
         return false;
index 644c38e..21b58fa 100644 (file)
@@ -70,7 +70,7 @@ bool Data::isNull() const
     return !m_buffer;
 }
 
-bool Data::apply(const std::function<bool (const uint8_t*, size_t)>&& applier) const
+bool Data::apply(const Function<bool (const uint8_t*, size_t)>& applier) const
 {
     if (!m_size)
         return false;
index 93d6a58..091193a 100644 (file)
@@ -32,6 +32,7 @@
 #include <dirent.h>
 #include <sys/stat.h>
 #include <sys/time.h>
+#include <wtf/Function.h>
 #include <wtf/text/CString.h>
 
 #if USE(SOUP)
@@ -55,7 +56,7 @@ static DirectoryEntryType directoryEntryType(uint8_t dtype)
     }
 }
 
-void traverseDirectory(const String& path, const std::function<void (const String&, DirectoryEntryType)>& function)
+void traverseDirectory(const String& path, const Function<void (const String&, DirectoryEntryType)>& function)
 {
     DIR* dir = opendir(WebCore::fileSystemRepresentation(path).data());
     if (!dir)
index 17407e8..1583606 100644 (file)
@@ -35,7 +35,7 @@ namespace WebKit {
 namespace NetworkCache {
 
 enum class DirectoryEntryType { Directory, File };
-void traverseDirectory(const String& path, const std::function<void (const String& fileName, DirectoryEntryType)>&);
+void traverseDirectory(const String& path, const Function<void (const String& fileName, DirectoryEntryType)>&);
 
 void deleteDirectoryRecursively(const String& path);
 
index a4359f3..d9abf6d 100644 (file)
@@ -301,10 +301,10 @@ bool SpeculativeLoadManager::canUsePendingPreload(const SpeculativeLoad& load, c
     return requestsHeadersMatch(load.originalRequest(), actualRequest);
 }
 
-bool SpeculativeLoadManager::retrieve(const GlobalFrameID& frameID, const Key& storageKey, const WebCore::ResourceRequest& request, RetrieveCompletionHandler&& completionHandler)
+bool SpeculativeLoadManager::canRetrieve(const Key& storageKey, const WebCore::ResourceRequest& request, const GlobalFrameID& frameID) const
 {
     // Check already preloaded entries.
-    if (auto preloadedEntry = m_preloadedEntries.take(storageKey)) {
+    if (auto preloadedEntry = m_preloadedEntries.get(storageKey)) {
         if (!canUsePreloadedEntry(*preloadedEntry, request)) {
             LOG(NetworkCacheSpeculativePreloading, "(NetworkProcess) Retrieval: Could not use preloaded entry to satisfy request for '%s' due to HTTP headers mismatch:", storageKey.identifier().utf8().data());
             logSpeculativeLoadingDiagnosticMessage(frameID, preloadedEntry->wasRevalidated() ? DiagnosticLoggingKeys::wastedSpeculativeWarmupWithRevalidationKey() : DiagnosticLoggingKeys::wastedSpeculativeWarmupWithoutRevalidationKey());
@@ -313,15 +313,13 @@ bool SpeculativeLoadManager::retrieve(const GlobalFrameID& frameID, const Key& s
 
         LOG(NetworkCacheSpeculativePreloading, "(NetworkProcess) Retrieval: Using preloaded entry to satisfy request for '%s':", storageKey.identifier().utf8().data());
         logSpeculativeLoadingDiagnosticMessage(frameID, preloadedEntry->wasRevalidated() ? DiagnosticLoggingKeys::successfulSpeculativeWarmupWithRevalidationKey() : DiagnosticLoggingKeys::successfulSpeculativeWarmupWithoutRevalidationKey());
-
-        completionHandler(preloadedEntry->takeCacheEntry());
         return true;
     }
 
     // Check pending speculative revalidations.
     auto* pendingPreload = m_pendingPreloads.get(storageKey);
     if (!pendingPreload) {
-        if (m_notPreloadedEntries.remove(storageKey))
+        if (m_notPreloadedEntries.get(storageKey))
             logSpeculativeLoadingDiagnosticMessage(frameID, DiagnosticLoggingKeys::entryWronglyNotWarmedUpKey());
         else
             logSpeculativeLoadingDiagnosticMessage(frameID, DiagnosticLoggingKeys::unknownEntryRequestKey());
@@ -337,10 +335,21 @@ bool SpeculativeLoadManager::retrieve(const GlobalFrameID& frameID, const Key& s
 
     LOG(NetworkCacheSpeculativePreloading, "(NetworkProcess) Retrieval: revalidation already in progress for '%s':", storageKey.identifier().utf8().data());
 
+    return true;
+}
+
+void SpeculativeLoadManager::retrieve(const Key& storageKey, RetrieveCompletionHandler&& completionHandler)
+{
+    if (auto preloadedEntry = m_preloadedEntries.take(storageKey)) {
+        completionHandler(preloadedEntry->takeCacheEntry());
+        return;
+    }
+    ASSERT(m_pendingPreloads.contains(storageKey));
     // FIXME: This breaks incremental loading when the revalidation is not successful.
-    auto addResult = m_pendingRetrieveRequests.ensure(storageKey, [] { return std::make_unique<Vector<RetrieveCompletionHandler>>(); });
+    auto addResult = m_pendingRetrieveRequests.ensure(storageKey, [] {
+        return std::make_unique<Vector<RetrieveCompletionHandler>>();
+    });
     addResult.iterator->value->append(WTFMove(completionHandler));
-    return true;
 }
 
 void SpeculativeLoadManager::registerLoad(const GlobalFrameID& frameID, const ResourceRequest& request, const Key& resourceKey)
index dd606d5..7ae8db4 100644 (file)
@@ -51,8 +51,10 @@ public:
 
     void registerLoad(const GlobalFrameID&, const WebCore::ResourceRequest&, const Key& resourceKey);
 
-    typedef std::function<void (std::unique_ptr<Entry>)> RetrieveCompletionHandler;
-    bool retrieve(const GlobalFrameID&, const Key& storageKey, const WebCore::ResourceRequest&, RetrieveCompletionHandler&&);
+    typedef Function<void (std::unique_ptr<Entry>)> RetrieveCompletionHandler;
+
+    bool canRetrieve(const Key& storageKey, const WebCore::ResourceRequest&, const GlobalFrameID&) const;
+    void retrieve(const Key& storageKey, RetrieveCompletionHandler&&);
 
 private:
     class PreloadedEntry;
index b06b1cf..8f7c4f3 100644 (file)
@@ -53,9 +53,9 @@ static double computeRecordWorth(FileTimes);
 struct Storage::ReadOperation {
     WTF_MAKE_FAST_ALLOCATED;
 public:
-    ReadOperation(const Key& key, const RetrieveCompletionHandler& completionHandler)
+    ReadOperation(const Key& key, RetrieveCompletionHandler&& completionHandler)
         : key(key)
-        , completionHandler(completionHandler)
+        , completionHandler(WTFMove(completionHandler))
     { }
 
     void cancel();
@@ -804,7 +804,7 @@ void Storage::traverse(const String& type, TraverseFlags flags, TraverseHandler&
 {
     ASSERT(RunLoop::isMain());
     ASSERT(traverseHandler);
-    // Avoid non-thread safe std::function copies.
+    // Avoid non-thread safe Function copies.
 
     auto traverseOperationPtr = std::make_unique<TraverseOperation>(type, flags, WTFMove(traverseHandler));
     auto& traverseOperation = *traverseOperationPtr;
@@ -888,7 +888,7 @@ void Storage::setCapacity(size_t capacity)
     shrinkIfNeeded();
 }
 
-void Storage::clear(const String& type, std::chrono::system_clock::time_point modifiedSinceTime, std::function<void ()>&& completionHandler)
+void Storage::clear(const String& type, std::chrono::system_clock::time_point modifiedSinceTime, Function<void ()>&& completionHandler)
 {
     ASSERT(RunLoop::isMain());
     LOG(NetworkCacheStorage, "(NetworkProcess) clearing cache");
index 2e9c251..023e84a 100644 (file)
@@ -59,14 +59,14 @@ public:
         Data body;
     };
     // This may call completion handler synchronously on failure.
-    typedef std::function<bool (std::unique_ptr<Record>)> RetrieveCompletionHandler;
+    typedef Function<bool (std::unique_ptr<Record>)> RetrieveCompletionHandler;
     void retrieve(const Key&, unsigned priority, RetrieveCompletionHandler&&);
 
     typedef Function<void (const Data& mappedBody)> MappedBodyHandler;
     void store(const Record&, MappedBodyHandler&&);
 
     void remove(const Key&);
-    void clear(const String& type, std::chrono::system_clock::time_point modifiedSinceTime, std::function<void ()>&& completionHandler);
+    void clear(const String& type, std::chrono::system_clock::time_point modifiedSinceTime, Function<void ()>&& completionHandler);
 
     struct RecordInfo {
         size_t bodySize;
@@ -179,7 +179,7 @@ private:
 };
 
 // FIXME: Remove, used by NetworkCacheStatistics only.
-using RecordFileTraverseFunction = std::function<void (const String& fileName, const String& hashString, const String& type, bool isBlob, const String& recordDirectoryPath)>;
+using RecordFileTraverseFunction = Function<void (const String& fileName, const String& hashString, const String& type, bool isBlob, const String& recordDirectoryPath)>;
 void traverseRecordsFiles(const String& recordsPath, const String& type, const RecordFileTraverseFunction&);
 
 }