Bug 150856 - [WK2] Add initial support for speculative resource revalidation to the WebKit disk cache
Summary: [WK2] Add initial support for speculative resource revalidation to the WebKit...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on: 150898 150951
Blocks: 151402
  Show dependency treegraph
 
Reported: 2015-11-03 14:21 PST by Chris Dumez
Modified: 2015-11-20 14:29 PST (History)
5 users (show)

See Also:


Attachments
WIP Patch (39.83 KB, patch)
2015-11-10 09:40 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (57.32 KB, patch)
2015-11-10 11:47 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
WIP Patch (39.87 KB, patch)
2015-11-10 11:50 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (43.80 KB, patch)
2015-11-11 10:23 PST, 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 2015-11-03 14:21:59 PST
Add initial support for speculative resource revalidation to the WebKit disk cache. The functionality will be disabled by default until it is more complete / mature.
Comment 1 Chris Dumez 2015-11-03 14:22:17 PST
rdar://problem/23092196
Comment 2 Chris Dumez 2015-11-10 09:40:19 PST
Created attachment 265188 [details]
WIP Patch
Comment 3 WebKit Commit Bot 2015-11-10 09:42:06 PST
Attachment 265188 [details] did not pass style-queue:


ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:346:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:430:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.h:106:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.h:107:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Chris Dumez 2015-11-10 11:47:52 PST
Created attachment 265209 [details]
WIP Patch
Comment 5 Chris Dumez 2015-11-10 11:50:43 PST
Created attachment 265210 [details]
WIP Patch
Comment 6 WebKit Commit Bot 2015-11-10 11:53:04 PST
Attachment 265210 [details] did not pass style-queue:


ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:346:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:428:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.h:105:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.h:106:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Antti Koivisto 2015-11-10 13:29:24 PST
Comment on attachment 265210 [details]
WIP Patch

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

> Source/WebKit2/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:136
> +void SpeculativeLoad::cleanup()

Maybe call this something else than "cleanup" since it actually invokes the completion handler.
Comment 8 Chris Dumez 2015-11-11 10:23:16 PST
Created attachment 265301 [details]
Patch
Comment 9 WebKit Commit Bot 2015-11-11 10:25:07 PST
Attachment 265301 [details] did not pass style-queue:


ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:346:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.cpp:427:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.h:105:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebKit2/NetworkProcess/cache/NetworkCache.h:106:  Extra space before ( in function call  [whitespace/parens] [4]
Total errors found: 4 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 10 Antti Koivisto 2015-11-11 13:19:20 PST
Comment on attachment 265301 [details]
Patch

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

> Source/WebKit2/NetworkProcess/cache/NetworkCache.h:87
> +using GlobalFrameID = std::pair<uint64_t /*webPageID*/, uint64_t /*webFrameID*/>;

It would be slightly nicer if this was a struct with named fields though I suppose that would make using it as hash key more complicated.
Comment 11 Chris Dumez 2015-11-11 13:24:16 PST
(In reply to comment #10)
> Comment on attachment 265301 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=265301&action=review
> 
> > Source/WebKit2/NetworkProcess/cache/NetworkCache.h:87
> > +using GlobalFrameID = std::pair<uint64_t /*webPageID*/, uint64_t /*webFrameID*/>;
> 
> It would be slightly nicer if this was a struct with named fields though I
> suppose that would make using it as hash key more complicated.

How strongly do you feel about this? :) As you said, I would need to provide HashTraits if I did this. Also, the code should really not need to access actual pair values. The only code that does currently is the Cache Statistics code which is not production code.
Comment 12 Antti Koivisto 2015-11-11 14:11:40 PST
(In reply to comment #11)
> (In reply to comment #10)
> > Comment on attachment 265301 [details]
> > Patch
> > 
> > View in context:
> > https://bugs.webkit.org/attachment.cgi?id=265301&action=review
> > 
> > > Source/WebKit2/NetworkProcess/cache/NetworkCache.h:87
> > > +using GlobalFrameID = std::pair<uint64_t /*webPageID*/, uint64_t /*webFrameID*/>;
> > 
> > It would be slightly nicer if this was a struct with named fields though I
> > suppose that would make using it as hash key more complicated.
> 
> How strongly do you feel about this? :) As you said, I would need to provide
> HashTraits if I did this. Also, the code should really not need to access
> actual pair values. The only code that does currently is the Cache
> Statistics code which is not production code.

Not strongly at all.
Comment 13 Chris Dumez 2015-11-11 14:12:38 PST
Comment on attachment 265301 [details]
Patch

go go go
Comment 14 WebKit Commit Bot 2015-11-11 15:01:30 PST
Comment on attachment 265301 [details]
Patch

Clearing flags on attachment: 265301

Committed r192328: <http://trac.webkit.org/changeset/192328>
Comment 15 WebKit Commit Bot 2015-11-11 15:01:36 PST
All reviewed patches have been landed.  Closing bug.