[webkit-reviews] review granted: [Bug 185831] [iOS] TestWebKitAPI.WebKit.WKHTTPCookieStoreWithoutProcessPool fails because cookies use different files with/without processpool : [Attachment 341206] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Sihui Liu
<sihui_liu at apple.com>'s request for review:
Bug 185831: [iOS] TestWebKitAPI.WebKit.WKHTTPCookieStoreWithoutProcessPool
fails because cookies use different files with/without processpool
https://bugs.webkit.org/show_bug.cgi?id=185831

Attachment 341206: Patch

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




--- Comment #13 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 341206
  --> https://bugs.webkit.org/attachment.cgi?id=341206
Patch

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

r=me, but consider making changes in feedback.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:380
> -    auto cookies = String(message.UTF8String);
> -    EXPECT_TRUE(cookies == "PersistentCookieName=CookieValue;
SessionCookieName=CookieValue" || cookies == "SessionCookieName=CookieValue;
PersistentCookieName=CookieValue");
> +   
EXPECT_STREQ("PersistentCookieName=CookieValue,SessionCookieName=CookieValue",
message.UTF8String);

Much better to use EXPECT_STREQ() here so we can see how the strings differ
when they fail!

Reminder: The expected value will change from using a comma to a semi-colon if
#2 is changed below.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKHTTPCookieStore.mm:401
> -    NSString *alertCookieHTML = @"<script>alert(document.cookie);</script>";
> +    NSString *alertCookieHTML = @"<script>var cookies =
document.cookie.split(';'); for (let i = 0; i < cookies.length; i ++)
{cookies[i] = cookies[i].trim();} cookies.sort(); alert(cookies);</script>";

This is okay, but two things:

1. There should be spaces around the '{' and '}' brackets in the inline
JavaScript.
2. It looks like alert(cookies) uses a comma to separate cookies; I would
prefer you use join(';') so the list still looks cookie-like.

    NSString *alertCookieHTML = @"<script>var cookies =
document.cookie.split(';'); for (let i = 0; i < cookies.length; i ++) {
cookies[i] = cookies[i].trim(); } cookies.sort();
alert(cookies.join(';'));</script>";