[webkit-reviews] review granted: [Bug 71337] Enable 8 Bit Strings in JavaScriptCore : [Attachment 115402] Patch for review

[bugzilla-daemon at webkit.org]

Geoffrey Garen <ggaren at apple.com> has granted Michael Saboff
<msaboff at apple.com>'s request for review:
Bug 71337: Enable 8 Bit Strings in JavaScriptCore
https://bugs.webkit.org/show_bug.cgi?id=71337

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

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


r=me

> Source/JavaScriptCore/ChangeLog:13
> +	   type.  This change riplled into WebCore code as well.

Typo: riplled.

> Source/JavaScriptCore/runtime/Identifier.cpp:172
> +    if (string.is8Bit()) {

Would be nice to have a template helper for this, to achieve only one copy of
the code vs two.

> Source/JavaScriptCore/wtf/text/StringImpl.cpp:540
> +	   const LChar* from = characters8();

This is another candidate for a template helper function.

> Source/JavaScriptCore/wtf/text/StringImpl.cpp:-1351
> -	       if (!bc)

This is another candidate for a template helper function -- would have only
needed to fix this bug in one place instead of two.

> Source/JavaScriptCore/wtf/text/StringImpl.cpp:1579
> +PassRefPtr<StringImpl> StringImpl::adopt(StringBuffer<LChar>& buffer)

This is another candidate for templatization.