[webkit-reviews] review denied: [Bug 14730] [curl] Cookie handling : [Attachment 19814] First part : implement CookieManager stubs & network bindings

[bugzilla-daemon at webkit.org]

Holger Freyther <freyther at handhelds.org> has denied Eric Seidel
<eric at webkit.org>'s request for review:
Bug 14730: [curl] Cookie handling
https://bugs.webkit.org/show_bug.cgi?id=14730

Attachment 19814: First part : implement CookieManager stubs & network bindings
https://bugs.webkit.org/attachment.cgi?id=19814&action=edit

------- Additional Comments from Holger Freyther <freyther at handhelds.org>
Sorry I think we need another iteration.


-    return cookieJar.get(url.string());
+    if (cookiesEnabled(document))
+	 return CookieManager::getCookieManager()->getCookies(url);
+    else
+	 return String();

Could we have an early return here?

-    return true;
+    CookieManager* manager = CookieManager::getCookieManager();
+    if (manager)
+	 return true;
+    else
+	 return false;

return manager; :)



+    // Cookie size should not be above 81920 (per construction and also
because we
+    // do not want to	cookie).
+    ASSERT(cookiePairs.length() <= 81920);

make that a runtime option? Why should big cookies make webkit assert?


+    strncpy(cookieChar, cookiePairs.ascii().data(),

what ascii is that? Is that available when build in release mode? I think it is
not.


+	 ~CookieManager();

can we make that private. With the current d'tor implementation I would feel
more happy with that.