[webkit-reviews] review denied: [Bug 193659] Switch NetworkStorageSession portions of ResourceLoadStatistics to Async message passing style : [Attachment 359844] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has denied Brent Fulgham
<bfulgham at webkit.org>'s request for review:
Bug 193659: Switch NetworkStorageSession portions of ResourceLoadStatistics to
Async message passing style
https://bugs.webkit.org/show_bug.cgi?id=193659

Attachment 359844: Patch

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




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

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

> Source/WebKit/NetworkProcess/Classifier/ShouldGrandfatherStatistics.h:40
> +template<> struct EnumTraits<WebKit::ShouldGrandfatherStatistics> {

Still not needed.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:604
> +	      
resourceLoadStatistics->dumpResourceLoadStatistics(WTFMove(completionHandler));

It seems like we should call the completion handler in the other cases, too.

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:723
>	   ASSERT_NOT_REACHED();

ditto

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:869
>	   ASSERT_NOT_REACHED();

ditto

> Source/WebKit/NetworkProcess/NetworkProcess.cpp:887
>	   ASSERT_NOT_REACHED();

ditto

> Source/WebKit/UIProcess/API/C/WKWebsiteDataStoreRef.cpp:409
> +#if ENABLE(RESOURCE_LOAD_STATISTICS)

Should we just call the callback with the context if the feature is not
enabled?