Bug 202795 - SpeculativeLoad should use CompletionHandler
Summary: SpeculativeLoad should use CompletionHandler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Rob Buis
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-10-10 06:22 PDT by Rob Buis
Modified: 2019-10-10 09:38 PDT (History)
6 users (show)

See Also:

Attachments
Patch (3.40 KB, patch)
2019-10-10 06:25 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Patch (1.78 KB, patch)
2019-10-10 08:53 PDT, Rob Buis 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Rob Buis 2019-10-10 06:22:20 PDT 
SpeculativeLoad should use CompletionHandler, as remarked in https://bugs.webkit.org/show_bug.cgi?id=201461 review.
Comment 1 Rob Buis 2019-10-10 06:25:06 PDT 
Created attachment 380638 [details]
Patch
Comment 2 Chris Dumez 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.
Comment 3 Rob Buis 2019-10-10 08:53:29 PDT 
Created attachment 380648 [details]
Patch
Comment 4 Rob Buis 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.
Comment 5 WebKit Commit Bot 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>
Comment 6 WebKit Commit Bot 2019-10-10 09:37:22 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-10-10 09:38:15 PDT 
<rdar://problem/56157710>