[webkit-reviews] review granted: [Bug 239306] Replace calls to substring(0, x) with the more concise left(x) : [Attachment 457578] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Chris Dumez <cdumez at apple.com>'s
request for review:
Bug 239306: Replace calls to substring(0, x) with the more concise left(x)
https://bugs.webkit.org/show_bug.cgi?id=239306

Attachment 457578: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 457578
  --> https://bugs.webkit.org/attachment.cgi?id=457578
Patch

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

> Source/WebCore/platform/network/MIMEHeader.cpp:75
> +	   key =
StringView(line).left(semiColonIndex).convertToASCIILowercase().stripWhiteSpace
();

Seems *so* unlikely this (and the code above) really wants stripWhiteSpace.
Almost certainly wants to strip based on some other definitions of spaces
rather than "all Unicode whitespace". Should be MIME whitespace definition,
which is probably like the HTML or HTTP whitespace definition.

Almost every call to stripWhiteSpace is a mistake.

Also cute that this capitalizes semicolon as if it was two words, or
hyphenated.

> Source/WebCore/svg/SVGURIReference.cpp:72
>      URL kurl(base, fragmentIdentifier);

Ah, yes, "kurl".

> Source/WebCore/workers/service/server/SWServer.cpp:1271
> +    if (!scheme.startsWithIgnoringASCIICase("http"))

Nice one. Could use the even-more-optimized startsWithLettersIgnoringASCIICase.

> Source/WebKit/UIProcess/API/glib/InputMethodFilter.cpp:243
> +    auto cursorPositionUTF8 = cursorPosition != text.length() ?
StringView(text).left(cursorPosition).utf8().length() : textUTF8.length();

Not great to call utf8().length() instead of an algorithm that can compute a
UTF-8 length, without also allocating memory and copying the characters. But
doesn’t need to be fixed right now.