Summary: | Spew: Unhandled web process message 'VisitedLinkTableController:VisitedLinkStateChanged' | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Joseph Pecoraro <joepeck> | ||||||
Component: | History | Assignee: | Chris Dumez <cdumez> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, ap, beidson, bfulgham, cdumez, commit-queue, dbates, ggaren, joepeck, thorton, tsavell, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Attachments: |
|
Description
Joseph Pecoraro
2019-02-18 13:12:02 PST
Likely a PSON regression, I'll investigate as soon as I have spare cycles. The WebProcess does not have a VisitedLinkTableController, likely because it does not have any pages any more. Before PSON, we use to terminate such processes. However, with PSON, we may keep processes without pages alive due to: - Suspended pages - WebProcess cache - Process pre-warming This basically mean we're sending VisitedLinkTableController:VisitedLinkStateChanged IPC to WebProcesses who do not care about such notifications. I do not believe this is actually harmful but the error logging is indeed unfortunate. Currently, WebProcessProxy::shutDown() is in charge of unregistering the process from the visitedLinkStores. I guess we have to do so when the process no longer has any WebPage. Created attachment 365086 [details]
Patch
Comment on attachment 365086 [details]
Patch
r=me
Comment on attachment 365086 [details] Patch Clearing flags on attachment: 365086 Committed r243142: <https://trac.webkit.org/changeset/243142> All reviewed patches have been landed. Closing bug. Reverted r243142 for reason: Caused assertion hits in WK2 Debug Committed r243154: <https://trac.webkit.org/changeset/243154> Comment on attachment 365086 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=365086&action=review > Source/WebKit/UIProcess/WebProcessProxy.cpp:404 > + auto users = m_visitedLinkStoresWithUsers.ensure(&visitedLinkStore, [] { this should be auto& XD Created attachment 365201 [details]
Patch
Committed r243159: <https://trac.webkit.org/changeset/243159> It looks like the changes in https://trac.webkit.org/changeset/243159/webkit broke 16 API tests on Debug. Build: https://build.webkit.org/builders/Apple%20Mojave%20Debug%20WK2%20%28Tests%29/builds/2006 I was able to reproduce and regress this down to 243159 locally. (In reply to Truitt Savell from comment #12) > It looks like the changes in https://trac.webkit.org/changeset/243159/webkit > > broke 16 API tests on Debug. > > Build: > https://build.webkit.org/builders/Apple%20Mojave%20Debug%20WK2%20%28Tests%29/ > builds/2006 > > I was able to reproduce and regress this down to 243159 locally. I can work on fixing these first thing tomorrow morning. If this is not soon enough, feel free to revert my change. (In reply to Chris Dumez from comment #13) > (In reply to Truitt Savell from comment #12) > > It looks like the changes in https://trac.webkit.org/changeset/243159/webkit > > > > broke 16 API tests on Debug. > > > > Build: > > https://build.webkit.org/builders/Apple%20Mojave%20Debug%20WK2%20%28Tests%29/ > > builds/2006 > > > > I was able to reproduce and regress this down to 243159 locally. > > I can work on fixing these first thing tomorrow morning. If this is not soon > enough, feel free to revert my change. Looking at these now. (In reply to Chris Dumez from comment #14) > (In reply to Chris Dumez from comment #13) > > (In reply to Truitt Savell from comment #12) > > > It looks like the changes in https://trac.webkit.org/changeset/243159/webkit > > > > > > broke 16 API tests on Debug. > > > > > > Build: > > > https://build.webkit.org/builders/Apple%20Mojave%20Debug%20WK2%20%28Tests%29/ > > > builds/2006 > > > > > > I was able to reproduce and regress this down to 243159 locally. > > > > I can work on fixing these first thing tomorrow morning. If this is not soon > > enough, feel free to revert my change. > > Looking at these now. Follow-up fix landed in <https://trac.webkit.org/changeset/243202>. |