Bug 194787

Summary: Spew: Unhandled web process message 'VisitedLinkTableController:VisitedLinkStateChanged'
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: HistoryAssignee: 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 Flags
Patch
none
Patch none

Description Joseph Pecoraro 2019-02-18 13:12:02 PST
When navigating in a Debug build I see lot of spew in the Console:

    ERROR: Unhandled web process message 'VisitedLinkTableController:VisitedLinkStateChanged'
    Source/WebKit/WebProcess/WebProcess.cpp(740) : virtual void WebKit::WebProcess::didReceiveMessage(IPC::Connection &, IPC::Decoder &)

Steps to Reproduce:
1. Open about:blank
2. Navigate to cnn.com
  => Spew in console about unhandled message
Comment 1 Radar WebKit Bug Importer 2019-02-18 13:14:43 PST
<rdar://problem/48175520>
Comment 2 Chris Dumez 2019-02-18 13:39:01 PST
Likely a PSON regression, I'll investigate as soon as I have spare cycles.
Comment 3 Chris Dumez 2019-02-19 09:17:37 PST
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.
Comment 4 Chris Dumez 2019-03-18 16:23:23 PDT
Created attachment 365086 [details]
Patch
Comment 5 Geoffrey Garen 2019-03-19 09:46:01 PDT
Comment on attachment 365086 [details]
Patch

r=me
Comment 6 Chris Dumez 2019-03-19 09:48:45 PDT
Comment on attachment 365086 [details]
Patch

Clearing flags on attachment: 365086

Committed r243142: <https://trac.webkit.org/changeset/243142>
Comment 7 Chris Dumez 2019-03-19 09:48:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 8 Chris Dumez 2019-03-19 11:39:06 PDT
Reverted r243142 for reason:

Caused assertion hits in WK2 Debug

Committed r243154: <https://trac.webkit.org/changeset/243154>
Comment 9 Chris Dumez 2019-03-19 12:04:56 PDT
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
Comment 10 Chris Dumez 2019-03-19 12:05:31 PDT
Created attachment 365201 [details]
Patch
Comment 11 Chris Dumez 2019-03-19 12:19:28 PDT
Committed r243159: <https://trac.webkit.org/changeset/243159>
Comment 12 Truitt Savell 2019-03-19 16:55:14 PDT
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.
Comment 13 Chris Dumez 2019-03-19 16:57:56 PDT
(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.
Comment 14 Chris Dumez 2019-03-20 08:43:50 PDT
(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.
Comment 15 Chris Dumez 2019-03-20 09:00:04 PDT
(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>.