[Webkit-unassigned] [Bug 30322] WebKit level persistent caching

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=30322

--- Comment #52 from Antti Koivisto <koivisto at iki.fi> ---
(In reply to comment #50)
> Is there just a single network cache? It seems like you would want it to be
> per session?

The initial patch has a singleton cache but there are very few singleton assumptions. It won't be difficult to transition to many-caches model later as needed. 

> > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:172
> > +            return nullptr;
> 
> Should the entry be removed from the cache in this case?

It is. We return decoding failure to the storage layer (from retrieve lambda) and the entry gets deleted.

> Why does this have to cast to void*?

"Cannot initialize a parameter of type 'void *' with an rvalue of type 'const uint8_t *' (aka 'const unsigned char *')". 

I'll switch to C++ cast at least.

> > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:212
> > +    if (partition.isEmpty())
> > +        partition = ASCIILiteral("No partition");
> 
> Can't we let an empty partition mean no partition? Seems stupid to write out
> "No partition" to disk (if that's what we're doing).

Currently cache entries are organised directory-per-partition on disk. It looks somewhat cleaner when non-partitioned entries also go to a directory of their own (called "No partition" for now, space guarantees no clashes with partitions).

> > Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:259
> > +    if (originalRequest.httpMethod() != "GET") {
> 
> I think you want equalIgnoringCase here?

I believe the method is always uppercase and all the code using equalIgnoringCase is just cargo-culting. But I may be wrong!

> I think you want to store some type of hash version here as well so we can
> bump the version if we decide to change the WTF hash (which is likely to
> happen in the future).

The general idea has been that we don't mind too much if cache entries become invalid due to format changes, just that we can detect now-invalid entries and ignore them. They'll get freshened quickly in normal browser use.

> What happens if there are symlinks inside the directory that links to the
> parent here?

I think nothing. We ignore DT_LNK entries when traversing. If someone adds symlinks to cache directories we won't empty them so can't delete them either.

> File I/O on the main thread!

Oh no!

> What happens if you try to update the cache while it's being cleared?

New entries may be created during clearing and depending where they are they may get immediately deleted. I'm not sure this is a practical problem.

Thanks for reviews, I'll update the patch.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141119/158bf94a/attachment-0002.html>