Bug 34878 – [Chromium] Provide for a frame-specific cookie jar

• Component: WebKit API
• Status: RESOLVED FIXED

[Darin Fisher (:fishd, Google)]

[Chromium] Provide for a frame-specific cookie jar

[Darin Fisher (:fishd, Google)]

Created attachment 48627 [details]
v1 patch

[Jeremy Orlow]

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.

[Darin Fisher (:fishd, Google)]

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

[Jeremy Orlow]

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

[Darin Fisher (:fishd, Google)]

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

[Jeremy Orlow]

Fair enough.

[Darin Fisher (:fishd, Google)]

Landed as http://trac.webkit.org/changeset/54742