Attachment 368730 Details for Bug 197464 – Patch

[patch] Patch

bug-197464-20190501162428.patch (text/plain), 12.72 KB, created by youenn fablet on 2019-05-01 16:24:28 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2019-05-01 16:24:28 PDT

Size: 12.72 KB

patch

obsolete

>Subversion Revision: 244802
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 67149f1dc1efedbb3ab59423d2442b173e1c0ef5..4ea65fd6c1e3f6ea7dbba282136a1edeef03a953 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2019-05-01  Youenn Fablet  <youenn@apple.com>
>+
>+        Cache.add and Cache.addAll should compute a correct response body size
>+        https://bugs.webkit.org/show_bug.cgi?id=197464
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Compute the response body size as we do for regular Cache.put
>+
>+        Test: http/wpt/cache-storage/cache-quota-add.any.html
>+
>+        * Modules/cache/CacheStorageConnection.cpp:
>+        (WebCore::CacheStorageConnection::computeRecordBodySize):
>+        * Modules/cache/CacheStorageConnection.h:
>+        * Modules/cache/DOMCache.cpp:
>+        (WebCore::FetchTasksHandler::addResponseBody):
>+        (WebCore::DOMCache::addAll):
>+        Compute the response body size requires getting access to the connection.
>+        'this' is added to the lambda which is fine since taskHandler keeps a
>+        Ref to 'this' in its completion handler.
>+        (WebCore::DOMCache::toConnectionRecord):
>+        * Modules/fetch/FetchResponse.h:
>+
> 2019-05-01  Youenn Fablet  <youenn@apple.com>
> 
>         Add back hasNullReferences() assert in Document::updateIsPlayingMedia
>diff --git a/Source/WebCore/Modules/cache/CacheStorageConnection.cpp b/Source/WebCore/Modules/cache/CacheStorageConnection.cpp
>index 0a943eb84cbc429f441f51117934f96c42231392..e394e56a8f8c38afcd8c96d914271632f8a2a600 100644
>--- a/Source/WebCore/Modules/cache/CacheStorageConnection.cpp
>+++ b/Source/WebCore/Modules/cache/CacheStorageConnection.cpp
>@@ -104,10 +104,10 @@ static inline uint64_t computeRealBodySize(const DOMCacheEngine::ResponseBody& b
>     return result;
> }
> 
>-uint64_t CacheStorageConnection::computeRecordBodySize(const FetchResponse& response, const DOMCacheEngine::ResponseBody& body, ResourceResponse::Tainting tainting)
>+uint64_t CacheStorageConnection::computeRecordBodySize(const FetchResponse& response, const DOMCacheEngine::ResponseBody& body)
> {
>     if (!response.opaqueLoadIdentifier()) {
>-        ASSERT_UNUSED(tainting, tainting != ResourceResponse::Tainting::Opaque);
>+        ASSERT(response.tainting() != ResourceResponse::Tainting::Opaque);
>         return computeRealBodySize(body);
>     }
> 
>diff --git a/Source/WebCore/Modules/cache/CacheStorageConnection.h b/Source/WebCore/Modules/cache/CacheStorageConnection.h
>index 104772ad70e9de975ca3a6ab4f2046954aa04b7a..9aa408d35a4e0e25faa716af203f66034a0a3351 100644
>--- a/Source/WebCore/Modules/cache/CacheStorageConnection.h
>+++ b/Source/WebCore/Modules/cache/CacheStorageConnection.h
>@@ -47,7 +47,7 @@ public:
>     void retrieveRecords(uint64_t cacheIdentifier, const URL&, DOMCacheEngine::RecordsCallback&&);
>     void batchDeleteOperation(uint64_t cacheIdentifier, const ResourceRequest&, CacheQueryOptions&&, DOMCacheEngine::RecordIdentifiersCallback&&);
>     void batchPutOperation(uint64_t cacheIdentifier, Vector<DOMCacheEngine::Record>&&, DOMCacheEngine::RecordIdentifiersCallback&&);
>-    uint64_t computeRecordBodySize(const FetchResponse&, const DOMCacheEngine::ResponseBody&, ResourceResponse::Tainting);
>+    uint64_t computeRecordBodySize(const FetchResponse&, const DOMCacheEngine::ResponseBody&);
> 
>     virtual void reference(uint64_t /* cacheIdentifier */) { }
>     virtual void dereference(uint64_t /* cacheIdentifier */) { }
>diff --git a/Source/WebCore/Modules/cache/DOMCache.cpp b/Source/WebCore/Modules/cache/DOMCache.cpp
>index a53c8150160f0de73f2ea771ed1b967e6da73a09..af9debe74ce7feaaed3317b33dde24dfee41182c 100644
>--- a/Source/WebCore/Modules/cache/DOMCache.cpp
>+++ b/Source/WebCore/Modules/cache/DOMCache.cpp
>@@ -155,10 +155,7 @@ static inline bool hasResponseVaryStarHeaderValue(const FetchResponse& response)
> 
> class FetchTasksHandler : public RefCounted<FetchTasksHandler> {
> public:
>-    explicit FetchTasksHandler(Function<void(ExceptionOr<Vector<Record>>&&)>&& callback)
>-        : m_callback(WTFMove(callback))
>-    {
>-    }
>+    static Ref<FetchTasksHandler> create(Ref<DOMCache>&& domCache, CompletionHandler<void(ExceptionOr<Vector<Record>>&&)>&& callback) { return adoptRef(*new FetchTasksHandler(WTFMove(domCache), WTFMove(callback))); }
> 
>     ~FetchTasksHandler()
>     {
>@@ -175,10 +172,13 @@ public:
>         return m_records.size() - 1;
>     }
> 
>-    void addResponseBody(size_t position, Ref<SharedBuffer>&& data)
>+    void addResponseBody(size_t position, FetchResponse& response, DOMCacheEngine::ResponseBody&& data)
>     {
>         ASSERT(!isDone());
>-        m_records[position].responseBody = WTFMove(data);
>+        auto& record = m_records[position];
>+        DOMCacheEngine::ResponseBody body = WTFMove(data);
>+        record.responseBodySize = m_domCache->connection().computeRecordBodySize(response, body);
>+        record.responseBody = WTFMove(body);
>     }
> 
>     bool isDone() const { return !m_callback; }
>@@ -190,8 +190,15 @@ public:
>     }
> 
> private:
>+    FetchTasksHandler(Ref<DOMCache>&& domCache, CompletionHandler<void(ExceptionOr<Vector<Record>>&&)>&& callback)
>+        : m_domCache(WTFMove(domCache))
>+        , m_callback(WTFMove(callback))
>+    {
>+    }
>+
>+    Ref<DOMCache> m_domCache;
>     Vector<Record> m_records;
>-    Function<void(ExceptionOr<Vector<Record>>&&)> m_callback;
>+    CompletionHandler<void(ExceptionOr<Vector<Record>>&&)> m_callback;
> };
> 
> ExceptionOr<Ref<FetchRequest>> DOMCache::requestFromInfo(RequestInfo&& info, bool ignoreMethod)
>@@ -227,7 +234,7 @@ void DOMCache::addAll(Vector<RequestInfo>&& infos, DOMPromiseDeferred<void>&& pr
>         requests.uncheckedAppend(requestOrException.releaseReturnValue());
>     }
> 
>-    auto taskHandler = adoptRef(*new FetchTasksHandler([protectedThis = makeRef(*this), this, promise = WTFMove(promise)](ExceptionOr<Vector<Record>>&& result) mutable {
>+    auto taskHandler = FetchTasksHandler::create(makeRef(*this), [this, promise = WTFMove(promise)](ExceptionOr<Vector<Record>>&& result) mutable {
>         if (result.hasException()) {
>             promise.reject(result.releaseException());
>             return;
>@@ -235,7 +242,7 @@ void DOMCache::addAll(Vector<RequestInfo>&& infos, DOMPromiseDeferred<void>&& pr
>         batchPutOperation(result.releaseReturnValue(), [promise = WTFMove(promise)](ExceptionOr<void>&& result) mutable {
>             promise.settle(WTFMove(result));
>         });
>-    }));
>+    });
> 
>     for (auto& request : requests) {
>         auto& requestReference = request.get();
>@@ -275,7 +282,7 @@ void DOMCache::addAll(Vector<RequestInfo>&& infos, DOMPromiseDeferred<void>&& pr
>             }
>             size_t recordPosition = taskHandler->addRecord(toConnectionRecord(request.get(), response, nullptr));
> 
>-            response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = SharedBuffer::create()] (ExceptionOr<ReadableStreamChunk*>&& result) mutable {
>+            response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = SharedBuffer::create(), response = makeRef(response)] (ExceptionOr<ReadableStreamChunk*>&& result) mutable {
>                 if (taskHandler->isDone())
>                     return;
> 
>@@ -287,7 +294,7 @@ void DOMCache::addAll(Vector<RequestInfo>&& infos, DOMPromiseDeferred<void>&& pr
>                 if (auto chunk = result.returnValue())
>                     data->append(reinterpret_cast<const char*>(chunk->data), chunk->size);
>                 else
>-                    taskHandler->addResponseBody(recordPosition, WTFMove(data));
>+                    taskHandler->addResponseBody(recordPosition, response, WTFMove(data));
>             });
>         });
>     }
>@@ -504,7 +511,7 @@ Record DOMCache::toConnectionRecord(const FetchRequest& request, FetchResponse&
> 
>     auto sizeWithPadding = response.bodySizeWithPadding();
>     if (!sizeWithPadding) {
>-        sizeWithPadding = m_connection->computeRecordBodySize(response, responseBody, cachedResponse.tainting());
>+        sizeWithPadding = m_connection->computeRecordBodySize(response, responseBody);
>         response.setBodySizeWithPadding(sizeWithPadding);
>     }
> 
>diff --git a/Source/WebCore/Modules/cache/DOMCache.h b/Source/WebCore/Modules/cache/DOMCache.h
>index 9951dda447fdbfc7aae6442eb9ed275fc3660ae1..fd4fd3f7a22a16f681a28e0ba2fd32042491a88a 100644
>--- a/Source/WebCore/Modules/cache/DOMCache.h
>+++ b/Source/WebCore/Modules/cache/DOMCache.h
>@@ -59,6 +59,8 @@ public:
>     using MatchCallback = WTF::Function<void(ExceptionOr<FetchResponse*>)>;
>     void doMatch(RequestInfo&&, CacheQueryOptions&&, MatchCallback&&);
> 
>+    CacheStorageConnection& connection() { return m_connection.get(); }
>+
> private:
>     DOMCache(ScriptExecutionContext&, String&& name, uint64_t identifier, Ref<CacheStorageConnection>&&);
> 
>diff --git a/Source/WebCore/Modules/fetch/FetchResponse.h b/Source/WebCore/Modules/fetch/FetchResponse.h
>index 1e549b00d970943cf6fe9d6c4ef4e65760e68369..74c1454f9aaf31241816c51e46e0f04e70ae1322 100644
>--- a/Source/WebCore/Modules/fetch/FetchResponse.h
>+++ b/Source/WebCore/Modules/fetch/FetchResponse.h
>@@ -102,6 +102,7 @@ public:
>     void consumeBodyReceivedByChunk(ConsumeDataByChunkCallback&&);
> 
>     WEBCORE_EXPORT ResourceResponse resourceResponse() const;
>+    ResourceResponse::Tainting tainting() const { return m_internalResponse.tainting(); }
> 
>     uint64_t bodySizeWithPadding() const { return m_bodySizeWithPadding; }
>     void setBodySizeWithPadding(uint64_t size) { m_bodySizeWithPadding = size; }
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f3bed47eec272f23f1eaf0cfae3dbd26e89b0a32..4c834ad04ed7c92f29cda19a7ed18c6f2284c015 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-05-01  Youenn Fablet  <youenn@apple.com>
>+
>+        Cache.add and Cache.addAll should compute a correct response body size
>+        https://bugs.webkit.org/show_bug.cgi?id=197464
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/wpt/cache-storage/cache-quota-add.any-expected.txt: Added.
>+        * http/wpt/cache-storage/cache-quota-add.any.html: Added.
>+        * http/wpt/cache-storage/cache-quota-add.any.js: Added.
>+
> 2019-05-01  Youenn Fablet  <youenn@apple.com>
> 
>         LayoutTest imported/w3c/web-platform-tests/xhr/event-upload-progress-crossorigin.htm is a flaky failure
>diff --git a/LayoutTests/http/wpt/cache-storage/cache-quota-add.any-expected.txt b/LayoutTests/http/wpt/cache-storage/cache-quota-add.any-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..4f20abf18ec60f0ca907c05d2a37cdb5fc641184
>--- /dev/null
>+++ b/LayoutTests/http/wpt/cache-storage/cache-quota-add.any-expected.txt
>@@ -0,0 +1,6 @@
>+CONSOLE MESSAGE: Cache API operation failed: Quota exceeded
>+CONSOLE MESSAGE: Cache API operation failed: Quota exceeded
>+
>+PASS Testing that cache.add checks against quota 
>+PASS Testing that cache.addAll checks against quota 
>+
>diff --git a/LayoutTests/http/wpt/cache-storage/cache-quota-add.any.html b/LayoutTests/http/wpt/cache-storage/cache-quota-add.any.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..2382913528e693b3a5d56c660a45060980b548c3
>--- /dev/null
>+++ b/LayoutTests/http/wpt/cache-storage/cache-quota-add.any.html
>@@ -0,0 +1 @@
>+
>\ No newline at end of file
>diff --git a/LayoutTests/http/wpt/cache-storage/cache-quota-add.any.js b/LayoutTests/http/wpt/cache-storage/cache-quota-add.any.js
>new file mode 100644
>index 0000000000000000000000000000000000000000..b6c32354e73609bb2c9885f253c7e441e16cecec
>--- /dev/null
>+++ b/LayoutTests/http/wpt/cache-storage/cache-quota-add.any.js
>@@ -0,0 +1,30 @@
>+if (window.testRunner)
>+    testRunner.setAllowStorageQuotaIncrease(false);
>+
>+async function doCleanup()
>+{
>+    var cachesKeys = await self.caches.keys();
>+    for (let name of cachesKeys) {
>+        let cache = await self.caches.open(name);
>+        let keys = await cache.keys();
>+        for (let key of keys)
>+            await cache.delete(key);
>+    }
>+}
>+
>+promise_test(async (test) => {
>+    const cache = await self.caches.open("add");
>+    const response399ko = new Response(new ArrayBuffer(399 * 1024));
>+    await cache.put("test", response399ko);
>+    return promise_rejects(test, "QuotaExceededError", cache.add("../preload/resources/square.png"));
>+}, "Testing that cache.add checks against quota");
>+
>+promise_test(async (test) => {
>+    await doCleanup();
>+    const cache = await self.caches.open("add2");
>+    const response380ko = new Response(new ArrayBuffer(380 * 1024));
>+    await cache.put("test", response380ko);
>+    return promise_rejects(test, "QuotaExceededError", cache.addAll(["../preload/resources/square.png?", "../preload/resources/square.png?1", "../preload/resources/square.png?2"]));
>+}, "Testing that cache.addAll checks against quota");
>+
>+done();

Actions: View | Formatted Diff | Diff

Attachments on bug 197464: 368676 | 368730 | 368953 | 368970