Bug 55986 - Use the Cookie Storage from the Private Browsing Storage Session directly
Summary: Use the Cookie Storage from the Private Browsing Storage Session directly
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Jessie Berlin
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2011-03-08 17:20 PST by Jessie Berlin
Modified: 2011-03-09 08:43 PST (History)
2 users (show)

See Also:

Attachments
Patch (deleted)
2011-03-08 18:02 PST, Jessie Berlin 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Jessie Berlin 2011-03-08 17:20:29 PST 
There is no need to create an in-memory copy because the Private Browsing Storage Session is already in-memory only.

<rdar://problem/9102381>
Comment 1 Jessie Berlin 2011-03-08 18:02:49 PST 
Created attachment 85119 [details]
Patch
Comment 2 Adam Roben (:aroben) 2011-03-09 05:15:52 PST 
Comment on attachment 85119 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=85119&action=review

Why doesn't Mac need the CreateInMemory fallback path?

> Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:81
> +        privateBrowsingCookieStorage() = wkCreateInMemoryHTTPCookieStorage();

You're leaking this CFHTTPCookieStorageRef.
Comment 3 Jessie Berlin 2011-03-09 05:58:47 PST 
(In reply to comment #2)
> (From update of attachment 85119 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=85119&action=review
> 
> Why doesn't Mac need the CreateInMemory fallback path?

Because if it isn't using the CFURLSTORAGESESSIONS, it calls wkSetCookieStoragePrivateBrowsingEnabled(enabled), which makes all cookies be stored in memory.

> 
> > Source/WebCore/platform/network/cf/CookieStorageCFNet.cpp:81
> > +        privateBrowsingCookieStorage() = wkCreateInMemoryHTTPCookieStorage();
> 
> You're leaking this CFHTTPCookieStorageRef.

Hrm, for some reason I thought the result of a Create wasn't already retained. Fixed.
Comment 4 Adam Roben (:aroben) 2011-03-09 06:44:28 PST 
(In reply to comment #3)
> (In reply to comment #2)
> > (From update of attachment 85119 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=85119&action=review
> > 
> > Why doesn't Mac need the CreateInMemory fallback path?
> 
> Because if it isn't using the CFURLSTORAGESESSIONS, it calls wkSetCookieStoragePrivateBrowsingEnabled(enabled), which makes all cookies be stored in memory.

I guess I don't understand what the difference between Mac and Windows is here.
Comment 5 Jessie Berlin 2011-03-09 07:12:06 PST 
(In reply to comment #4)
> (In reply to comment #3)
> > (In reply to comment #2)
> > > (From update of attachment 85119 [details] [details] [details])
> > > View in context: https://bugs.webkit.org/attachment.cgi?id=85119&action=review
> > > 
> > > Why doesn't Mac need the CreateInMemory fallback path?
> > 
> > Because if it isn't using the CFURLSTORAGESESSIONS, it calls wkSetCookieStoragePrivateBrowsingEnabled(enabled), which makes all cookies be stored in memory.
> 
> I guess I don't understand what the difference between Mac and Windows is here.

wkSetCookieStoragePrivateBrowsingEnabled (and the functionality behind it) doesn't exist on Windows. wkSetCookieStoragePrivateBrowsingEnabled was the way it was done on Mac before my changes to add the CFURLSTORAGESESSIONS stuff.
Comment 6 Jessie Berlin 2011-03-09 08:42:58 PST 
Comment on attachment 85119 [details]
Patch

Committed in r80643
http://trac.webkit.org/changeset/80643