RESOLVED FIXED 181166
Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html is flaky
https://bugs.webkit.org/show_bug.cgi?id=181166
Summary Layout Test imported/w3c/web-platform-tests/service-workers/service-worker/re...
Matt Lewis
Reported 2017-12-26 17:46:23 PST
The following layout test is flaky on WK2 imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html Probable cause: https://trac.webkit.org/changeset/226274/webkit Flakiness Dashboard: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=imported%2Fw3c%2Fweb-platform-tests%2Fservice-workers%2Fservice-worker%2Fregister-same-scope-different-script-url.https.html This test recently became extremely flaky. https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r226294%20(1953)/results.html https://build.webkit.org/builders/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/builds/1953 --- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https-expected.txt +++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https-actual.txt @@ -1,7 +1,9 @@ + +Harness Error (TIMEOUT), message = null PASS Register different scripts concurrently PASS Register then register new script URL PASS Register then register new script URL that 404s FAIL Register then register new script that does not install assert_unreached: unexpected rejection: assert_equals: on redundant, installing should be null expected null but got object "[object ServiceWorker]" Reached unreachable code -PASS Register same-scope new script url effect on controller +TIMEOUT Register same-scope new script url effect on controller Test timed out
Attachments
Patch (18.91 KB, patch)
2018-02-04 13:05 PST, Chris Dumez
no flags
Patch (18.91 KB, patch)
2018-02-05 09:36 PST, Chris Dumez
no flags
Matt Lewis
Comment 1 2017-12-26 17:53:33 PST
Chris Dumez
Comment 2 2018-02-02 13:58:23 PST
I managed to reproduce the flakiness like so: run-webkit-tests -gf --repeat-each=25 imported/w3c/web-platform-tests/service-workers/service-worker/register-same-scope-different-script-url.https.html
Radar WebKit Bug Importer
Comment 3 2018-02-02 13:59:55 PST
Chris Dumez
Comment 4 2018-02-02 14:25:12 PST
Looks like the registration.unregister() promise at the end of the last test sometimes does not get resolved.
Chris Dumez
Comment 5 2018-02-02 15:30:23 PST
If I disable soft-update, then it is no longer flaky. It seems that SoftUpdate jobs sometimes "hang" the jobQueue so that it is no longer processing new jobs.
Chris Dumez
Comment 6 2018-02-04 13:05:34 PST
youenn fablet
Comment 7 2018-02-05 09:27:31 PST
Comment on attachment 333054 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=333054&action=review > Source/WebCore/workers/service/ServiceWorkerJobData.cpp:39 > : m_identifier { connectionIdentifier, generateThreadSafeObjectIdentifier<ServiceWorkerJobIdentifierType>() } Preexisting issue but seems a bit weird to have m_identifier here and sourceContext which are both member fields of ServiceWorkerJobData. > Source/WebCore/workers/service/server/SWServer.cpp:503 > + // Abort if the job that scheduled this has not been cancelled. s/has not/has/ > Source/WebCore/workers/service/server/SWServer.cpp:633 > + jobQueue->cancelJobsFromServiceWorker(worker.identifier()); Maybe just me but I would read it more easily with something like cancelServiceWorkerJobs. Ditto below.
Chris Dumez
Comment 8 2018-02-05 09:35:49 PST
(In reply to youenn fablet from comment #7) > Comment on attachment 333054 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=333054&action=review > > > Source/WebCore/workers/service/ServiceWorkerJobData.cpp:39 > > : m_identifier { connectionIdentifier, generateThreadSafeObjectIdentifier<ServiceWorkerJobIdentifierType>() } > > Preexisting issue but seems a bit weird to have m_identifier here and > sourceContext which are both member fields of ServiceWorkerJobData. > > > Source/WebCore/workers/service/server/SWServer.cpp:503 > > + // Abort if the job that scheduled this has not been cancelled. > > s/has not/has/ > > > Source/WebCore/workers/service/server/SWServer.cpp:633 > > + jobQueue->cancelJobsFromServiceWorker(worker.identifier()); > > Maybe just me but I would read it more easily with something like > cancelServiceWorkerJobs. Ditto below. I personally prefer cancelJobsFromServiceWorker to cancelServiceWorkerJobs. "cancelServiceWorkerJobs" sounds very generic given that we are in service worker code.
Chris Dumez
Comment 9 2018-02-05 09:36:55 PST
WebKit Commit Bot
Comment 10 2018-02-05 10:12:12 PST
Comment on attachment 333086 [details] Patch Clearing flags on attachment: 333086 Committed r228101: <https://trac.webkit.org/changeset/228101>
WebKit Commit Bot
Comment 11 2018-02-05 10:12:14 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.