[webkit-reviews] review granted: [Bug 181822] Resource Load Statistics: Implement callback support for removal of WebsiteDataType::ResourceLoadStatistics : [Attachment 331683] Patch

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has granted John Wilander
<wilander at apple.com>'s request for review:
Bug 181822: Resource Load Statistics: Implement callback support for removal of
WebsiteDataType::ResourceLoadStatistics
https://bugs.webkit.org/show_bug.cgi?id=181822

Attachment 331683: Patch

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




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

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

> Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp:308
> +void
WebResourceLoadStatisticsStore::grandfatherExistingWebsiteData(WTF::CompletionH
andler<void ()>&& callback)

WTF should be unnecessary.
Space before () is against style guidelines.
Everywhere in this patch.

> Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:563
> +    auto completionHandlerCopy = makeBlockPtr(completionHandler);

This could just be done inside the lambda capture.  Then you won't need a local
variable with "Copy" in its name.  Otherwise this should be WTFMoved into the
lambda.

> Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStore.mm:582
> +    auto completionHandlerCopy = makeBlockPtr(completionHandler);

ditto

> Source/WebKit/UIProcess/API/Cocoa/WKWebsiteDataStorePrivate.h:82
>  - (void)_resourceLoadStatisticsClearInMemoryAndPersistentStore
WK_API_AVAILABLE(macosx(10.13), ios(11.0));

These could use a WK_API_DEPRECATED_WITH_REPLACEMENT