Bug 174980

Summary: [Fetch API] Implement abortable fetch
Product: WebKit Reporter: Jake Archibald <jaffathecake>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, acombemorel, cdumez, chi187, commit-queue, dbates, denilsonsa, dpaddock, esprehn+autocc, ews, jordi, kangil.han, kondapallykalyan, martin, mike, plaztiksyke, ray, toniotgz, webkit-bug-importer, webkit, youennf, zmyaro, zowers
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://github.com/web-platform-tests/wpt/pull/14646
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Archive of layout-test-results from ews126 for ios-simulator-wk2
none
Patch
none
Archive of layout-test-results from ews122 for ios-simulator-wk2
none
Patch for landing
none
Patch for landing
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Patch for landing none

Description Jake Archibald 2017-07-31 02:31:59 PDT
This includes AbortController & AbortSignal https://dom.spec.whatwg.org/#aborting-ongoing-activities.

Fetch abort tests are at https://github.com/w3c/web-platform-tests/pull/6484.

Fetch abort spec is in progress https://github.com/whatwg/fetch/pull/523.

Mozilla intends to start implementing next month, using the tests rather than the spec.
Comment 1 Antonio Torres 2018-07-24 20:48:13 PDT
Is there a roadmap for these features? I cannot find them in Webkit Status, and Firefox and Chrome already have support for aborts.
Comment 2 molsson 2018-11-25 07:26:06 PST
It's a strange that:

* caniuse.com claims that Safari supports AbortController
* window.AbortController is defined in Safari

...but a simple testcase like this:

http://temp.minimum.se/abort-native.html

Shows that Safari 12.0.1 sends the request even if controller.abort() has been called, i.e. only some "placeholder AbortController" has been added in Safari. It is not working at all.
Comment 3 Alexandre C 2018-11-25 23:41:26 PST
I confirm what molsson mentioned. 
The abort on fetch request on Safari 12.0.1 doesn't work at all.
Comment 4 Sergey Rubanov 2018-12-10 07:10:19 PST
This is really sad that only Webkit is missing Abortable Fetch since April 2018
Comment 5 Radar WebKit Bug Importer 2018-12-19 19:39:38 PST
<rdar://problem/46861402>
Comment 6 youenn fablet 2018-12-21 09:00:39 PST
Created attachment 357950 [details]
Patch
Comment 7 youenn fablet 2018-12-21 09:02:09 PST
Created attachment 357951 [details]
Patch
Comment 8 youenn fablet 2018-12-21 09:15:08 PST
Created attachment 357952 [details]
Patch
Comment 9 youenn fablet 2018-12-21 10:42:40 PST
Created attachment 357959 [details]
Patch
Comment 10 Build Bot 2018-12-21 12:42:26 PST
Comment on attachment 357959 [details]
Patch

Attachment 357959 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10509229

New failing tests:
imported/w3c/web-platform-tests/fetch/api/abort/general-serviceworker.https.html
Comment 11 Build Bot 2018-12-21 12:42:28 PST
Created attachment 357966 [details]
Archive of layout-test-results from ews126 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews126  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 12 youenn fablet 2019-01-03 12:06:08 PST
Created attachment 358267 [details]
Patch
Comment 13 Build Bot 2019-01-03 17:02:55 PST
Comment on attachment 358267 [details]
Patch

Attachment 358267 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10621118

New failing tests:
imported/w3c/web-platform-tests/fetch/api/abort/general-serviceworker.https.html
Comment 14 Build Bot 2019-01-03 17:02:57 PST
Created attachment 358291 [details]
Archive of layout-test-results from ews122 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews122  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 15 youenn fablet 2019-01-03 20:52:14 PST
Comment on attachment 358267 [details]
Patch

Failing test is due to SSL connection issue which seems to happen on iOS in some cases. This does not seem related with the changes but may make the above test flaky on iOS.
Comment 16 Chris Dumez 2019-01-04 09:09:07 PST
Comment on attachment 358267 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=358267&action=review

> Source/WebCore/Modules/fetch/FetchBodyOwner.h:130
> +    struct LoadingError {

Can you really have both a ResourceError and an exception at the same time. If not, could we use a Variant instead?
Comment 17 youenn fablet 2019-01-04 09:58:31 PST
(In reply to Chris Dumez from comment #16)
> Comment on attachment 358267 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=358267&action=review
> 
> > Source/WebCore/Modules/fetch/FetchBodyOwner.h:130
> > +    struct LoadingError {
> 
> Can you really have both a ResourceError and an exception at the same time.
> If not, could we use a Variant instead?

I hesitated on this one.
I think we can only have one.
I went with a struct instead of a Variant as I thought it makes the accessors to resource error/exception simpler and easier to use.
I'll do the Variant experiment.
Comment 18 youenn fablet 2019-01-04 11:20:13 PST
Created attachment 358340 [details]
Patch for landing
Comment 19 youenn fablet 2019-01-04 11:21:40 PST
Created attachment 358342 [details]
Patch for landing
Comment 20 youenn fablet 2019-01-04 11:23:19 PST
(In reply to youenn fablet from comment #19)
> Created attachment 358342 [details]
> Patch for landing

With Variant
Comment 21 Build Bot 2019-01-04 13:13:25 PST
Comment on attachment 358342 [details]
Patch for landing

Attachment 358342 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: https://webkit-queues.webkit.org/results/10631023

New failing tests:
imported/w3c/web-platform-tests/fetch/api/abort/general-serviceworker.https.html
Comment 22 Build Bot 2019-01-04 13:13:27 PST
Created attachment 358354 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.13.6
Comment 23 youenn fablet 2019-01-04 14:43:38 PST
Created attachment 358374 [details]
Patch for landing
Comment 24 youenn fablet 2019-01-04 14:44:11 PST
(In reply to youenn fablet from comment #23)
> Created attachment 358374 [details]
> Patch for landing

Marking imported/w3c/web-platform-tests/fetch/api/abort/general-serviceworker.https.html as flaky on iOS
Comment 25 WebKit Commit Bot 2019-01-04 15:33:11 PST
The commit-queue encountered the following flaky tests while processing attachment 358374 [details]:

fetch/fetch-worker-crash.html bug 187257 (author: youennf@gmail.com)
The commit-queue is continuing to process your patch.
Comment 26 WebKit Commit Bot 2019-01-04 15:33:19 PST
The commit-queue encountered the following flaky tests while processing attachment 358374 [details]:

http/wpt/css/css-animations/start-animation-001.html bug 190903 (authors: dino@apple.com, fred.wang@free.fr, and graouts@apple.com)
The commit-queue is continuing to process your patch.
Comment 27 WebKit Commit Bot 2019-01-04 16:00:48 PST
The commit-queue encountered the following flaky tests while processing attachment 358374 [details]:

http/wpt/css/css-animations/start-animation-001.html bug 190903 (authors: dino@apple.com, fred.wang@free.fr, and graouts@apple.com)
The commit-queue is continuing to process your patch.
Comment 28 WebKit Commit Bot 2019-01-04 16:01:53 PST
Comment on attachment 358374 [details]
Patch for landing

Clearing flags on attachment: 358374

Committed r239644: <https://trac.webkit.org/changeset/239644>
Comment 29 WebKit Commit Bot 2019-01-04 16:01:55 PST
All reviewed patches have been landed.  Closing bug.