Bug 175546

Summary: XHR should only fire an abort event if the cancellation was requested by the client
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebCore Misc.Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 175482    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Description Chris Dumez 2017-08-14 13:44:21 PDT
XHR should only fire an abort event if the cancellation was requested by the client, otherwise it should fire an error event.

Specification:
- https://xhr.spec.whatwg.org/#handle-errors
- https://xhr.spec.whatwg.org/#the-abort()-method

Blink matches the specification.
Comment 1 Chris Dumez 2017-08-14 15:16:59 PDT
Created attachment 318067 [details]
Patch
Comment 2 Chris Dumez 2017-08-14 15:24:11 PDT
Created attachment 318070 [details]
Patch
Comment 3 Chris Dumez 2017-08-14 16:11:47 PDT
Created attachment 318079 [details]
Patch
Comment 4 youenn fablet 2017-08-14 16:39:30 PDT
Comment on attachment 318079 [details]
Patch

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

> Source/WebCore/xml/XMLHttpRequest.cpp:1009
> +    // The XHR specification says we should only fire an abort event if the cancelation was requested by the client.

Not sure this comment is needed.

For cancellation errors that are not due to client, they are probably due to some safety security nets.
Would it make sense to long them in the console or is it redundant with web inspector?

> LayoutTests/fast/frames/frame-unload-crash-expected.txt:-2
> -CONSOLE MESSAGE: line 13: XMLHttpRequest cannot load frame-unload-crash-2.html due to access control checks.

This comment might be useful if it is true.
It might indeed be that the below abort is due to access control checks.

> LayoutTests/http/tests/navigation/page-cache-xhr-in-pagehide.html:65
> +    xhr.send();*/

This code is commented out.
Would this test be useful as WPT?
Comment 5 Chris Dumez 2017-08-14 16:50:49 PDT
Comment on attachment 318079 [details]
Patch

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

>> LayoutTests/fast/frames/frame-unload-crash-expected.txt:-2
>> -CONSOLE MESSAGE: line 13: XMLHttpRequest cannot load frame-unload-crash-2.html due to access control checks.
> 
> This comment might be useful if it is true.
> It might indeed be that the below abort is due to access control checks.

The load is cancelled due to a navigation. The console message was inaccurate.

>> LayoutTests/http/tests/navigation/page-cache-xhr-in-pagehide.html:65
>> +    xhr.send();*/
> 
> This code is commented out.
> Would this test be useful as WPT?

Oops. Will remove this commented code.

PageCache is browser-specific so I do not think this should be a WPT.
Comment 6 Chris Dumez 2017-08-14 16:51:32 PDT
Created attachment 318086 [details]
Patch
Comment 7 Chris Dumez 2017-08-14 18:36:10 PDT
Comment on attachment 318086 [details]
Patch

Clearing flags on attachment: 318086

Committed r220731: <http://trac.webkit.org/changeset/220731>
Comment 8 Chris Dumez 2017-08-14 18:36:11 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-08-14 18:38:34 PDT
<rdar://problem/33888764>