Increase heart beat timer at service worker startup time
Created attachment 386833 [details] Patch
Created attachment 386842 [details] Patch
Could you provide a link to something you think is caused by this timer being too slow?
https://build.webkit.org/results/Apple%20iOS%2013%20Simulator%20Debug%20WK2%20(Tests)/r254057%20(1555)/http/wpt/service-workers/update-service-worker.https-pretty-diff.html is an example.
Comment on attachment 386842 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=386842&action=review > Source/WebCore/workers/service/context/ServiceWorkerThread.h:110 > Seconds m_heartBeatTimeout { heartBeatTimeout }; > + Seconds m_heartBeatStartTimeout { heartBeatTimeout }; Your ChangeLog entry indicates that you just replaced heartBeatTimeout's initializer from 60 to 300, but you did something more complicated than that. What does heartBeatStartTimeout mean?
Created attachment 386950 [details] Patch
(In reply to Alex Christensen from comment #5) > Comment on attachment 386842 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=386842&action=review > > > Source/WebCore/workers/service/context/ServiceWorkerThread.h:110 > > Seconds m_heartBeatTimeout { heartBeatTimeout }; > > + Seconds m_heartBeatStartTimeout { heartBeatTimeout }; > > Your ChangeLog entry indicates that you just replaced heartBeatTimeout's > initializer from 60 to 300, but you did something more complicated than > that. What does heartBeatStartTimeout mean? Right, I initially did that but this messed up with the sw spinning tests. I beefed up the change log.
Created attachment 387215 [details] Patch
(In reply to youenn fablet from comment #8) > Created attachment 387215 [details] > Patch Reduced the patch size.
Comment on attachment 387215 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387215&action=review > Source/WebCore/workers/service/context/ServiceWorkerThread.cpp:79 > + , m_heartBeatTimeout(SWContextManager::singleton().connection()->shouldUseShortTimeout() ? heartBeatTimeoutForTest : heartBeatTimeout * 5) Isn't this a bit confusing? The parameter you pass in isn't what is actually used. Wouldn't it be better to have the calling site pass in a value that is 5x bigger?
Comment on attachment 387215 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=387215&action=review > Source/WebCore/ChangeLog:8 > + As seen in the bots, especially slow bots, like ios sim debug, 60 seconds is not always sufficient to start service workers. Isn't the issue that we're using the "short" timeout of 1 second when running the layout tests and 1 second is simply not enough time sometimes to start a service worker in iOS Debug?
(In reply to Chris Dumez from comment #11) > Comment on attachment 387215 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=387215&action=review > > > Source/WebCore/ChangeLog:8 > > + As seen in the bots, especially slow bots, like ios sim debug, 60 seconds is not always sufficient to start service workers. > > Isn't the issue that we're using the "short" timeout of 1 second when > running the layout tests and 1 second is simply not enough time sometimes to > start a service worker in iOS Debug? Oh right, let's only use the short timeout values for the spin service worker tests (and disable those tests in iOS sim debug).
Created attachment 387320 [details] Patch
Comment on attachment 387320 [details] Patch Clearing flags on attachment: 387320 Committed r254341: <https://trac.webkit.org/changeset/254341>
All reviewed patches have been landed. Closing bug.
<rdar://problem/58480107>