Bug 187206 – Resource Load Statistics: Make network process calls only for the process pool that the page belongs to

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[John Wilander]

In the WebKit::WebsiteDataStore infrastructure for ITP and the Storage Access API, instead of iterating over all process pools, we should resolve which process pool the page belongs to and call the network process only for that pool.

[Radar WebKit Bug Importer]

<rdar://problem/41659160>

[John Wilander]

Created attachment 343975 [details]
Patch

[Chris Dumez]

Comment on attachment 343975 [details]
Patch

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

r=me with nits.

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1221
> +    if (auto webPage = WebProcessProxy::webPage(pageID)) {

auto*

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1224
> +    } else

I think I'd rather we do an early return:
auto* webPage = WebProcessProxy::webPage(pageID);
if (!webPage) {
    completionHandler(false);
    return;
}
auto& networkProcess = webPage->process().processPool().ensureNetworkProcess();
networkProcess.hasStorageAccessForFrame(m_sessionID, resourceDomain, firstPartyDomain, frameID, pageID, WTFMove(completionHandler));

Same comments apply below.

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1231
> +        processPool->networkProcess()->getAllStorageAccessEntries(m_sessionID, WTFMove(completionHandler));

This is still wrong but I guess we can fix separately since you do not have a pageID here.

[John Wilander]

(In reply to Chris Dumez from comment #3)
> Comment on attachment 343975 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=343975&action=review
> 
> r=me with nits.
> 
> > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1221
> > +    if (auto webPage = WebProcessProxy::webPage(pageID)) {
> 
> auto*

Will fix.

> > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1224
> > +    } else
> 
> I think I'd rather we do an early return:
> auto* webPage = WebProcessProxy::webPage(pageID);
> if (!webPage) {
>     completionHandler(false);
>     return;
> }
> auto& networkProcess =
> webPage->process().processPool().ensureNetworkProcess();
> networkProcess.hasStorageAccessForFrame(m_sessionID, resourceDomain,
> firstPartyDomain, frameID, pageID, WTFMove(completionHandler));

Will fix.

> Same comments apply below.

Will fix.

> > Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1231
> > +        processPool->networkProcess()->getAllStorageAccessEntries(m_sessionID, WTFMove(completionHandler));
> 
> This is still wrong but I guess we can fix separately since you do not have
> a pageID here.

I'll add a FIXME comment.

Thanks, Chris!

[John Wilander]

Created attachment 343982 [details]
Patch for landing

[WebKit Commit Bot]

Comment on attachment 343982 [details]
Patch for landing

Clearing flags on attachment: 343982

Committed r233384: <https://trac.webkit.org/changeset/233384>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.