| Summary: | Do not timeout a load intercepted by service worker that receives a response | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | youenn fablet <youennf> | ||||||
| Component: | Service Workers | Assignee: | youenn fablet <youennf> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | achristensen, beidson, cdumez, commit-queue, ggaren, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| See Also: | https://bugs.webkit.org/show_bug.cgi?id=202188 | ||||||||
| Attachments: |
|
||||||||
|
Description
youenn fablet
2019-10-10 02:57:08 PDT
Created attachment 380628 [details]
Patch
Comment on attachment 380628 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380628&action=review > Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:94 > m_timeoutTimer.stop(); Do we need this? Can didFinish() be called without didReceiveResponse() getting called first? Maybe this could be an assertion? (In reply to Chris Dumez from comment #2) > Comment on attachment 380628 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=380628&action=review > > > Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:94 > > m_timeoutTimer.stop(); > > Do we need this? Can didFinish() be called without didReceiveResponse() > getting called first? Maybe this could be an assertion? Rob told me that this happens in a few case apparently and works fine for ResourceLoader. Stopping it there is mostly for consistency and is not strictly needed since didFinish will synchronously destroy the ServiceWorkerFetchTask. (In reply to youenn fablet from comment #3) > (In reply to Chris Dumez from comment #2) > > Comment on attachment 380628 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=380628&action=review > > > > > Source/WebKit/NetworkProcess/ServiceWorker/ServiceWorkerFetchTask.cpp:94 > > > m_timeoutTimer.stop(); > > > > Do we need this? Can didFinish() be called without didReceiveResponse() > > getting called first? Maybe this could be an assertion? > > Rob told me that this happens in a few case apparently and works fine for > ResourceLoader. That said, our service worker fetch handler is probably enforcing to either send a didReceiveResponse or didFail. Will add an ASSERT. Created attachment 380669 [details]
Patch for landing
Comment on attachment 380669 [details] Patch for landing Clearing flags on attachment: 380669 Committed r250985: <https://trac.webkit.org/changeset/250985> All reviewed patches have been landed. Closing bug. |