Bug 174174 – [WK2] WebsiteDataStore::setResourceLoadStatisticsEnabled() updates processPools the store is not associated with

• Component: WebKit2
• Status: RESOLVED FIXED

[Chris Dumez]

WebsiteDataStore::setResourceLoadStatisticsEnabled() updates processPools the store is not associated with.

[Chris Dumez]

Created attachment 314655 [details]
Patch

[Brent Fulgham]

Comment on attachment 314655 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314655&action=review

> Source/WebKit2/ChangeLog:15
> +        as well as corresponding methods as those appear to be unused.

Yikes! Good catch.

[Brent Fulgham]

Comment on attachment 314655 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=314655&action=review

Looks good. r=me.

> Source/WebKit2/UIProcess/WebProcessPool.cpp:1117
> +    sendToAllProcesses(Messages::WebProcess::SetResourceLoadStatisticsEnabled(enabled));

This is much tidier!

> Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:1271
> +    for (auto& processPool : processPools())

Idea: processPools() should have a method that takes a lambda and applies it to each process in the pool.

> Source/WebKit2/UIProcess/WebsiteData/WebsiteDataStore.cpp:1281
> +    m_resourceLoadStatistics->registerSharedResourceLoadObserver([this] (const Vector<String>& domainsToRemove, const Vector<String>& domainsToAdd, bool shouldClearFirst) {

I noticed that 'this' is refcounted. Do we need to capture it in this lambda?

[WebKit Commit Bot]

Comment on attachment 314655 [details]
Patch

Clearing flags on attachment: 314655

Committed r219161: <http://trac.webkit.org/changeset/219161>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.