[webkit-reviews] review denied: [Bug 5232] Rewrite QTextCodec::fromUnicode to use ICU : [Attachment 5042] proposed patch

[bugzilla-request-daemon at opendarwin.org]

Darin Adler <darin at apple.com> has denied Alexey Proskuryakov <ap at nypop.com>'s
request for review:
Bug 5232: Rewrite QTextCodec::fromUnicode to use ICU
http://bugzilla.opendarwin.org/show_bug.cgi?id=5232

Attachment 5042: proposed patch
http://bugzilla.opendarwin.org/attachment.cgi?id=5042&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Looks OK.

It's unfortunate that the loop uses QCString::append because that makes the
loop n^2 in the size of the string. Each conversion buffer has its size
recalculated using strlen() and worse, the base QCString has its size
recalculated using strlen() each time QCString::append is called.

A more efficient version would use QCString::resize and keep track of the
length of the result.

The original CFString version had this problem too, so I won't do a review- on
this just because of it, but it's worth fixing.

I'm concerned about the handling of U+005C in ISO-2022-JP, but it looks like
it's no worse after this change than before.

What are we going to do about character sets not yet supported by ICU, such as
KOI8-U (see bug 4195)?

Setting review- until I have the answer to at least this last question.