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

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Wed Aug 27 13:12:20 PDT 2008

Holger Freyther <freyther at handhelds.org> has denied Eric Seidel
<eric at webkit.org>'s request for review:
Bug 14730: [curl] Cookie handling

Attachment 19814: First part : implement CookieManager stubs & network bindings

------- 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

+	 ~CookieManager();

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

More information about the webkit-reviews mailing list