Bug 202787 – Do not timeout a load intercepted by service worker that receives a response

• Component: Service Workers
• Status: RESOLVED FIXED

[youenn fablet]

Do not timeout a load intercepted by service worker that receives a response.

[youenn fablet]

Created attachment 380628 [details]
Patch

[Chris Dumez]

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?

[youenn fablet]

(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.

[youenn fablet]

(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.

[youenn fablet]

Created attachment 380669 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 380669 [details]
Patch for landing

Clearing flags on attachment: 380669

Committed r250985: <https://trac.webkit.org/changeset/250985>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/56165481>