[webkit-reviews] review requested: [Bug 101621] CookieJar uses Document class, which is a layering violation : [Attachment 173113] patch for review

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Nov 8 13:51:14 PST 2012


Alexey Proskuryakov <ap at webkit.org> has asked  for review:
Bug 101621: CookieJar uses Document class, which is a layering violation
https://bugs.webkit.org/show_bug.cgi?id=101621

Attachment 173113: patch for review
https://bugs.webkit.org/attachment.cgi?id=173113&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
Chromium bot is slow today. Fixed errors that EWS reported for other ports.
There will probably be more errors, but this is ready for review.

> One of the design pressures in Mark's patch is that Chromium might want to
use different cookie jars in different frames.	That's the reason for the
introduction of the CookieJar class rather than using free functions.

This design goal makes sense, and I think that my patch doesn't make it harder.
The WebCore level interface to CookieJar is unchanged - I would certainly
prefer if chromium turned to a platform level abstraction like
NetworkingContext, but you can keep using a Frame if such a change is
undesirable.

Unfortunately, converting CookieJar into a class won't work for Mac (and Mark's
patch leaves Mac in a confusing state). We don't have a separate independent
cookie storage that could be encapsulated in a class - in most cases, cookie
storage is part of a "storage session". This is why getting cookie storage from
a networking context is the design I prefer.

I'm extremely sad that we've got so much wasted effort on Mark's part.


More information about the webkit-reviews mailing list