Bug 230565 - Pending preconnect key should include User-Agent
Summary: Pending preconnect key should include User-Agent
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Ben Nham
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-21 10:35 PDT by Ben Nham
Modified: 2021-09-25 09:12 PDT (History)
4 users (show)

See Also:

Attachments
Patch (7.66 KB, patch)
2021-09-21 11:06 PDT, Ben Nham 		no flags Details | Formatted Diff | Diff
Patch (7.74 KB, patch)
2021-09-21 22:33 PDT, Ben Nham 		no flags Details | Formatted Diff | Diff
Patch (8.07 KB, patch)
2021-09-24 14:00 PDT, Ben Nham 		no flags Details | Formatted Diff | Diff
Show Obsolete (2) 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 Ben Nham 2021-09-21 10:35:26 PDT 
Pending preconnect key should include User-Agent
Comment 1 Ben Nham 2021-09-21 11:06:16 PDT 
Created attachment 438836 [details]
Patch
Comment 2 Ben Nham 2021-09-21 11:08:39 PDT 
<rdar://83259078>
Comment 3 Ben Nham 2021-09-21 22:33:24 PDT 
Created attachment 438922 [details]
Patch
Comment 4 Chris Dumez 2021-09-24 13:14:09 PDT 
Comment on attachment 438922 [details]
Patch

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

r=me with changes.

> Source/WebKit/NetworkProcess/NetworkLoadScheduler.cpp:156
> +static std::tuple<String, String> mainResourceLoadKey(String protocolHostAndPort, String userAgent)

`const String&` for both parameters to avoid ref-counting churn.

> Source/WebKit/NetworkProcess/NetworkLoadScheduler.h:54
> +    void startedPreconnectForMainResource(const URL&, const String&);

Please do not omit the second parameter name as it is not obvious what it is.

> Source/WebKit/NetworkProcess/NetworkLoadScheduler.h:55
> +    void finishedPreconnectForMainResource(const URL&, const String&, const WebCore::ResourceError&);

ditto.

> Source/WebKit/NetworkProcess/NetworkLoadScheduler.h:81
> +    using PendingPreconnectMap = HashMap<std::tuple<String, String>, PendingMainResourcePreconnectInfo>;

We probably want inline comments to indicate what those Strings are.
Comment 5 Ben Nham 2021-09-24 14:00:25 PDT 
Created attachment 439195 [details]
Patch
Comment 6 EWS 2021-09-25 09:12:33 PDT 
Committed r283081 (242139@main): <https://commits.webkit.org/242139@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 439195 [details].