Bug 234892 - Make WebResourceLoadStatisticsStore a MessageReceiver
Summary: Make WebResourceLoadStatisticsStore a MessageReceiver
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-01-05 11:29 PST by Alex Christensen
Modified: 2022-01-16 12:39 PST (History)
10 users (show)

See Also:


Attachments
Patch (117.87 KB, patch)
2022-01-05 11:31 PST, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (118.60 KB, patch)
2022-01-05 11:47 PST, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (118.55 KB, patch)
2022-01-05 12:05 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (118.63 KB, patch)
2022-01-05 17:35 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (118.63 KB, patch)
2022-01-05 17:36 PST, Alex Christensen
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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