Bug 176135

Summary: Update CacheStorage caches only if it is updated
Product: WebKit Reporter: youenn fablet <youennf>
Component: WebCore Misc.Assignee: youenn fablet <youennf>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, buildbot, cdumez, cgarcia, commit-queue, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

youenn fablet
Reported 2017-08-30 16:47:41 PDT
This will improve efficiency
Attachments
Patch (26.22 KB, patch)
2017-08-30 16:51 PDT, youenn fablet
no flags
Patch for landing (27.39 KB, patch)
2017-08-31 15:50 PDT, youenn fablet
no flags
youenn fablet
Comment 1 2017-08-30 16:51:50 PDT
Alex Christensen
Comment 2 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.
youenn fablet
Comment 3 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!
Alex Christensen
Comment 4 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.
youenn fablet
Comment 5 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.
youenn fablet
Comment 6 2017-08-31 15:50:14 PDT
Created attachment 319534 [details] Patch for landing
WebKit Commit Bot
Comment 7 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>
WebKit Commit Bot
Comment 8 2017-08-31 16:20:08 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 9 2017-09-27 12:48:12 PDT
Note You need to log in before you can comment on or make changes to this bug.