Bug 234892

Summary: Make WebResourceLoadStatisticsStore a MessageReceiver
Product: WebKit Reporter: Alex Christensen <achristensen>
Component: New BugsAssignee: Alex Christensen <achristensen>
Status: NEW ---    
Severity: Normal CC: annulen, darin, ews-watchlist, gyuyoung.kim, hi, mkwst, ryuan.choi, sergio, webkit-bug-importer, wilander
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
ews-feeder: commit-queue-
Patch
ews-feeder: commit-queue-
Patch
none
Patch
none
Patch ews-feeder: commit-queue-

Description Alex Christensen 2022-01-05 11:29:23 PST
Make WebResourceLoadStatisticsStore a MessageReceiver
Comment 1 Alex Christensen 2022-01-05 11:31:26 PST
Created attachment 448413 [details]
Patch
Comment 2 Alex Christensen 2022-01-05 11:47:40 PST
Created attachment 448414 [details]
Patch
Comment 3 Alex Christensen 2022-01-05 12:05:44 PST
Created attachment 448415 [details]
Patch
Comment 4 John Wilander 2022-01-05 16:02:58 PST
Comment on attachment 448415 [details]
Patch

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

r=me bar addressing the test failures.

> Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:-85
> -void WebResourceLoadStatisticsStore::setNotifyPagesWhenDataRecordsWereScanned(bool value)

Was this intentionally deleted? Doesn't seem to be part of the gist of this patch. A comment on why it can be removed would be good.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:539
>  void NetworkProcess::updatePrevalentDomainsToBlockCookiesFor(PAL::SessionID sessionID, const Vector<RegistrableDomain>& domainsToBlock, CompletionHandler<void()>&& completionHandler)

Was this particular one not moved?

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:684
> +void NetworkProcess::setShouldEnableSameSiteStrictEnforcementForTesting(PAL::SessionID sessionID, WebCore::SameSiteStrictEnforcementEnabled enabled, CompletionHandler<void()>&& completionHandler)

Nice catch.

> Source/WebKit/UIProcess/WebPageProxy.cpp:5572
> +    websiteDataStore().networkProcess().send(Messages::WebResourceLoadStatisticsStore::LogFrameNavigation(RegistrableDomain { targetURL }, RegistrableDomain { pageURL }, RegistrableDomain { sourceURL }, isRedirect, frame.isMainFrame(), MonotonicTime::now() - m_didFinishDocumentLoadForMainFrameTimestamp, wasPotentiallyInitiatedByUser), sessionID().toUInt64());

Would be nice to be able to send session ID objects directly.
Comment 5 Alex Christensen 2022-01-05 17:17:59 PST
(In reply to John Wilander from comment #4)
> Comment on attachment 448415 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=448415&action=review
> 
> r=me bar addressing the test failures.
> 
> > Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:-85
> > -void WebResourceLoadStatisticsStore::setNotifyPagesWhenDataRecordsWereScanned(bool value)
> 
> Was this intentionally deleted? Doesn't seem to be part of the gist of this
> patch. A comment on why it can be removed would be good.
It was dead code.  Will note in change log.

> 
> > Source/WebKit/NetworkProcess/NetworkProcess.cpp:539
> >  void NetworkProcess::updatePrevalentDomainsToBlockCookiesFor(PAL::SessionID sessionID, const Vector<RegistrableDomain>& domainsToBlock, CompletionHandler<void()>&& completionHandler)
> 
> Was this particular one not moved?
I only moved the ones that were straightforwardly doing nothing but finding the WebResourceLoadStatisticsStore and forwarding parameters to it.  This one goes to the NetworkStorageSession, so it'll be moved in a different patch.
Comment 6 Alex Christensen 2022-01-05 17:35:17 PST
Created attachment 448457 [details]
Patch
Comment 7 Alex Christensen 2022-01-05 17:36:47 PST
Created attachment 448458 [details]
Patch
Comment 8 Radar WebKit Bug Importer 2022-01-12 11:30:22 PST
<rdar://problem/87471616>
Comment 9 Darin Adler 2022-01-16 12:39:33 PST
Comment on attachment 448458 [details]
Patch

TestWebKitAPI.ResourceLoadStatistics.DataSummaryWithCachedProcess test is failing