[webkit-reviews] review granted: [Bug 20065] Remove UChar* accessors from StringImpl : [Attachment 22340] First attempt, push logic from String.cpp into StringImpl.cpp

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 20065: Remove UChar* accessors from StringImpl
https://bugs.webkit.org/show_bug.cgi?id=20065

Attachment 22340: First attempt, push logic from String.cpp into StringImpl.cpp
https://bugs.webkit.org/attachment.cgi?id=22340&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
These changes are OK, but I don't think they really free us up to change the
implementation. There are clients of String::characters() and for good reason.

 102	 // String::append and insert are extremely in-efficient, if you're
doing many
 103	 // appends and inserts, consider using a Vector<UChar> or
SegmetedString

inefficient doesn't have a "-" in it.

SegmentedString has an "n" in it.

 162	 m_impl = m_impl->insert(charactersToInsert, lengthToInsert, position);


Why did you remove the ASSERT(m_impl) from this function?

I noticed that StringImpl::append(StringImpl*) does not optimize the
zero-length case. The old code didn't either, but that's not good.

I also think it's a bit strange to use codePointAt everywhere instead of the []
operator.

r=me