[webkit-reviews] review granted: [Bug 54886] WebResourceCacheManager needs to manage the CFURLCache : [Attachment 83171] [PATCH] Fix

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Brian Weinstein
<bweinstein at apple.com>'s request for review:
Bug 54886: WebResourceCacheManager needs to manage the CFURLCache
https://bugs.webkit.org/show_bug.cgi?id=54886

Attachment 83171: [PATCH] Fix
https://bugs.webkit.org/attachment.cgi?id=83171&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=83171&action=review

I think you should share the Mac and Windows code by putting it in a new
WebProcess/ResourceCache/cfnet/WebResourceCacheManagerCFNet.cpp and using a
macro or an inline function to make it so you can use the same function names
on both platforms. If you do that you should probably upload a new patch.

> Source/WebKit2/WebProcess/ResourceCache/WebResourceCacheManager.cpp:72
> +	   CFStringRef host = (CFStringRef)
CFArrayGetValueAtIndex(cfURLHosts.get(), i);

Please use a C++ cast.

> Source/WebKit2/WebProcess/ResourceCache/WebResourceCacheManager.cpp:73
> +	   origins.add(SecurityOrigin::create("http", host, 0));

Should we store "http" in a String so we don't do the char*->String conversion
on every iteration of the loop?

> Source/WebKit2/WebProcess/ResourceCache/WebResourceCacheManager.h:63
> +    CFArrayRef cfURLCacheHostNames() const;
> +    void clearCFURLCacheForHostNames(CFArrayRef) const;

The getter should return a RetainPtr. Maybe these should be static member
functions?