Bug 34878 - [Chromium] Provide for a frame-specific cookie jar
Summary: [Chromium] Provide for a frame-specific cookie jar
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit API (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Darin Fisher (:fishd, Google)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-11 23:59 PST by Darin Fisher (:fishd, Google)
Modified: 2010-02-12 15:30 PST (History)
1 user (show)

See Also:


Attachments
v1 patch (16.10 KB, patch)
2010-02-12 00:01 PST, Darin Fisher (:fishd, Google)
jorlow: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Darin Fisher (:fishd, Google) 2010-02-11 23:59:02 PST
[Chromium] Provide for a frame-specific cookie jar
Comment 1 Darin Fisher (:fishd, Google) 2010-02-12 00:01:40 PST
Created attachment 48627 [details]
v1 patch
Comment 2 Jeremy Orlow 2010-02-12 02:40:18 PST
Comment on attachment 48627 [details]
v1 patch


> Index: WebKit/chromium/src/ChromiumBridge.cpp
> ===================================================================
> --- WebKit/chromium/src/ChromiumBridge.cpp	(revision 54703)
> +++ WebKit/chromium/src/ChromiumBridge.cpp	(working copy)
> @@ -112,6 +113,17 @@ static WebWidgetClient* toWebWidgetClien
>      return chromeClientImpl->webView()->client();
>  }
>  
> +static WebCookieJar* getCookieJar(const Document* document)
> +{
> +    WebFrameImpl* frameImpl = WebFrameImpl::fromFrame(document->frame());
> +    if (!frameImpl || !frameImpl->client())
> +        return 0;
> +    WebCookieJar* cookieJar = frameImpl->client()->cookieJar();
> +    if (!cookieJar)
> +        cookieJar = webKitClient()->cookieJar();

This is temporary as to not break things during the transition, right?  If so, add a FIXME.  If not, are we going to leave 2 ways to do this?  I'm not completely against it, but lets not turn the WebKit API into Perl.
Comment 3 Darin Fisher (:fishd, Google) 2010-02-12 08:41:26 PST
(In reply to comment #2)
> This is temporary as to not break things during the transition, right?  If so,
> add a FIXME.  If not, are we going to leave 2 ways to do this?  I'm not
> completely against it, but lets not turn the WebKit API into Perl.

Sorry, I should have explained this better.  I intend to keep an accessor to the global CookieJar on WebKitClient.  That way, simple apps like DRT/TestShell can continue to talk to the platform layer for cookies.  This is important since our plan for porting DRT to Chromium's WebKit API involves exporting a GetWebKitClient function from the chromium code base.  It would be awkward for DRT's WebFrameClient to have to turn around and ask the platform layer for cookies.
Comment 4 Jeremy Orlow 2010-02-12 08:45:39 PST
(In reply to comment #3)
> (In reply to comment #2)
> > This is temporary as to not break things during the transition, right?  If so,
> > add a FIXME.  If not, are we going to leave 2 ways to do this?  I'm not
> > completely against it, but lets not turn the WebKit API into Perl.
> 
> Sorry, I should have explained this better.  I intend to keep an accessor to
> the global CookieJar on WebKitClient.  That way, simple apps like DRT/TestShell
> can continue to talk to the platform layer for cookies.  This is important
> since our plan for porting DRT to Chromium's WebKit API involves exporting a
> GetWebKitClient function from the chromium code base.  It would be awkward for
> DRT's WebFrameClient to have to turn around and ask the platform layer for
> cookies.

In such a case, wouldn't the cookie jar just be a singleton?

I'm not sure I like the idea of adding "there's more than one way to do it" to our WebKit API unless absolutely necessary.  It's also bad if our tests are hitting one code path and production is using another.
Comment 5 Darin Fisher (:fishd, Google) 2010-02-12 10:38:41 PST
> In such a case, wouldn't the cookie jar just be a singleton?

WebKitClient is where we place singletons.  The idea here is that the embedder may wish to have a different cookie jar for a particular frame.  This is all to avoid having to pass a WebFrame parameter to the WebCookieJar methods.  I could do that, but that seems like a layering violation.  This was my solution to avoid that layering violation.


> I'm not sure I like the idea of adding "there's more than one way to do it" to
> our WebKit API unless absolutely necessary.  It's also bad if our tests are
> hitting one code path and production is using another.

It is not that different of a code path, and the differences would still exist if we parameterize WebCookieJar methods with a WebFrame.  The getCookieJar method helps minimize the difference.
Comment 6 Jeremy Orlow 2010-02-12 10:46:52 PST
Fair enough.
Comment 7 Darin Fisher (:fishd, Google) 2010-02-12 15:30:17 PST
Landed as http://trac.webkit.org/changeset/54742