Summary: | http/tests/workers/service/basic-register.html is a flaky failure. | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Matt Lewis <jlewis3> | ||||
Component: | New Bugs | Assignee: | Chris Dumez <cdumez> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | cdumez, webkit-bug-importer, youennf | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=177876 | ||||||
Attachments: |
|
Description
Matt Lewis
2017-10-18 17:58:35 PDT
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. |