Skip to content

Commit

Permalink
Correctly reject in-progress body methods with AbortError
Browse files Browse the repository at this point in the history
Prior to this change, response body methods such as
response.body.arrayBuffer() would reject with a TypeError if the fetch
was aborted. This change makes them correctly reject with an AbortError
instead.

This implements stage #3 of the "Body Cancellation" section of the
design doc:

https://docs.google.com/document/d/1OuoCG2uiijbAwbCw9jaS7tHEO0LBO_4gMNio1ox0qlY/edit#heading=h.fvc7d7q07oio

Bug: 817687
Change-Id: Ifde322d9c22485a8ba9d14fd4ffca65c4fb4745a
Reviewed-on: https://chromium-review.googlesource.com/954765
Commit-Queue: Adam Rice <ricea@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Yutaka Hirano <yhirano@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544335}
  • Loading branch information
ricea authored and chromium-wpt-export-bot committed Mar 20, 2018
1 parent 7cb5db5 commit 32b86b8
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 0 deletions.
1 change: 1 addition & 0 deletions lint.whitelist
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ SET TIMEOUT: screen-orientation/lock-bad-argument.html
SET TIMEOUT: screen-orientation/onchange-event.html
SET TIMEOUT: screen-orientation/resources/sandboxed-iframe-locking.html
SET TIMEOUT: secure-contexts/basic-popup-and-iframe-tests.https.js
SET TIMEOUT: service-workers/cache-storage/script-tests/cache-abort.js
SET TIMEOUT: service-workers/service-worker/activation.https.html
SET TIMEOUT: service-workers/service-worker/fetch-frame-resource.https.html
SET TIMEOUT: service-workers/service-worker/fetch-request-redirect.https.html
Expand Down
81 changes: 81 additions & 0 deletions service-workers/cache-storage/script-tests/cache-abort.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
if (self.importScripts) {
importScripts('/resources/testharness.js');
importScripts('../resources/test-helpers.js');
importScripts('/common/utils.js');
}

// We perform the same tests on put, add, addAll. Parameterise the tests to
// reduce repetition.
const methodsToTest = {
put: async (cache, request) => {
const response = await fetch(request);
return cache.put(request, response);
},
add: async (cache, request) => cache.add(request),
addAll: async (cache, request) => cache.addAll([request]),
};

for (const method in methodsToTest) {
const perform = methodsToTest[method];

cache_test(async (cache, test) => {
const controller = new AbortController();
const signal = controller.signal;
controller.abort();
const request = new Request('../resources/simple.txt', { signal });
return promise_rejects(test, 'AbortError', perform(cache, request),
`${method} should reject`);
}, `${method}() on an already-aborted request should reject with AbortError`);

cache_test(async (cache, test) => {
const controller = new AbortController();
const signal = controller.signal;
const request = new Request('../resources/simple.txt', { signal });
const promise = perform(cache, request);
controller.abort();
return promise_rejects(test, 'AbortError', promise,
`${method} should reject`);
}, `${method}() synchronously followed by abort should reject with ` +
`AbortError`);

cache_test(async (cache, test) => {
const controller = new AbortController();
const signal = controller.signal;
const stateKey = token();
const abortKey = token();
const request = new Request(
`../../../fetch/api/resources/infinite-slow-response.py?stateKey=${stateKey}&abortKey=${abortKey}`,
{ signal });

const promise = perform(cache, request);

// Wait for the server to start sending the response body.
let opened = false;
do {
// Normally only one fetch to 'stash-take' is needed, but the fetches
// will be served in reverse order sometimes
// (i.e., 'stash-take' gets served before 'infinite-slow-response').

const response =
await fetch(`../../../fetch/api/resources/stash-take.py?key=${stateKey}`);
const body = await response.json();
if (body === 'open') opened = true;
} while (!opened);

// Sadly the above loop cannot guarantee that the browser has started
// processing the response body. This delay is needed to make the test
// failures non-flaky in Chrome version 66. My deepest apologies.
await new Promise(resolve => setTimeout(resolve, 250));

controller.abort();

await promise_rejects(test, 'AbortError', promise,
`${method} should reject`);

// infinite-slow-response.py doesn't know when to stop.
return fetch(`../../../fetch/api/resources/stash-put.py?key=${abortKey}`);
}, `${method}() followed by abort after headers received should reject ` +
`with AbortError`);
}

done();
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!DOCTYPE html>
<title>Cache Storage: Abort</title>
<link rel="help" href="https://fetch.spec.whatwg.org/#request-signal">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../../service-worker/resources/test-helpers.sub.js"></script>
<script>
service_worker_test('../script-tests/cache-abort.js');
</script>
8 changes: 8 additions & 0 deletions service-workers/cache-storage/window/cache-abort.https.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<title>Cache Storage: Abort</title>
<link rel="help" href="https://fetch.spec.whatwg.org/#request-signal">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="../resources/test-helpers.js"></script>
<script src="/common/utils.js"></script>
<script src="../script-tests/cache-abort.js"></script>
8 changes: 8 additions & 0 deletions service-workers/cache-storage/worker/cache-abort.https.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<!DOCTYPE html>
<title>>Cache Storage: Abort</title>
<link rel="help" href="https://fetch.spec.whatwg.org/#request-signal">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script>
fetch_tests_from_worker(new Worker('../script-tests/cache-abort.js'));
</script>

0 comments on commit 32b86b8

Please sign in to comment.