Bug 54886 – WebResourceCacheManager needs to manage the CFURLCache

• Component: WebKit2
• Status: RESOLVED FIXED

[Brian Weinstein]

WebResourceCacheManager needs to manage the CFURLCache

It currently only supposed the WebCore memory cache. It should also support the CFURLCache on platforms that use CFNetwork.

[Brian Weinstein]

Created attachment 83171 [details]
[PATCH] Fix

[Adam Roben (:aroben)]

Comment on attachment 83171 [details]
[PATCH] Fix

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?

[Build Bot]

Attachment 83171 [details] did not build on win:
Build output: http://queues.webkit.org/results/7942034

[Brian Weinstein]

Created attachment 83190 [details]
[PATCH] Take 2 - Old Mac Project File

The Mac project file hasn't been updated yet with the refactoring to WebResourceCacheManagerCFNet yet, but will be before landing.

[Adam Roben (:aroben)]

Comment on attachment 83190 [details]
[PATCH] Take 2 - Old Mac Project File

Brian said he's going to update WebKit2.xcodeproj soon, once he can use his Mac again. And that he'll update WKSI before checking in.

[Build Bot]

Attachment 83190 [details] did not build on win:
Build output: http://queues.webkit.org/results/7938892

[Brian Weinstein]

Created attachment 83205 [details]
[PATCH] Take 3 - Mac Project File + Feature Define

[Build Bot]

Attachment 83205 [details] did not build on win:
Build output: http://queues.webkit.org/results/7939861

[Brian Weinstein]

Landed in r79252.

[Alexey Proskuryakov]

WebKitSystemInterface.h capitalizes names differently for the very reason of preventing directly using them. Please have a look at how all other WK exports work - you need to add them WebCore/WebCoreSystemInterface, and then to both WebKit1 and WebKit2 WebSystemInterface.

Please fix it soon.

[Alexey Proskuryakov]

Brian explained that this WKSI use is not in WebCore, so what I said made no sense.

[Alexey Proskuryakov]

It's still strange that this is the only WKSI function that we ever used in cross-platform code in WebKit. Could this mean that this code should really be moved to WebCore? WebKit should ideally be nothing but a thin wrapper around WebCore.