[webkit-reviews] review granted: [Bug 195247] [SOUP] Misnamed function in SoupNetworkSession : [Attachment 363424] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Sat Mar 2 15:12:46 PST 2019


Daniel Bates <dbates at webkit.org> has granted Michael Catanzaro
<mcatanzaro at igalia.com>'s request for review:
Bug 195247: [SOUP] Misnamed function in SoupNetworkSession
https://bugs.webkit.org/show_bug.cgi?id=195247

Attachment 363424: Patch

https://bugs.webkit.org/attachment.cgi?id=363424&action=review




--- Comment #2 from Daniel Bates <dbates at webkit.org> ---
Comment on attachment 363424
  --> https://bugs.webkit.org/attachment.cgi?id=363424
Patch

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

> Source/WebCore/platform/network/soup/SoupNetworkSession.cpp:101
> +static HashMap<String, HostTLSCertificateSet, ASCIICaseInsensitiveHash>&
allowedCertificates()

Ok as-is. Just a thought, define a type alias for the HashMap to avoid
repetition below.

> Source/WebCore/platform/network/soup/SoupNetworkSession.cpp:287
> +    auto it = allowedCertificates().find(requestURL.host().toString());

Ok as-is, but can be made a bit more efficient by using
toStringWithoutCopying(). Even better, as I type this I could swear we solved
this issue with template magic and made it so you could pass StringView to
HashMap::find(). Can you check? I think we call this magic a HashTranslator or
adapter and there is a template overload of HashMap::find(). And if we don’t
have such magic because we are missing the HashTranslator for StringView then
please either add it OR file a bug so that we can add it.


More information about the webkit-reviews mailing list