http/tests/workers/service/basic-register.html is a flaky failure according to the dashboard: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fworkers%2Fservice%2Fbasic-register.html build: https://build.webkit.org/results/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/r223608%20(5296)/results.html https://build.webkit.org/builders/Apple%20El%20Capitan%20Release%20WK2%20(Tests)/builds/5296 diff: --- /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/http/tests/workers/service/basic-register-expected.txt +++ /Volumes/Data/slave/elcapitan-release-tests-wk2/build/layout-test-results/http/tests/workers/service/basic-register-actual.txt @@ -1,7 +1,7 @@ PASS: No service worker is initially registered for this origin PASS: No service worker is initially registered for this origin in private session Registered! -PASS: A service worker is now registered for this origin +FAIL: No service worker is registered for this origin PASS: No service worker is registered for this origin in private session Registered!
<rdar://problem/35065315>
I cannot reproduce yet. My bet is that there is a race and we sometimes check the store from the WebProcess before it has received the shared memory handle from the Storage Process.
I saw some issues with service workers tests when running all http/tests/workers tests with iterations=5 and -f. Do not remember whether this was this particular test.
Found the problem. Patch is coming shortly.
Created attachment 324203 [details] Patch
Comment on attachment 324203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324203&action=review > Source/WebKit/StorageProcess/ServiceWorker/WebSWOriginStore.cpp:47 > + m_store.flushPendingChanges(); scheduleAddition will add to the pending list and will start the timer. Then flushPendingChanges will stop the timer and process the operations. Is there a way to simplify this? > Source/WebKit/StorageProcess/ServiceWorker/WebSWOriginStore.cpp:50 > +void WebSWOriginStore::addAll(const Vector<SecurityOrigin>& origins) addAll is not used AFAICT. If we keep it and have a removeAll, is there still a need for the timer?
Comment on attachment 324203 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=324203&action=review >> Source/WebKit/StorageProcess/ServiceWorker/WebSWOriginStore.cpp:47 >> + m_store.flushPendingChanges(); > > scheduleAddition will add to the pending list and will start the timer. > Then flushPendingChanges will stop the timer and process the operations. > Is there a way to simplify this? Yes, I will look into factoring this better in a follow-up. I need to consider the VisitedLinkHashStore which uses the same data structure. >> Source/WebKit/StorageProcess/ServiceWorker/WebSWOriginStore.cpp:50 >> +void WebSWOriginStore::addAll(const Vector<SecurityOrigin>& origins) > > addAll is not used AFAICT. > If we keep it and have a removeAll, is there still a need for the timer? The Timer is at the lower level object (SharedStringHashStore), which is used by the VisitedLinkHashStore. This is where the timer is coming from.
Comment on attachment 324203 [details] Patch Clearing flags on attachment: 324203 Committed r223690: <https://trac.webkit.org/changeset/223690>
All reviewed patches have been landed. Closing bug.