[webkit-reviews] review granted: [Bug 198635] [WKHTTPCookieStore getAllCookies:] may return duplicate cookies : [Attachment 371643] Patch

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Sihui Liu <sihui_liu at apple.com>'s
request for review:
Bug 198635: [WKHTTPCookieStore getAllCookies:] may return duplicate cookies
https://bugs.webkit.org/show_bug.cgi?id=198635

Attachment 371643: Patch

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




--- Comment #13 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 371643
  --> https://bugs.webkit.org/attachment.cgi?id=371643
Patch

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

> Source/WebKit/ChangeLog:15
> +

You should explain that the other case where we need to do a different
comparison check.

> Source/WebCore/platform/Cookie.h:181
> +	   static const bool emptyValueIsZero = false;
> +	   static const bool hasIsEmptyValueFunction = true;

Do we really need to define both? I thought defining hasIsEmptyValueFunction
was enough.

> Source/WebKit/UIProcess/WebsiteData/WebsiteDataStore.cpp:1944
> +    for (auto& pendingCookie : pendingCookies()) {

Just use m_pendingCookies.removeIf instead of creating a temporary Vector?