Fix inefficiency in HTTPHeaderMap::set(CFStringRef, const String&).
Created attachment 375639 [details] Patch
Comment on attachment 375639 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=375639&action=review > Source/WebCore/platform/network/HTTPHeaderMap.cpp:75 > + if (findHTTPHeaderName(StringView(reinterpret_cast<const LChar*>(nameCharacters), length), headerName)) > + set(headerName, value); > + else > + setUncommonHeader(String(nameCharacters, length), value); Strikes me as strange that StringView requires the cast to LChar* and String does not.
(In reply to Darin Adler from comment #2) > Comment on attachment 375639 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=375639&action=review > > > Source/WebCore/platform/network/HTTPHeaderMap.cpp:75 > > + if (findHTTPHeaderName(StringView(reinterpret_cast<const LChar*>(nameCharacters), length), headerName)) > > + set(headerName, value); > > + else > > + setUncommonHeader(String(nameCharacters, length), value); > > Strikes me as strange that StringView requires the cast to LChar* and String > does not. StringView(const LChar*, unsigned length); StringView(const UChar*, unsigned length); StringView(const char*); We're missing: StringView(const char*, unsigned length); Should I add it?
(In reply to Chris Dumez from comment #3) > StringView(const LChar*, unsigned length); > StringView(const UChar*, unsigned length); > StringView(const char*); > > We're missing: > StringView(const char*, unsigned length); > > Should I add it? Yes. If we already have the one without a length, then yes I think you should add it. The counter argument is that if we have a "const char*" with UTF-8 in it, we don’t want someone to accidentally interpret it as Latin-1. But I’m not sure that leaving this one particular overload out of StringView helps much with that problem.
Created attachment 375643 [details] Patch
Comment on attachment 375643 [details] Patch Clearing flags on attachment: 375643 Committed r248316: <https://trac.webkit.org/changeset/248316>
All reviewed patches have been landed. Closing bug.
<rdar://problem/54004727>