[webkit-reviews] review granted: [Bug 239453] Use convertToASCIILowercase() less and more SortedArrayMap / SortedArraySet : [Attachment 457809] 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 239453: Use convertToASCIILowercase() less and more SortedArrayMap /
SortedArraySet
https://bugs.webkit.org/show_bug.cgi?id=239453

Attachment 457809: Patch

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




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

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

> Source/WebCore/Modules/applicationmanifest/ApplicationManifestParser.cpp:157
> +    if (auto* displayValue =
displayValues.tryGet(StringView(stringValue).stripWhiteSpace()))

Not changing in this patch: Seems like this should be stripping leading and
trailing *HTML* whitespace, probably a subtle and unimportant that it strips
all *ASCII* whitespace, which includes one character that HTML does not. I also
think it’s a little strange that the StringView function is named
stripWhiteSpace, because we normally say ASCIISpace or HTMLSpace or HTTPSpace
and just "space" typically means some kind of Unicode space.

> Source/WebDriver/WebDriverService.cpp:232
> +    if (auto* methodValue = httpMethods.tryGet(method))
> +	   return *methodValue;
>      return std::nullopt;

Makes me wish we had a "convert pointer to optional" function you could have
used here.