Commit bc660fc7 authored by Ben Kelly's avatar Ben Kelly Committed by Commit Bot

CacheStorage: Make addAll() support throttled network requests.

Cache.addAll() must wait for the all requests to succeed before starting
to store the results in CacheStorage.  Previously this was done by
waiting for all the Response objects to become available and only then
would it begin loading the bodies into blobs.  This does not work with
throttling since throttling requires bodies to be loaded in order to
make progress.

The main change in this CL is to refactor things so we now immediately
load the body to a blob when a Response becomes available.  We then wait
for all Responses and their bodies before proceeding to store the
result.  This should not use too much memory since loading a Response to
a blob streams data to the BlobRegistry, which may then subsequently
stream to disk if necessary.

One notable change here is that code cache generation now requires
loading the body into an ArrayBuffer from a blob.  Previously we loaded
the response to the ArrayBuffer and then converted that into a blob.
This will be slightly slower for putting scripts in CacheStorage, but
that should be fine since generally we do not optimize for write
performance.

Other changes in this CL:

* Adds a cache.addAll() test for the throttled service worker case.
* Adds `cache:no-store` to the existing throttled service worker test
  since that more easily triggers true throttling behavior.  This in
  turn also required draining the response bodies in the test.
* Refactored cache add(), addAll(), and put() to all go through the
  same set of objects where appropriate.
* FetchHandler is used in add/addAll to wait for each individual fetch()
  to complete.
* ResponseBodyLoader is used in add/addAll/put to take a Response and
  load its body as a blob.
* BarrierCallbackForPutResponse waits for a list of Response and blob
  objects to become available before calling PutImpl().
* BarrierCallbackForPutComplete waits for any code cache generation and
  the final cache operation to complete.

Bug: 1035448
Change-Id: I8c9b6b5831cfd21bd266e526a138de4cbf03171f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2404267
Commit-Queue: Ben Kelly <wanderview@chromium.org>
Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#811379}
parent 7da4aa18
......@@ -2967,6 +2967,7 @@ class ServiceWorkerThrottlingTest : public ServiceWorkerBrowserTest {
"Connection: close\r\n"
"Content-Length: 32\r\n"
"Content-Type: text/html\r\n"
"Cache-Control: no-store\r\n"
"\r\n"
"<title>ERROR</title>Hello world.";
std::move(send_).Run(kPageResponse, std::move(done_));
......@@ -3042,4 +3043,43 @@ IN_PROC_BROWSER_TEST_F(ServiceWorkerThrottlingTest, ThrottleInstalling) {
observer->Wait();
}
IN_PROC_BROWSER_TEST_F(ServiceWorkerThrottlingTest,
ThrottleInstallingWithCacheAddAll) {
// Register a service worker that loads 3 resources in its install
// handler via cache.addAll(). The test server will cause these loads
// to block which should trigger throttling on the third request.
RegisterServiceWorkerAndWaitForState(
"/service_worker/throttling_blocking_cache_addall_sw.js",
"/service_worker/throttling_blocking_cache_addall",
ServiceWorkerVersion::INSTALLING);
// Register a second service worker that also loads 3 resources in
// its install handler using cache.addAll(). The test server will not
// block these loads and the worker should progress to the activated state.
//
// This second service worker is used to wait for the first worker
// to potentially request its resources. By the time the second worker
// activates the first worker should have requested its resources and
// triggered throttling. This avoids the need for an arbitrary timeout.
RegisterServiceWorkerAndWaitForState(
"/service_worker/throttling_non_blocking_cache_addall_sw.js",
"/service_worker/throttling_non_blocking_cache_addall",
ServiceWorkerVersion::ACTIVATED);
// If throttling worked correctly then there should only be 2 outstanding
// requests blocked by the test server.
EXPECT_EQ(2, GetBlockingResponseCount());
auto observer = base::MakeRefCounted<WorkerStateObserver>(
wrapper(), ServiceWorkerVersion::ACTIVATED);
observer->Init();
// Stop blocking the resources loaded by the first service worker.
StopBlocking();
// Verify that throttling correctly notes when resources can load and
// the first service worker fully activates.
observer->Wait();
}
} // namespace content
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
self.addEventListener('install', evt => {
evt.waitUntil(async function() {
const c = await caches.open('foo');
return c.addAll([
'./empty.js?1&block',
'./empty.js?2&block',
'./empty.js?3&block',
]);
}());
});
......@@ -5,9 +5,9 @@
self.addEventListener('install', evt => {
evt.waitUntil(async function() {
return Promise.all([
fetch('./foo/1?block'),
fetch('./foo/2?block'),
fetch('./foo/3?block'),
fetch('./foo/1?block').then(r => r.blob()),
fetch('./foo/2?block').then(r => r.blob()),
fetch('./foo/3?block').then(r => r.blob()),
]);
}());
});
// Copyright 2020 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
self.addEventListener('install', evt => {
evt.waitUntil(async function() {
const c = await caches.open('bar');
return c.addAll([
'./empty.js?1',
'./empty.js?2',
'./empty.js?3',
]);
}());
});
......@@ -5,9 +5,9 @@
self.addEventListener('install', evt => {
evt.waitUntil(async function() {
return Promise.all([
fetch('./foo/1'),
fetch('./foo/2'),
fetch('./foo/3'),
fetch('./foo/1').then(r => r.blob()),
fetch('./foo/2').then(r => r.blob()),
fetch('./foo/3').then(r => r.blob()),
]);
}());
});
......@@ -44,6 +44,7 @@ class CacheStorageBlobClientList;
class ExceptionState;
class Response;
class Request;
class ScriptPromiseResolver;
class ScriptState;
typedef RequestOrUSVString RequestInfo;
......@@ -87,11 +88,11 @@ class MODULES_EXPORT Cache final : public ScriptWrappable {
void Trace(Visitor*) const override;
private:
class BarrierCallbackForPut;
class BlobHandleCallbackForPut;
class BarrierCallbackForPutResponse;
class BarrierCallbackForPutComplete;
class CodeCacheHandleCallbackForPut;
class FetchResolvedForAdd;
friend class FetchResolvedForAdd;
class ResponseBodyLoader;
class FetchHandler;
ScriptPromise MatchImpl(ScriptState*,
const Request*,
......@@ -106,12 +107,13 @@ class MODULES_EXPORT Cache final : public ScriptWrappable {
ScriptPromise DeleteImpl(ScriptState*,
const Request*,
const CacheQueryOptions*);
ScriptPromise PutImpl(ScriptState*,
const String& method_name,
const HeapVector<Member<Request>>&,
const HeapVector<Member<Response>>&,
ExceptionState&,
int64_t trace_id);
void PutImpl(ScriptPromiseResolver*,
const String& method_name,
const HeapVector<Member<Request>>&,
const HeapVector<Member<Response>>&,
const WTF::Vector<scoped_refptr<BlobDataHandle>>& blob_list,
ExceptionState&,
int64_t trace_id);
ScriptPromise KeysImpl(ScriptState*,
const Request*,
const CacheQueryOptions*);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment