| Summary: | Simplify VisitedLinkStore process registration logic | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Dumez <cdumez> | ||||
| Component: | WebKit2 | Assignee: | Chris Dumez <cdumez> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | achristensen, beidson, commit-queue, ggaren, webkit-bug-importer, youennf | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Nightly Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
|
Description
Chris Dumez
2019-11-21 14:03:00 PST
Created attachment 384089 [details]
Patch
Comment on attachment 384089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384089&action=review > Source/WebKit/UIProcess/WebProcessProxy.cpp:-386 > - m_visitedLinkStoresWithUsers.clear(); This means the visitedLinkStore will keep shutdown but still existing processes. Can this create some changes of behavior? Comment on attachment 384089 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=384089&action=review r=me > Source/WebKit/UIProcess/VisitedLinkStore.cpp:46 > + RELEASE_ASSERT(m_processes.computesEmpty()); FWIW, I think this function was supposed to be named computeIsEmpty(). >> Source/WebKit/UIProcess/WebProcessProxy.cpp:-386 >> - m_visitedLinkStoresWithUsers.clear(); > > This means the visitedLinkStore will keep shutdown but still existing processes. > Can this create some changes of behavior? Interesting point -- but I can't think of any consequence. We might try to send a message to shut a process -- and then the message should be dropped. (In reply to youenn fablet from comment #2) > Comment on attachment 384089 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384089&action=review > > > Source/WebKit/UIProcess/WebProcessProxy.cpp:-386 > > - m_visitedLinkStoresWithUsers.clear(); > > This means the visitedLinkStore will keep shutdown but still existing > processes. > Can this create some changes of behavior? This is explained in the change log. It does not matter because IPC send() is a no-op for processes that are no longer running. So the VisitedLinkStore may now try to send IPC to processes that are no longer running but it will not matter. Comment on attachment 384089 [details] Patch Clearing flags on attachment: 384089 Committed r252812: <https://trac.webkit.org/changeset/252812> All reviewed patches have been landed. Closing bug. |