Summary: | WebResourceCacheManager needs to manage the CFURLCache | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brian Weinstein <bweinstein> | ||||||||
Component: | WebKit2 | Assignee: | Brian Weinstein <bweinstein> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | ap, aroben, buildbot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | PC | ||||||||||
OS: | OS X 10.5 | ||||||||||
Attachments: |
|
Description
Brian Weinstein
2011-02-21 09:07:52 PST
Created attachment 83171 [details]
[PATCH] Fix
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? Attachment 83171 [details] did not build on win: Build output: http://queues.webkit.org/results/7942034 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.
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.
Attachment 83190 [details] did not build on win: Build output: http://queues.webkit.org/results/7938892 Created attachment 83205 [details]
[PATCH] Take 3 - Mac Project File + Feature Define
Attachment 83205 [details] did not build on win: Build output: http://queues.webkit.org/results/7939861 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. Brian explained that this WKSI use is not in WebCore, so what I said made no sense. 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. |