[webkit-reviews] review granted: [Bug 193297] Activate the WebResourceLoadStatisticsStore in the NetworkProcess and deactivate it in the UIProcess. : [Attachment 360025] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 193297: Activate the WebResourceLoadStatisticsStore in the NetworkProcess
and deactivate it in the UIProcess.
https://bugs.webkit.org/show_bug.cgi?id=193297

Attachment 360025: Patch

https://bugs.webkit.org/attachment.cgi?id=360025&action=review




--- Comment #8 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 360025
  --> https://bugs.webkit.org/attachment.cgi?id=360025
Patch

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

> Source/WebCore/loader/ResourceLoadObserver.h:77
> +    WEBCORE_EXPORT void
setLogWebSocketLoadingNotificationCallback(WTF::Function<void(PAL::SessionID,
const String&, const String&, WallTime)>&&);

WTF:: is not needed.

>
Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:506
> +    postTask([this, targetPrimaryDomain =
targetPrimaryDomain.isolatedCopy(), mainFramePrimaryDomain =
mainFramePrimaryDomain.isolatedCopy(), lastSeen, completionHandler =
WTFMove(completionHandler)]() mutable {

Should we have a protectedThis or a weak pointer?

> Source/WebKit/WebProcess/WebProcess.cpp:400
> +   
ResourceLoadObserver::shared().setLogSubresourceRedirectNotificationCallback([t
his] (PAL::SessionID sessionID, const String& sourcePrimaryDomain, const
String& targetPrimaryDomain) {

'this' should probably be a weak pointer.  I can imagine some using after
freeing from loads updating after a page has been torn down.