WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
176135
Update CacheStorage caches only if it is updated
https://bugs.webkit.org/show_bug.cgi?id=176135
Summary
Update CacheStorage caches only if it is updated
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
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
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
youenn fablet
Comment 1
2017-08-30 16:51:50 PDT
Created
attachment 319422
[details]
Patch
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
<
rdar://problem/34694049
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug