Bug 114538 - Query directly for cache partition names
Summary: Query directly for cache partition names
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Vicki Pfau
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-04-12 15:12 PDT by Vicki Pfau
Modified: 2013-04-14 15:47 PDT (History)
2 users (show)

See Also:


Attachments
Patch (116.37 KB, patch)
2013-04-12 15:56 PDT, Vicki Pfau
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vicki Pfau 2013-04-12 15:12:05 PDT
The heuristic used in WebKit currently for detecting cache partitions fails in a sizable number of cases. We should call back into the system to ask it for the names of the partitions instead.
Comment 1 Vicki Pfau 2013-04-12 15:56:11 PDT
Created attachment 197898 [details]
Patch
Comment 2 David Kilzer (:ddkilzer) 2013-04-12 17:23:01 PDT
Comment on attachment 197898 [details]
Patch

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

r=me  (Wow, that's a lot of indirection to get the list of origins.)

> Source/WebKit2/WebProcess/ResourceCache/WebResourceCacheManager.cpp:59
> +#if USE(CFURLCACHE)
> +    __block
> +#endif
>      MemoryCache::SecurityOriginSet origins;

I think it would be clearer to write it this way:

#if USE(CFURLCACHE)
    __block MemoryCache::SecurityOriginSet origins;
#else
    MemoryCache::SecurityOriginSet origins;
#endif

Also, should the #if check be this since it's the only time that 'origins' is used in a block?

#if USE(CFURLCACHE) && ENABLE(CACHE_PARTITIONING)
    __block MemoryCache::SecurityOriginSet origins;
#else
    MemoryCache::SecurityOriginSet origins;
#endif
Comment 3 Vicki Pfau 2013-04-12 17:25:42 PDT
(In reply to comment #2)
> (From update of attachment 197898 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=197898&action=review
> 
> r=me  (Wow, that's a lot of indirection to get the list of origins.)
> 
> > Source/WebKit2/WebProcess/ResourceCache/WebResourceCacheManager.cpp:59
> > +#if USE(CFURLCACHE)
> > +    __block
> > +#endif
> >      MemoryCache::SecurityOriginSet origins;
> 
> I think it would be clearer to write it this way:
> 
> #if USE(CFURLCACHE)
>     __block MemoryCache::SecurityOriginSet origins;
> #else
>     MemoryCache::SecurityOriginSet origins;
> #endif

I suppose it probably would be better style this way, yes.

> Also, should the #if check be this since it's the only time that 'origins' is used in a block?
> 
> #if USE(CFURLCACHE) && ENABLE(CACHE_PARTITIONING)
>     __block MemoryCache::SecurityOriginSet origins;
> #else
>     MemoryCache::SecurityOriginSet origins;
> #endif

Hm, yeah, I must have missed that, but you're right.
Comment 4 Vicki Pfau 2013-04-12 18:08:44 PDT
Committed r148329: <http://trac.webkit.org/changeset/148329>
Comment 5 Vicki Pfau 2013-04-12 18:09:38 PDT
<rdar://problem/13395445>
Comment 6 Sam Weinig 2013-04-14 15:47:55 PDT
This should have been signed off by a WebKit2 owner.  Please try to remember that in the future.