[webkit-reviews] review granted: [Bug 72323] Towards 8 bit Strings - Update utf8() and ascii() methods for 8 bit strings : [Attachment 115059] Patch

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 72323: Towards 8 bit Strings - Update utf8() and ascii() methods for 8 bit
strings
https://bugs.webkit.org/show_bug.cgi?id=72323

Attachment 115059: Patch
https://bugs.webkit.org/attachment.cgi?id=115059&action=review

------- Additional Comments from Geoffrey Garen <ggaren at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=115059&action=review


r=me

> Source/JavaScriptCore/runtime/UString.cpp:425
> +    if (is8Bit()) {
> +	   const LChar* characters = this->characters8();
> +
> +	   ConversionResult result = convertUTF16ToUTF8(&characters, characters
+ length, &buffer, buffer + bufferVector.size());
> +	   ASSERT_UNUSED(result, result != targetExhausted); // (length * 3)
should be sufficient for any conversion

I think it's confusing to pretend that our underlying buffer is UTF16, when our
convention is to say that it's Latin1. So, to produce UTF8 data from an LChar
buffer, let's use a function called convertLatin1ToUTF8.

> Source/JavaScriptCore/wtf/text/WTFString.cpp:732
> +	   ConversionResult result = convertUTF16ToUTF8(&characters, characters
+ length, &buffer, buffer + bufferVector.size());
> +	   ASSERT_UNUSED(result, result != targetExhausted); // (length * 3)
should be sufficient for any conversion

Same comment here.