[webkit-reviews] review granted: [Bug 209369] Add the capability to change all of a website's cookies to SameSite=Strict : [Attachment 394287] 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 209369: Add the capability to change all of a website's cookies to
SameSite=Strict
https://bugs.webkit.org/show_bug.cgi?id=209369

Attachment 394287: Patch

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




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

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

> Source/WTF/wtf/PlatformHave.h:447
> +#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101500) ||
(PLATFORM(IOS_FAMILY) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 130000)

Unfortunately, many existing checks in this file are incorrect.  You have to
explicitly state that this is also available on watchOS and tvOS.

> Source/WebCore/platform/network/NetworkStorageSession.cpp:95
> +    UNUSED_PARAM(domain);

Just leave out the parameter name.
(const RegistrableDomain&,

> Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:124
> +    RetainPtr<NSMutableArray> oldCookiesToDelete = adoptNS([[NSMutableArray
alloc] init]);

These can be RetainPtr<NSMutableArray<NSHTTPCookie>>

> Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:130
> +	       NSMutableDictionary<NSHTTPCookiePropertyKey, id>
*mutableProperties = [[nsCookie properties] mutableCopy];

RetainPtr<NSMutableDictionary<NSHTTPCookiePropertyKey, id>> mutableProperties =
adoptNS([[nsCookie properties] mutableCopy]);
Then we don't need to manually release below.

> Source/WebCore/platform/network/cocoa/NetworkStorageSessionCocoa.mm:139
> +    if (!newCookiesToAdd.get().count)
> +	   return completionHandler();

I don't think this early return actually does anything.