Bug 158851 - [WK2] Improve serialization of SubresourcesEntry to network disk cache
Summary: [WK2] Improve serialization of SubresourcesEntry to network disk cache
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-06-16 15:10 PDT by Chris Dumez
Modified: 2016-06-16 19:29 PDT (History)
4 users (show)

See Also:


Attachments
Patch (8.25 KB, patch)
2016-06-16 15:19 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (13.53 KB, patch)
2016-06-16 16:09 PDT, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2016-06-16 15:10:42 PDT
Improve serialization of SubresourcesEntry to network disk cache.
Comment 1 Chris Dumez 2016-06-16 15:19:29 PDT
Created attachment 281479 [details]
Patch
Comment 2 Antti Koivisto 2016-06-16 15:46:58 PDT
Comment on attachment 281479 [details]
Patch

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

> Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h:90
> -    static const unsigned version = 7;
> +    static const unsigned version = 8;

Since you are increasing the version can you also make the directory names more consistent/informative? Currently we have foo.bar/resource/ and foo.bar/subresources/ which sound confusingly similar. Also they could be capitalized like Blobs/Records. Resources/SpeculativeDependencies or something.
Comment 3 Chris Dumez 2016-06-16 15:51:22 PDT
(In reply to comment #2)
> Comment on attachment 281479 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=281479&action=review
> 
> > Source/WebKit2/NetworkProcess/cache/NetworkCacheStorage.h:90
> > -    static const unsigned version = 7;
> > +    static const unsigned version = 8;
> 
> Since you are increasing the version can you also make the directory names
> more consistent/informative? Currently we have foo.bar/resource/ and
> foo.bar/subresources/ which sound confusingly similar. Also they could be
> capitalized like Blobs/Records. Resources/SpeculativeDependencies or
> something.

Hmm. Right now I build the path using key.type() and the type is either "resource" / "subresources". We could easily capitalize the type when using it as folder name. However, if I rename "subresources" to something else, I would have to rename my SubresourcesEntry class as well (which I don't really want to do right now).

How about we just capitalize the type when using it as folder name in this patch?
Comment 4 Chris Dumez 2016-06-16 16:09:21 PDT
Created attachment 281499 [details]
Patch
Comment 5 WebKit Commit Bot 2016-06-16 16:39:29 PDT
Comment on attachment 281499 [details]
Patch

Clearing flags on attachment: 281499

Committed r202148: <http://trac.webkit.org/changeset/202148>
Comment 6 WebKit Commit Bot 2016-06-16 16:39:33 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Darin Adler 2016-06-16 19:21:13 PDT
Since subresource is one (non-English, jargon) word, not two word ("sub resources"), it should be spelled Subresources, not SubResources.
Comment 8 Chris Dumez 2016-06-16 19:29:34 PDT
(In reply to comment #7)
> Since subresource is one (non-English, jargon) word, not two word ("sub
> resources"), it should be spelled Subresources, not SubResources.

Darn, I thought about this. Since it is sub-resource and not subresource, I thought it made sense to capitalize as SubResource. Apparently I was wrong.