[webkit-reviews] review granted: [Bug 179907] Reduce WTF::String operations that do unnecessary Unicode operations instead of ASCII : [Attachment 327412] Patch

[bugzilla-daemon at webkit.org]

Sam Weinig <sam at webkit.org> has granted Darin Adler <darin at apple.com>'s request
for review:
Bug 179907: Reduce WTF::String operations that do unnecessary Unicode
operations instead of ASCII
https://bugs.webkit.org/show_bug.cgi?id=179907

Attachment 327412: Patch

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




--- Comment #17 from Sam Weinig <sam at webkit.org> ---
Comment on attachment 327412
  --> https://bugs.webkit.org/attachment.cgi?id=327412
Patch

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

> Source/JavaScriptCore/dfg/DFGDesiredIdentifiers.cpp:84
> +    ASSERT(result->refCount());

Does this still do the right thing for immortal strings?

> Source/WebCore/page/SecurityOrigin.cpp:333
>  static bool isFeedWithNestedProtocolInHTTPFamily(const URL& url)
>  {

I wonder if we still need this function, given we don't really display feed:
urls anymore.

> Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp:95
> +	   auto key = header.left(splitPosition).stripWhiteSpace();

Can this be done as a single additional allocation by doing something like (the
admittedly much longer):

auto key = StringView { header
}.left(splitPosition).stripLeadingAndTrailingMatchedCharacters(SpaceOrNewlinePr
edicate { }).toString();

> Source/WebCore/platform/network/curl/ResourceResponseCurl.cpp:112
> -    auto httpVersionEndPosition = statusLine.find(" ");
> +    auto httpVersionEndPosition = statusLine.find(' ');

I wonder if we can make this a compile error, by statically asserting that a
literal string is longer than one character long.