[webkit-reviews] review denied: [Bug 209926] Prevent non app-bound domain cookies from being read or set using API calls : [Attachment 395318] Patch

[bugzilla-daemon at webkit.org]

Brady Eidson <beidson at apple.com> has denied katherine_cheney at apple.com's
request for review:
Bug 209926: Prevent non app-bound domain cookies from being read or set using
API calls
https://bugs.webkit.org/show_bug.cgi?id=209926

Attachment 395318: Patch

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




--- Comment #4 from Brady Eidson <beidson at apple.com> ---
Comment on attachment 395318
  --> https://bugs.webkit.org/attachment.cgi?id=395318
Patch

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

> Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:42
> +#define HTTP_COOKIE_STORE_ADDITIONS_1 false
> +#define HTTP_COOKIE_STORE_ADDITIONS_2

Working with Kate out-of-band on a better name for these

> Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:65
> +void HTTPCookieStore::filterAppBoundCookies(const Vector<WebCore::Cookie>&
cookies, CompletionHandler<void(Vector<WebCore::Cookie>&&)>&&
completionHandler)

I'd change this to:

Vector<WebCore::Cookie> HTTPCookieStore::filterAppBoundCookies(const
Vector<WebCore::Cookie>& cookies) {...}

Have it return the filtered vector.

And have the caller call the completion handler.

> Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:96
> +	       filterAppBoundCookies(allCookies, WTFMove(completionHandler));

"Filter" is an operation completely distinct from calling a completion handler.

Make my above change, and this becomes:
completionHandler(filterAppBoundCookies(allCookies));

Reads nicer.

> Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:103
> +	   filterAppBoundCookies(cookies, WTFMove(completionHandler));

Ditto.

> Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:115
> +	   filterAppBoundCookies(cookies, WTFMove(completionHandler));

Ditto.

> Source/WebKit/UIProcess/API/APIHTTPCookieStore.cpp:121
> +    filterAppBoundCookies(cookies, [this, protectedThis = makeRef(*this),
completionHandler = WTFMove(completionHandler)] (auto&& appBoundCookies)
mutable {

Can get rid of the lambda here and just continue executing code on the
resulting vector.