[Webkit-unassigned] [Bug 109923] [Curl] The function cookiesForDOM() does not behave correctly.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=109923


Brent Fulgham <bfulgham at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #188545|review?                     |review-
               Flag|                            |




--- Comment #2 from Brent Fulgham <bfulgham at webkit.org>  2013-02-15 20:34:48 PST ---
(From update of attachment 188545)
View in context: https://bugs.webkit.org/attachment.cgi?id=188545&action=review

Nice work putting all of these pieces together.  A few minor formatting nits, but otherwise it looks great!

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:32
> +static void readCurlCookieToken(const char*& cookie, String &token)

This should be "String& token)"

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:37
> +        token.append(cookie[0]);

It's too bad WTF String doesn't support a character append with a length (like it does with UChar).  Then we could use strchr to find the '\t' character, then append the distance without having to handroll this loop.

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:44
> +static void addMatchingCurlCookie(const char* cookie, const String &domain, const String &path, String &cookies)

These should be written as "String& domain" and "String& path", not "String &domain", etc.

> Source/WebCore/platform/network/curl/CookieJarCurl.cpp:119
> +    cookies = cookies + strName + String("=") + strValue + String("; ");

Do you know if we are paying the cost of creating String temporaries on this line (e.g., the "=" and "; " values)? Could these be declared as static const values so we don't reconstruct them each time we pass through here?  Maybe this function isn't used enough for this to be a concern.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.