RESOLVED FIXED 202795
SpeculativeLoad should use CompletionHandler 
https://bugs.webkit.org/show_bug.cgi?id=202795
Summary SpeculativeLoad should use CompletionHandler
Rob Buis
Reported 2019-10-10 06:22:20 PDT
SpeculativeLoad should use CompletionHandler, as remarked in https://bugs.webkit.org/show_bug.cgi?id=201461 review.
Attachments
Patch (3.40 KB, patch)
2019-10-10 06:25 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Patch (1.78 KB, patch)
2019-10-10 08:53 PDT, Rob Buis 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Rob Buis
Comment 1 2019-10-10 06:25:06 PDT
Created attachment 380638 [details] Patch
Chris Dumez
Comment 2 2019-10-10 08:25:25 PDT
Comment on attachment 380638 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380638&action=review > Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:45 > +SpeculativeLoad::SpeculativeLoad(Cache& cache, const GlobalFrameID& globalFrameID, const ResourceRequest& request, std::unique_ptr<NetworkCache::Entry> cacheEntryForValidation, WTF::CompletionHandler<void(std::unique_ptr<NetworkCache::Entry>)>&& completionHandler) No WTF:: please. Same comment applies everywhere below. > Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h:-46 > - typedef Function<void (std::unique_ptr<NetworkCache::Entry>)> RevalidationCompletionHandler; Why did you drop the typedef? I would have kept it and simply used a CompletionHandler instead of a Function.
Rob Buis
Comment 3 2019-10-10 08:53:29 PDT
Created attachment 380648 [details] Patch
Rob Buis
Comment 4 2019-10-10 08:54:58 PDT
Comment on attachment 380638 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=380638&action=review >> Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:45 >> +SpeculativeLoad::SpeculativeLoad(Cache& cache, const GlobalFrameID& globalFrameID, const ResourceRequest& request, std::unique_ptr<NetworkCache::Entry> cacheEntryForValidation, WTF::CompletionHandler<void(std::unique_ptr<NetworkCache::Entry>)>&& completionHandler) > > No WTF:: please. Same comment applies everywhere below. Fixed. >> Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoad.h:-46 >> - typedef Function<void (std::unique_ptr<NetworkCache::Entry>)> RevalidationCompletionHandler; > > Why did you drop the typedef? I would have kept it and simply used a CompletionHandler instead of a Function. I saw another such usage of CompletionHandler and assumed that was the way to go.
WebKit Commit Bot
Comment 5 2019-10-10 09:37:20 PDT
Comment on attachment 380648 [details] Patch Clearing flags on attachment: 380648 Committed r250974: <https://trac.webkit.org/changeset/250974>
WebKit Commit Bot
Comment 6 2019-10-10 09:37:22 PDT
All reviewed patches have been landed. Closing bug.
Radar WebKit Bug Importer
Comment 7 2019-10-10 09:38:15 PDT
<rdar://problem/56157710>
Note You need to log in before you can comment on or make changes to this bug.