Skip to content

Commit 84ca2ab

Browse files
youennfyouennf
committed
Cache.add and Cache.addAll should compute a correct response body size
https://bugs.webkit.org/show_bug.cgi?id=197464 Reviewed by Chris Dumez. Source/WebCore: 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: LayoutTests: * 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. Canonical link: https://commits.webkit.org/211710@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@244918 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent 8861d1d commit 84ca2ab

10 files changed

Lines changed: 95 additions & 15 deletions

File tree

‎LayoutTests/ChangeLog‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,14 @@
1+
2019-05-03 Youenn Fablet <[email protected]>
2+
3+
Cache.add and Cache.addAll should compute a correct response body size
4+
https://bugs.webkit.org/show_bug.cgi?id=197464
5+
6+
Reviewed by Chris Dumez.
7+
8+
* http/wpt/cache-storage/cache-quota-add.any-expected.txt: Added.
9+
* http/wpt/cache-storage/cache-quota-add.any.html: Added.
10+
* http/wpt/cache-storage/cache-quota-add.any.js: Added.
11+
112
2019-05-03 Youenn Fablet <[email protected]>
213

314
LayoutTest imported/w3c/web-platform-tests/xhr/event-upload-progress-crossorigin.htm is a flaky failure

‎LayoutTests/http/wpt/cache-storage/cache-quota-add.any-expected.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
CONSOLE MESSAGE: Cache API operation failed: Quota exceeded
2+
CONSOLE MESSAGE: Cache API operation failed: Quota exceeded
3+
4+
PASS Testing that cache.add checks against quota
5+
PASS Testing that cache.addAll checks against quota
6+

‎LayoutTests/http/wpt/cache-storage/cache-quota-add.any.html‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<!-- This file is required for WebKit test infrastructure to run the templated test -->

‎LayoutTests/http/wpt/cache-storage/cache-quota-add.any.js‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
if (window.testRunner)
2+
testRunner.setAllowStorageQuotaIncrease(false);
3+
4+
async function doCleanup()
5+
{
6+
var cachesKeys = await self.caches.keys();
7+
for (let name of cachesKeys) {
8+
let cache = await self.caches.open(name);
9+
let keys = await cache.keys();
10+
for (let key of keys)
11+
await cache.delete(key);
12+
}
13+
}
14+
15+
promise_test(async (test) => {
16+
const cache = await self.caches.open("add");
17+
const response399ko = new Response(new ArrayBuffer(399 * 1024));
18+
await cache.put("test", response399ko);
19+
return promise_rejects(test, "QuotaExceededError", cache.add("../preload/resources/square.png"));
20+
}, "Testing that cache.add checks against quota");
21+
22+
promise_test(async (test) => {
23+
await doCleanup();
24+
const cache = await self.caches.open("add2");
25+
const response380ko = new Response(new ArrayBuffer(380 * 1024));
26+
await cache.put("test", response380ko);
27+
return promise_rejects(test, "QuotaExceededError", cache.addAll(["../preload/resources/square.png?", "../preload/resources/square.png?1", "../preload/resources/square.png?2"]));
28+
}, "Testing that cache.addAll checks against quota");
29+
30+
done();

‎Source/WebCore/ChangeLog‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,26 @@
1+
2019-05-03 Youenn Fablet <[email protected]>
2+
3+
Cache.add and Cache.addAll should compute a correct response body size
4+
https://bugs.webkit.org/show_bug.cgi?id=197464
5+
6+
Reviewed by Chris Dumez.
7+
8+
Compute the response body size as we do for regular Cache.put
9+
10+
Test: http/wpt/cache-storage/cache-quota-add.any.html
11+
12+
* Modules/cache/CacheStorageConnection.cpp:
13+
(WebCore::CacheStorageConnection::computeRecordBodySize):
14+
* Modules/cache/CacheStorageConnection.h:
15+
* Modules/cache/DOMCache.cpp:
16+
(WebCore::FetchTasksHandler::addResponseBody):
17+
(WebCore::DOMCache::addAll):
18+
Compute the response body size requires getting access to the connection.
19+
'this' is added to the lambda which is fine since taskHandler keeps a
20+
Ref to 'this' in its completion handler.
21+
(WebCore::DOMCache::toConnectionRecord):
22+
* Modules/fetch/FetchResponse.h:
23+
124
2019-05-03 Tomoki Imai <[email protected]>
225

