Attachment 359292 Details for Bug 193323 – Patch

[patch] Patch

bug-193323-20190116124117.patch (text/plain), 18.08 KB, created by youenn fablet on 2019-01-16 12:41:18 PST

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-01-16 12:41:18 PST

Size: 18.08 KB

patch

obsolete

>Subversion Revision: 239989
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 853ffd8eebdb8505d37955030bd5a6bdd92c332c..9c962c197d72494a96f782221c1daebd43b034bc 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,42 @@
>+2019-01-16  Youenn Fablet  <youenn@apple.com>
>+
>+        Pipe cache quota request from Network Process to UIProcess
>+        https://bugs.webkit.org/show_bug.cgi?id=193296
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When cache storage hits quota for a given origin, pipe the request back to NetworkProcess.
>+        No record will be put until the quota request is answered.
>+        The request is sent from CacheStorage::Caches to CacheStorage::Engine and to NetworkProcess.
>+        NetworkProcess then sends it to NetworkProcessProxy.
>+        Currently NetworkProcessProxy just answers by keeping the quota as it is.
>+        In the future, NetworkProcessProxy should make a delegate call to let the app using WebKit
>+        make a decision on the quota. This will allow prompting user to bump it as done for other data.
>+
>+        * NetworkProcess/NetworkProcess.cpp:
>+        (WebKit::NetworkProcess::requestCacheStorageSpace):
>+        * NetworkProcess/NetworkProcess.h:
>+        * NetworkProcess/cache/CacheStorageEngine.cpp:
>+        (WebKit::CacheStorage::Engine::~Engine):
>+        (WebKit::CacheStorage::Engine::from):
>+        (WebKit::CacheStorage::Engine::Engine):
>+        (WebKit::CacheStorage::Engine::requestSpace):
>+        (WebKit::CacheStorage::Engine::requestSpaceCompleted):
>+        * NetworkProcess/cache/CacheStorageEngine.h:
>+        * NetworkProcess/cache/CacheStorageEngineCache.cpp:
>+        (WebKit::CacheStorage::Cache::put):
>+        (WebKit::CacheStorage::Cache::retryPuttingPendingRecords):
>+        * NetworkProcess/cache/CacheStorageEngineCache.h:
>+        * NetworkProcess/cache/CacheStorageEngineCaches.cpp:
>+        (WebKit::CacheStorage::Caches::requestSpace):
>+        (WebKit::CacheStorage::Caches::notifyCachesOfRequestSpaceEnd):
>+        * NetworkProcess/cache/CacheStorageEngineCaches.h:
>+        (WebKit::CacheStorage::Caches::isRequestingSpace const):
>+        * UIProcess/Network/NetworkProcessProxy.cpp:
>+        (WebKit::NetworkProcessProxy::requestCacheStorageSpace):
>+        * UIProcess/Network/NetworkProcessProxy.h:
>+        * UIProcess/Network/NetworkProcessProxy.messages.in:
>+
> 2019-01-15  Youenn Fablet  <youenn@apple.com>
> 
>         ServiceWorkerContainer is leaking due to a ref cycle
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.cpp b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>index 0cd1524eb61c598c6cb0d5a415e233a2e69589a7..4e97a9c43010acfb42b92a1ff6f02857105abada 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.cpp
>@@ -1410,6 +1410,11 @@ void NetworkProcess::addServiceWorkerSession(PAL::SessionID sessionID, String& s
> }
> #endif // ENABLE(SERVICE_WORKER)
> 
>+void NetworkProcess::requestCacheStorageSpace(PAL::SessionID sessionID, const ClientOrigin& origin, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, CompletionHandler<void(Optional<uint64_t>)>&& callback)
>+{
>+    parentProcessConnection()->sendWithAsyncReply(Messages::NetworkProcessProxy::RequestCacheStorageSpace { sessionID, origin, quota, currentSize, spaceRequired }, WTFMove(callback), 0);
>+}
>+
> #if !PLATFORM(COCOA)
> void NetworkProcess::initializeProcess(const ChildProcessInitializationParameters&)
> {
>diff --git a/Source/WebKit/NetworkProcess/NetworkProcess.h b/Source/WebKit/NetworkProcess/NetworkProcess.h
>index a8a5124bb5edd88676fb491022f27ecec0c67f6a..aec3ea40dacb16049550e771bd800b69fa5861e9 100644
>--- a/Source/WebKit/NetworkProcess/NetworkProcess.h
>+++ b/Source/WebKit/NetworkProcess/NetworkProcess.h
>@@ -44,6 +44,7 @@
> #include <wtf/MemoryPressureHandler.h>
> #include <wtf/NeverDestroyed.h>
> #include <wtf/RetainPtr.h>
>+#include <wtf/WeakPtr.h>
> 
> namespace IPC {
> class FormDataReference;
>@@ -61,6 +62,7 @@ class NetworkStorageSession;
> class ResourceError;
> class SWServer;
> enum class StoredCredentialsPolicy : bool;
>+struct ClientOrigin;
> struct MessageWithMessagePorts;
> struct SecurityOriginData;
> struct SoupNetworkProxySettings;
>@@ -96,6 +98,7 @@ class NetworkProcess : public ChildProcess, private DownloadManager::Client, pub
> #if ENABLE(INDEXED_DATABASE)
>     , public WebCore::IDBServer::IDBBackingStoreTemporaryFileHandler
> #endif
>+    , public CanMakeWeakPtr<NetworkProcess>
> {
>     WTF_MAKE_NONCOPYABLE(NetworkProcess);
> public:
>@@ -222,11 +225,12 @@ public:
> 
>     void ref() const override { ThreadSafeRefCounted<NetworkProcess>::ref(); }
>     void deref() const override { ThreadSafeRefCounted<NetworkProcess>::deref(); }
>-    
>+
>     CacheStorage::Engine* findCacheEngine(const PAL::SessionID&);
>     CacheStorage::Engine& ensureCacheEngine(const PAL::SessionID&, Function<Ref<CacheStorage::Engine>()>&&);
>     void removeCacheEngine(const PAL::SessionID&);
>-    
>+    void requestCacheStorageSpace(PAL::SessionID, const WebCore::ClientOrigin&, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, CompletionHandler<void(Optional<uint64_t>)>&&);
>+
> private:
>     NetworkProcess();
> 
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp b/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp
>index 801d22e2ade82e85d9b85c0d5a5274b78aea8709..b2b5dc6639225c92dac58460d052bb78fb18f151 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.cpp
>@@ -85,7 +85,7 @@ void Engine::from(NetworkProcess& networkProcess, PAL::SessionID sessionID, Func
> 
>     networkProcess.cacheStorageParameters(sessionID, [networkProcess = makeRef(networkProcess), sessionID, callback = WTFMove(callback)] (auto&& rootPath, auto quota) mutable {
>         callback(networkProcess->ensureCacheEngine(sessionID, [&] {
>-            return adoptRef(*new Engine { String { rootPath }, quota });
>+            return adoptRef(*new Engine { sessionID, networkProcess.get(), String { rootPath }, quota });
>         }));
>     });
> }
>@@ -188,8 +188,10 @@ void Engine::clearCachesForOrigin(NetworkProcess& networkProcess, PAL::SessionID
>     });
> }
> 
>-Engine::Engine(String&& rootPath, uint64_t quota)
>-    : m_rootPath(WTFMove(rootPath))
>+Engine::Engine(PAL::SessionID sessionID, NetworkProcess& process, String&& rootPath, uint64_t quota)
>+    : m_sessionID(sessionID)
>+    , m_networkProcess(makeWeakPtr(process))
>+    , m_rootPath(WTFMove(rootPath))
>     , m_quota(quota)
> {
>     if (!m_rootPath.isNull())
>@@ -651,6 +653,15 @@ String Engine::representation()
>     return builder.toString();
> }
> 
>+void Engine::requestSpace(const WebCore::ClientOrigin& origin, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, RequestSpaceCallback&& callback)
>+{
>+    if (!m_networkProcess) {
>+        callback({ });
>+        return;
>+    }
>+    m_networkProcess->requestCacheStorageSpace(m_sessionID, origin, quota, currentSize, spaceRequired, WTFMove(callback));
>+}
>+
> } // namespace CacheStorage
> 
> } // namespace WebKit
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h b/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h
>index 1ebcb73616570df5073cffd5b5ed085414d8d216..a813949cfa19f1891bf7a67857806b1919adc012 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngine.h
>@@ -90,8 +90,11 @@ public:
>     const NetworkCache::Salt& salt() const { return m_salt.value(); }
>     uint64_t nextCacheIdentifier() { return ++m_nextCacheIdentifier; }
> 
>+    using RequestSpaceCallback = CompletionHandler<void(Optional<uint64_t>)>;
>+    void requestSpace(const WebCore::ClientOrigin&, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, RequestSpaceCallback&&);
>+
> private:
>-    Engine(String&& rootPath, uint64_t quota);
>+    Engine(PAL::SessionID, NetworkProcess&, String&& rootPath, uint64_t quota);
> 
>     void open(const WebCore::ClientOrigin&, const String& cacheName, WebCore::DOMCacheEngine::CacheIdentifierCallback&&);
>     void remove(uint64_t cacheIdentifier, WebCore::DOMCacheEngine::CacheIdentifierCallback&&);
>@@ -129,6 +132,8 @@ private:
> 
>     Cache* cache(uint64_t cacheIdentifier);
> 
>+    PAL::SessionID m_sessionID;
>+    WeakPtr<NetworkProcess> m_networkProcess;
>     HashMap<WebCore::ClientOrigin, RefPtr<Caches>> m_caches;
>     uint64_t m_nextCacheIdentifier { 0 };
>     String m_rootPath;
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp
>index e452dba5e331354592ce920cc284ba6623f0aa5a..5e1e94a160eb9bb63d0174db9796e873b762fbb5 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.cpp
>@@ -400,10 +400,15 @@ void Cache::storeRecords(Vector<Record>&& records, RecordIdentifiersCallback&& c
>     }
> }
> 
>-void Cache::put(Vector<Record>&& records, RecordIdentifiersCallback&& callback)
>+void Cache::put(Vector<Record>&& records, RecordIdentifiersCallback&& callback, CanRequestMoreSpace canRequestMoreSpace)
> {
>     ASSERT(m_state == State::Open);
> 
>+    if (m_caches.isRequestingSpace()) {
>+        m_pendingPutRequests.append({ WTFMove(records), WTFMove(callback) });
>+        return;
>+    }
>+
>     WebCore::CacheQueryOptions options;
>     uint64_t spaceRequired = 0;
> 
>@@ -423,19 +428,33 @@ void Cache::put(Vector<Record>&& records, RecordIdentifiersCallback&& callback)
>         return;
>     }
> 
>+    if (canRequestMoreSpace == CanRequestMoreSpace::No) {
>+        callback(makeUnexpected(DOMCacheEngine::Error::QuotaExceeded));
>+        return;
>+    }
>+
>     m_caches.requestSpace(spaceRequired, [caches = makeRef(m_caches), identifier = m_identifier, records = WTFMove(records), callback = WTFMove(callback)](Optional<DOMCacheEngine::Error>&& error) mutable {
>         if (error) {
>             callback(makeUnexpected(error.value()));
>             return;
>         }
>         auto* cache = caches->find(identifier);
>-        if (!cache)
>+        if (!cache) {
>+            callback(makeUnexpected(DOMCacheEngine::Error::Internal));
>             return;
>+        }
> 
>-        cache->storeRecords(WTFMove(records), WTFMove(callback));
>+        cache->put(WTFMove(records), WTFMove(callback), CanRequestMoreSpace::No);
>     });
> }
> 
>+void Cache::retryPuttingPendingRecords()
>+{
>+    auto pendingPutRequests = WTFMove(m_pendingPutRequests);
>+    for (auto& request : pendingPutRequests)
>+        put(WTFMove(request.records), WTFMove(request.callback));
>+}
>+
> void Cache::remove(WebCore::ResourceRequest&& request, WebCore::CacheQueryOptions&& options, RecordIdentifiersCallback&& callback)
> {
>     ASSERT(m_state == State::Open);
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.h b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.h
>index cf1b3b613e151fd0b6270992b26ae91455219c29..ae8436f952914e3a35620345c7bc6c53e4747fab 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.h
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCache.h
>@@ -69,7 +69,8 @@ public:
>     void retrieveRecords(const URL&, WebCore::DOMCacheEngine::RecordsCallback&&);
>     WebCore::DOMCacheEngine::CacheInfo info() const { return { m_identifier, m_name }; }
> 
>-    void put(Vector<WebCore::DOMCacheEngine::Record>&&, WebCore::DOMCacheEngine::RecordIdentifiersCallback&&);
>+    enum class CanRequestMoreSpace { No, Yes };
>+    void put(Vector<WebCore::DOMCacheEngine::Record>&&, WebCore::DOMCacheEngine::RecordIdentifiersCallback&&, CanRequestMoreSpace = CanRequestMoreSpace::Yes);
>     void remove(WebCore::ResourceRequest&&, WebCore::CacheQueryOptions&&, WebCore::DOMCacheEngine::RecordIdentifiersCallback&&);
> 
>     Vector<NetworkCache::Key> keys() const;
>@@ -77,6 +78,8 @@ public:
>     void dispose();
>     void clearMemoryRepresentation();
> 
>+    void retryPuttingPendingRecords();
>+
>     static Optional<WebCore::DOMCacheEngine::Record> decode(const NetworkCache::Storage::Record&);
>     static NetworkCache::Storage::Record encode(const RecordInformation&, const WebCore::DOMCacheEngine::Record&);
> 
>@@ -120,6 +123,12 @@ private:
>     HashMap<String, Vector<RecordInformation>> m_records;
>     uint64_t m_nextRecordIdentifier { 0 };
>     Vector<WebCore::DOMCacheEngine::CompletionCallback> m_pendingOpeningCallbacks;
>+
>+    struct PendingPutRequest {
>+        Vector<WebCore::DOMCacheEngine::Record> records;
>+        WebCore::DOMCacheEngine::RecordIdentifiersCallback callback;
>+    };
>+    Vector<PendingPutRequest> m_pendingPutRequests;
> };
> 
> } // namespace CacheStorage
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp
>index faa88c963c36f7622287d78b57eeb13459ade5e5..1913470cdd220ce3be975df9900ef350d071763c 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp
>@@ -481,9 +481,35 @@ void Caches::readRecordsList(Cache& cache, NetworkCache::Storage::TraverseHandle
> 
> void Caches::requestSpace(uint64_t spaceRequired, WebCore::DOMCacheEngine::CompletionCallback&& callback)
> {
>-    // FIXME: Implement quota increase request.
>+    ASSERT(!m_isRequestingSpace);
>+
>     ASSERT(m_quota < m_size + spaceRequired);
>-    callback(Error::QuotaExceeded);
>+
>+    if (!m_engine) {
>+        callback(Error::QuotaExceeded);
>+        return;
>+    }
>+
>+    m_isRequestingSpace = true;
>+    m_engine->requestSpace(m_origin, m_quota, m_size, spaceRequired, [this, protectedThis = makeRef(*this), callback = WTFMove(callback)] (Optional<uint64_t> newQuota) {
>+        m_isRequestingSpace = false;
>+        if (!newQuota) {
>+            callback(Error::QuotaExceeded);
>+            notifyCachesOfRequestSpaceEnd();
>+            return;
>+        }
>+        m_quota = *newQuota;
>+        callback({ });
>+        notifyCachesOfRequestSpaceEnd();
>+    });
>+}
>+
>+void Caches::notifyCachesOfRequestSpaceEnd()
>+{
>+    for (auto& cache : m_caches)
>+        cache.retryPuttingPendingRecords();
>+    for (auto& cache : m_removedCaches)
>+        cache.retryPuttingPendingRecords();
> }
> 
> void Caches::writeRecord(const Cache& cache, const RecordInformation& recordInformation, Record&& record, uint64_t previousRecordSize, CompletionCallback&& callback)
>diff --git a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h
>index f8e72bfee72915f0323a60352a482db46e852544..489622ceed13bc5e83b5b91be824ab088eb614f6 100644
>--- a/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h
>+++ b/Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.h
>@@ -61,6 +61,7 @@ public:
>     void readRecord(const NetworkCache::Key&, WTF::Function<void(Expected<WebCore::DOMCacheEngine::Record, WebCore::DOMCacheEngine::Error>&&)>&&);
> 
>     bool hasEnoughSpace(uint64_t spaceRequired) const { return m_quota >= m_size + spaceRequired; }
>+    bool isRequestingSpace() const { return m_isRequestingSpace; }
>     void requestSpace(uint64_t spaceRequired, WebCore::DOMCacheEngine::CompletionCallback&&);
>     void writeRecord(const Cache&, const RecordInformation&, WebCore::DOMCacheEngine::Record&&, uint64_t previousRecordSize, WebCore::DOMCacheEngine::CompletionCallback&&);
> 
>@@ -93,7 +94,10 @@ private:
>     void makeDirty() { ++m_updateCounter; }
>     bool isDirty(uint64_t updateCounter) const;
> 
>+    void notifyCachesOfRequestSpaceEnd();
>+
>     bool m_isInitialized { false };
>+    bool m_isRequestingSpace { false };
>     Engine* m_engine { nullptr };
>     uint64_t m_updateCounter { 0 };
>     WebCore::ClientOrigin m_origin;
>diff --git a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
>index d4d32049f3966fed84c45501b23a5f14133730b3..600c0bc39aec39269a8dfed9f5aece90e34224b9 100644
>--- a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
>+++ b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp
>@@ -734,6 +734,19 @@ void NetworkProcessProxy::establishWorkerContextConnectionToNetworkProcessForExp
> }
> #endif
> 
>+void NetworkProcessProxy::requestCacheStorageSpace(PAL::SessionID sessionID, const WebCore::ClientOrigin& origin, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, CompletionHandler<void(Optional<uint64_t> quota)>&& completionHandler)
>+{
>+    auto* store = websiteDataStoreFromSessionID(sessionID);
>+
>+    if (!store) {
>+        completionHandler({ });
>+        return;
>+    }
>+
>+    // FIXME: Ask WebsiteDataStore about updating the quota for this origin.
>+    completionHandler(quota);
>+}
>+
> } // namespace WebKit
> 
> #undef MESSAGE_CHECK
>diff --git a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
>index e348a85b954de28fa511c38f12da506c88819786..bfa8b8c57a0bf16cfe04b37388453be0ed978cd2 100644
>--- a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
>+++ b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.h
>@@ -166,6 +166,8 @@ private:
>     void establishWorkerContextConnectionToNetworkProcessForExplicitSession(WebCore::SecurityOriginData&&, PAL::SessionID);
> #endif
> 
>+    void requestCacheStorageSpace(PAL::SessionID, const WebCore::ClientOrigin&, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired, CompletionHandler<void(Optional<uint64_t> quota)>&&);
>+
>     WebsiteDataStore* websiteDataStoreFromSessionID(PAL::SessionID);
> 
>     // ProcessLauncher::Client
>diff --git a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
>index 2aae3eb4a872afb507137a313cf78caa68ac1c30..959267cc4fbc2815eec06d7ad5b7216ddb4ce837 100644
>--- a/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
>+++ b/Source/WebKit/UIProcess/Network/NetworkProcessProxy.messages.in
>@@ -63,4 +63,6 @@ messages -> NetworkProcessProxy LegacyReceiver {
>     EstablishWorkerContextConnectionToNetworkProcess(struct WebCore::SecurityOriginData origin)
>     EstablishWorkerContextConnectionToNetworkProcessForExplicitSession(struct WebCore::SecurityOriginData origin, PAL::SessionID explicitSession)
> #endif
>+
>+    RequestCacheStorageSpace(PAL::SessionID sessionID, struct WebCore::ClientOrigin origin, uint64_t quota, uint64_t currentSize, uint64_t spaceRequired) -> (Optional<uint64_t> newQuota) Async
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 193323: 358810 | 359292 | 359293 | 359386 | 359394 | 359400 | 359413 | 359428 | 359497