[webkit-reviews] review granted: [Bug 128517] Stop using String::deprecatedCharacters to call WTF::Collator : [Attachment 223713] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Darin Adler <darin at apple.com>'s
request for review:
Bug 128517: Stop using String::deprecatedCharacters to call WTF::Collator
https://bugs.webkit.org/show_bug.cgi?id=128517

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

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=223713&action=review


r=me

This patch doesn't apply any more, so it would be useful to let EWS work on an
updated version.

> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:71
> +static inline char* copyASCIIString(CFStringRef string)

Maybe a name like copyShortASCIIString or copyLocaleNameToASCIIString would
make it more obvious why the implementation is not general purpose.

> Source/WTF/wtf/unicode/icu/CollatorICU.cpp:95
> +    static char* defaultLocale = copyDefaultLocale();

I like to ASSERT(isMainThread()) everywhere a static variables are used like
this.

Or maybe this could be made thread safe.