Skip to content

Commit

Permalink
Don't use AbortSignal::Follow in fetch and cache-storage
Browse files Browse the repository at this point in the history
AbortSignal's "follow" algorithm is replaced with a new algorithm for
creating a dependent abort signal ([1], [2]), which is implemented as
a new AbortSignal constructor. This CL replaces Follow with the new
ctor, behind the AbortSignalAny REF.

There is a slight compat risk with the change (when the feature ships):
 1. fetch: abort is propagated to dependent signals after abort event
    dispatch for a signal, whereas "follow" propagated abort before (as
    abort algorithms), which is observable. We could alternatively use
    weak abort algorithms in AbortSignal.any(), but that would be more
    complicated and partially limit GC optimizations.

 2. cache: The use of "follow" in cache.addAll was not specced and
    could lead to abort events firing when they shouldn't have. The new
    behavior preserves the specced behavior (aborting outstanding
    requests on failure) without dispatching events when abort is
    triggered internally.

Note: the virtual test expectations match the current expectations
except for the test modified in fetch/api/abort/general.any.js.

[1] whatwg/dom#1152
[2] whatwg/fetch#1646

Bug: 1323391
Low-Coverage-Reason: the uncovered lines in request.cc are covered by WPT tests
Change-Id: I5c25048fcf9f8db759e2f0181ac338c8b603b451
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4515276
Reviewed-by: Nidhi Jaju <nidhijaju@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Scott Haseley <shaseley@chromium.org>
Reviewed-by: Ben Kelly <wanderview@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1144953}
  • Loading branch information
shaseley authored and chromium-wpt-export-bot committed May 16, 2023
1 parent 25eb850 commit c703590
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion fetch/api/abort/general.any.js
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ test(() => {

controller.abort();

assert_array_equals(log, ['clone-aborted', 'original-aborted'], "Abort events fired in correct order");
assert_array_equals(log, ['original-aborted', 'clone-aborted'], "Abort events fired in correct order");
assert_true(request.signal.aborted, 'Signal aborted');
assert_true(clonedRequest.signal.aborted, 'Signal aborted');
}, "Clone aborts with original controller");

0 comments on commit c703590

Please sign in to comment.