Bug 176135 - Update CacheStorage caches only if it is updated
Summary: Update CacheStorage caches only if it is updated
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: youenn fablet
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-08-30 16:47 PDT by youenn fablet
Modified: 2017-09-27 12:48 PDT (History)
6 users (show)

See Also:


Attachments
Patch (26.22 KB, patch)
2017-08-30 16:51 PDT, youenn fablet
no flags Details | Formatted Diff | Diff
Patch for landing (27.39 KB, patch)
2017-08-31 15:50 PDT, youenn fablet
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description youenn fablet 2017-08-30 16:47:41 PDT
This will improve efficiency
Comment 1 youenn fablet 2017-08-30 16:51:50 PDT
Created attachment 319422 [details]
Patch
Comment 2 Alex Christensen 2017-08-31 11:03:16 PDT
Comment on attachment 319422 [details]
Patch

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

> Source/WebKit/ChangeLog:9
> +        When sending the list of caches, CacheStorageEngineCaches will compare its counter with the one provided.

Will this work if multiple processes are using the same cache?

> Source/WebCore/Modules/cache/DOMCache.h:80
> +    template<class Decoder> static bool decode(Decoder&, CacheInfos&);

You could use the cool new decoder that returns a std::optional instead of taking a reference.
Comment 3 youenn fablet 2017-08-31 11:05:40 PDT
(In reply to Alex Christensen from comment #2)
> Comment on attachment 319422 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319422&action=review
> 
> > Source/WebKit/ChangeLog:9
> > +        When sending the list of caches, CacheStorageEngineCaches will compare its counter with the one provided.
> 
> Will this work if multiple processes are using the same cache?

That should work.
Each counter given to CacheStorageEngineCaches comes from a corresponding Document.
We should be able to test that kind of behavior with a single process containing several iframes.

> > Source/WebCore/Modules/cache/DOMCache.h:80
> > +    template<class Decoder> static bool decode(Decoder&, CacheInfos&);
> 
> You could use the cool new decoder that returns a std::optional instead of
> taking a reference.

Ahah, will look at it!
Comment 4 Alex Christensen 2017-08-31 14:48:29 PDT
Comment on attachment 319422 [details]
Patch

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

> Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp:236
> +    ++m_updateCounter;

We need to make sure to never forget to increment the updateCounter.  Maybe we could wrap this in a function that indicates that we are marking the cache as dirty instead of incrementing a mysterious integer.
Comment 5 youenn fablet 2017-08-31 14:54:49 PDT
(In reply to Alex Christensen from comment #4)
> Comment on attachment 319422 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=319422&action=review
> 
> > Source/WebKit/NetworkProcess/cache/CacheStorageEngineCaches.cpp:236
> > +    ++m_updateCounter;
> 
> We need to make sure to never forget to increment the updateCounter.  Maybe
> we could wrap this in a function that indicates that we are marking the
> cache as dirty instead of incrementing a mysterious integer.

Sounds good, will do this.

I will try to update the decoders as a follow-up patch.
Comment 6 youenn fablet 2017-08-31 15:50:14 PDT
Created attachment 319534 [details]
Patch for landing
Comment 7 WebKit Commit Bot 2017-08-31 16:20:07 PDT
Comment on attachment 319534 [details]
Patch for landing

Clearing flags on attachment: 319534

Committed r221454: <http://trac.webkit.org/changeset/221454>
Comment 8 WebKit Commit Bot 2017-08-31 16:20:08 PDT
All reviewed patches have been landed.  Closing bug.
Comment 9 Radar WebKit Bug Importer 2017-09-27 12:48:12 PDT
<rdar://problem/34694049>