Bug 216266 – XHR.timeout is affected by long tasks

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Samuel]

Created attachment 408218 [details]
Files to easily reproduce the bug

Steps to reproduce:

A zip file with the front and back-end needed to reproduce this issue is attached.

Open the zip file, and serve that folder (in mac you can run "http-serve") and visit the index.html file.
The folder has a "server.js" file. Run it with "node server.js."
Refresh multiple times, and you will see that sometimes the timeout callback is called, even though it shouldn't (console will log "timeout!").
What the script and server reproduce:

The timeout is set to 1000ms.
The request is made.
A blocking script is running for 2000ms (a while loop).
The server responds in 100ms (concurrently to step 3).
The timeout callback is called, and the request is canceled.
Actual results:

The request is canceled and the timeout callback is called.

Expected results:

The request should not be canceled (Chrome behaves correctly, Firefox has the same issue).
It seems like the timer counting the time of the request is running on the main thread and therefore affected by long tasks, this should not be the case.

[Radar WebKit Bug Importer]

<rdar://problem/68908150>

[youenn fablet]

Created attachment 408903 [details]
Patch

[EWS Watchlist]

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

[youenn fablet]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/25561

[youenn fablet]

Created attachment 408908 [details]
Patch

[youenn fablet]

Created attachment 408913 [details]
Patch

[youenn fablet]

Created attachment 408918 [details]
Patch

[Alex Christensen]

Comment on attachment 408918 [details]
Patch

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

> Source/WebCore/loader/ThreadableLoader.h:79
> +        virtual void computeIsDone() = 0;

I think it would be much better to have a CompletionHandler<void(bool)>&& here than introduce a new function on the client that this will call as a callback.

> Source/WebCore/loader/WorkerThreadableLoader.h:63
> +        void notifyIsDone(bool isDone);

ditto

[youenn fablet]

(In reply to Alex Christensen from comment #8)
> Comment on attachment 408918 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=408918&action=review
> 
> > Source/WebCore/loader/ThreadableLoader.h:79
> > +        virtual void computeIsDone() = 0;
> 
> I think it would be much better to have a CompletionHandler<void(bool)>&&
> here than introduce a new function on the client that this will call as a
> callback.
> 
> > Source/WebCore/loader/WorkerThreadableLoader.h:63
> > +        void notifyIsDone(bool isDone);
> 
> ditto

I initially tried completion handlers within WorkerThreadableLoader that but this does not work well given ThreadableLoader/ThreadableLoaderClientWrapper model.
The alternative is to do like other things I did in the past and keep a map of completion handlers tied to the ScriptExecutionContext.

I chose to keep the existing model but could be convinced to go with the completion handler map.

[EWS]

Committed r267227: <https://trac.webkit.org/changeset/267227>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 408918 [details].