326
[Cairo] Improve ShadowBlur performance using tiling optimization

‎Source/WebCore/Modules/cache/CacheStorageConnection.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,10 @@ static inline uint64_t computeRealBodySize(const DOMCacheEngine::ResponseBody& b
104104
return result;
105105
}
106106

107-
uint64_t CacheStorageConnection::computeRecordBodySize(const FetchResponse& response, const DOMCacheEngine::ResponseBody& body, ResourceResponse::Tainting tainting)
107+
uint64_t CacheStorageConnection::computeRecordBodySize(const FetchResponse& response, const DOMCacheEngine::ResponseBody& body)
108108
{
109109
if (!response.opaqueLoadIdentifier()) {
110-
ASSERT_UNUSED(tainting, tainting != ResourceResponse::Tainting::Opaque);
110+
ASSERT(response.tainting() != ResourceResponse::Tainting::Opaque);
111111
return computeRealBodySize(body);
112112
}
113113

‎Source/WebCore/Modules/cache/CacheStorageConnection.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ class CacheStorageConnection : public ThreadSafeRefCounted<CacheStorageConnectio
4747
void retrieveRecords(uint64_t cacheIdentifier, const URL&, DOMCacheEngine::RecordsCallback&&);
4848
void batchDeleteOperation(uint64_t cacheIdentifier, const ResourceRequest&, CacheQueryOptions&&, DOMCacheEngine::RecordIdentifiersCallback&&);
4949
void batchPutOperation(uint64_t cacheIdentifier, Vector<DOMCacheEngine::Record>&&, DOMCacheEngine::RecordIdentifiersCallback&&);
50-
uint64_t computeRecordBodySize(const FetchResponse&, const DOMCacheEngine::ResponseBody&, ResourceResponse::Tainting);
50+
uint64_t computeRecordBodySize(const FetchResponse&, const DOMCacheEngine::ResponseBody&);
5151

5252
virtual void reference(uint64_t /* cacheIdentifier */) { }
5353
virtual void dereference(uint64_t /* cacheIdentifier */) { }

‎Source/WebCore/Modules/cache/DOMCache.cpp‎

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -155,10 +155,7 @@ static inline bool hasResponseVaryStarHeaderValue(const FetchResponse& response)
155155

156156
class FetchTasksHandler : public RefCounted<FetchTasksHandler> {
157157
public:
158-
explicit FetchTasksHandler(Function<void(ExceptionOr<Vector<Record>>&&)>&& callback)
159-
: m_callback(WTFMove(callback))
160-
{
161-
}
158+
static Ref<FetchTasksHandler> create(Ref<DOMCache>&& domCache, CompletionHandler<void(ExceptionOr<Vector<Record>>&&)>&& callback) { return adoptRef(*new FetchTasksHandler(WTFMove(domCache), WTFMove(callback))); }
162159

163160
~FetchTasksHandler()
164161
{
@@ -175,10 +172,12 @@ class FetchTasksHandler : public RefCounted<FetchTasksHandler> {
175172
return m_records.size() - 1;
176173
}
177174

178-
void addResponseBody(size_t position, Ref<SharedBuffer>&& data)
175+
void addResponseBody(size_t position, FetchResponse& response, DOMCacheEngine::ResponseBody&& data)
179176
{
180177
ASSERT(!isDone());
181-
m_records[position].responseBody = WTFMove(data);
178+
auto& record = m_records[position];
179+
record.responseBodySize = m_domCache->connection().computeRecordBodySize(response, data);
180+
record.responseBody = WTFMove(data);
182181
}
183182

184183
bool isDone() const { return !m_callback; }
@@ -190,8 +189,15 @@ class FetchTasksHandler : public RefCounted<FetchTasksHandler> {
190189
}
191190

192191
private:
192+
FetchTasksHandler(Ref<DOMCache>&& domCache, CompletionHandler<void(ExceptionOr<Vector<Record>>&&)>&& callback)
193+
: m_domCache(WTFMove(domCache))
194+
, m_callback(WTFMove(callback))
195+
{
196+
}
197+
198+
Ref<DOMCache> m_domCache;
193199
Vector<Record> m_records;
194-
Function<void(ExceptionOr<Vector<Record>>&&)> m_callback;
200+
CompletionHandler<void(ExceptionOr<Vector<Record>>&&)> m_callback;
195201
};
196202

197203
ExceptionOr<Ref<FetchRequest>> DOMCache::requestFromInfo(RequestInfo&& info, bool ignoreMethod)
@@ -227,15 +233,15 @@ void DOMCache::addAll(Vector<RequestInfo>&& infos, DOMPromiseDeferred<void>&& pr
227233
requests.uncheckedAppend(requestOrException.releaseReturnValue());
228234
}
229235

230-
auto taskHandler = adoptRef(*new FetchTasksHandler([protectedThis = makeRef(*this), this, promise = WTFMove(promise)](ExceptionOr<Vector<Record>>&& result) mutable {
236+
auto taskHandler = FetchTasksHandler::create(*this, [this, promise = WTFMove(promise)](ExceptionOr<Vector<Record>>&& result) mutable {
231237
if (result.hasException()) {
232238
promise.reject(result.releaseException());
233239
return;
234240
}
235241
batchPutOperation(result.releaseReturnValue(), [promise = WTFMove(promise)](ExceptionOr<void>&& result) mutable {
236242
promise.settle(WTFMove(result));
237243
});
238-
}));
244+
});
239245

240246
for (auto& request : requests) {
241247
auto& requestReference = request.get();
@@ -275,7 +281,7 @@ void DOMCache::addAll(Vector<RequestInfo>&& infos, DOMPromiseDeferred<void>&& pr
275281
}
276282
size_t recordPosition = taskHandler->addRecord(toConnectionRecord(request.get(), response, nullptr));
277283

278-
response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = SharedBuffer::create()] (ExceptionOr<ReadableStreamChunk*>&& result) mutable {
284+
response.consumeBodyReceivedByChunk([taskHandler = WTFMove(taskHandler), recordPosition, data = SharedBuffer::create(), response = makeRef(response)] (ExceptionOr<ReadableStreamChunk*>&& result) mutable {
279285
if (taskHandler->isDone())
280286
return;
281287

@@ -287,7 +293,7 @@ void DOMCache::addAll(Vector<RequestInfo>&& infos, DOMPromiseDeferred<void>&& pr
287293
if (auto chunk = result.returnValue())
288294
data->append(reinterpret_cast<const char*>(chunk->data), chunk->size);
289295
else
290-
taskHandler->addResponseBody(recordPosition, WTFMove(data));
296+
taskHandler->addResponseBody(recordPosition, response, WTFMove(data));
291297
});
292298
});
293299
}
@@ -504,7 +510,7 @@ Record DOMCache::toConnectionRecord(const FetchRequest& request, FetchResponse&
504510

505511
auto sizeWithPadding = response.bodySizeWithPadding();
506512
if (!sizeWithPadding) {
507-
sizeWithPadding = m_connection->computeRecordBodySize(response, responseBody, cachedResponse.tainting());
513+
sizeWithPadding = m_connection->computeRecordBodySize(response, responseBody);
508514
response.setBodySizeWithPadding(sizeWithPadding);
509515
}
510516

‎Source/WebCore/Modules/cache/DOMCache.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,8 @@ class DOMCache final : public RefCounted<DOMCache>, public ActiveDOMObject {
5959
using MatchCallback = WTF::Function<void(ExceptionOr<FetchResponse*>)>;
6060
void doMatch(RequestInfo&&, CacheQueryOptions&&, MatchCallback&&);
6161

62+
CacheStorageConnection& connection() { return m_connection.get(); }
63+
6264
private:
6365
DOMCache(ScriptExecutionContext&, String&& name, uint64_t identifier, Ref<CacheStorageConnection>&&);
6466

‎Source/WebCore/Modules/fetch/FetchResponse.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@ class FetchResponse final : public FetchBodyOwner, public CanMakeWeakPtr<FetchRe
102102
void consumeBodyReceivedByChunk(ConsumeDataByChunkCallback&&);
103103

104104
WEBCORE_EXPORT ResourceResponse resourceResponse() const;
105+
ResourceResponse::Tainting tainting() const { return m_internalResponse.tainting(); }
105106

106107
uint64_t bodySizeWithPadding() const { return m_bodySizeWithPadding; }
107108
void setBodySizeWithPadding(uint64_t size) { m_bodySizeWithPadding = size; }

0 commit comments

Comments
 (0)