|Summary:||XHR.timeout is affected by long tasks|
|Component:||Page Loading||Assignee:||youenn fablet <youennf>|
|Severity:||Normal||CC:||achristensen, ap, beidson, cdumez, clopez, ews-watchlist, japhet, webkit-bug-importer, youennf|
Description Samuel 2020-09-08 01:18:07 PDT
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.
Comment 3 EWS Watchlist 2020-09-16 01:07:57 PDT
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
Comment 4 youenn fablet 2020-09-16 01:13:30 PDT
Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/25561
Comment 8 Alex Christensen 2020-09-17 11:14:44 PDT
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
Comment 9 youenn fablet 2020-09-17 11:54:54 PDT
(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.