We should adjust WebKit API calls to only be able to set and access app-bound domain cookies.
<rdar://problem/61071428>
Created attachment 395312 [details] Patch
Created attachment 395318 [details] Patch
Comment on attachment 395318 [details] 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.
(In reply to Brady Eidson from comment #4) > Comment on attachment 395318 [details] > 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. I did not note this at the time - THere's an inner async lambda in here, necessitating passing in the outer lambda. NM! > > > 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.
Comment on attachment 395318 [details] Patch Change the names as we discussed then this is good
Created attachment 395415 [details] Patch for landing
(In reply to Brady Eidson from comment #6) > Comment on attachment 395318 [details] > Patch > > Change the names as we discussed then this is good Thank you!
Committed r259520: <https://trac.webkit.org/changeset/259520> All reviewed patches have been landed. Closing bug and clearing flags on attachment 395415 [details].