[webkit-reviews] review granted: [Bug 22168] Chromium is seeing crashes using TextIterator : [Attachment 25036] Add ASSERT to catch crash after advance()

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Eric Seidel <eric at webkit.org>'s
request for review:
Bug 22168: Chromium is seeing crashes using TextIterator
https://bugs.webkit.org/show_bug.cgi?id=22168

Attachment 25036: Add ASSERT to catch crash after advance()
https://bugs.webkit.org/attachment.cgi?id=25036&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
I see no harm in adding this assertion, but little benefit in doing so. The
line that initializes m_lastCharacter will crash if str.characters() is 0; this
will move that crash up a few lines but not detect any additional failure
cases.

The evidence does not match the theory that renderer->text() is a null string.
In TextIterator::handleTextBox we've already fetched renderer->text() and
dereferenced it by calling str[runStart] before calling emitText. So there's no
real chance that it's a null string in that code path.

If characters() is 0x02 that does not indicate a null string. Instead it
indicates a StringImpl that has been deallocated or overwritten. characters()
is 0 in a null string.

r=me because there's no harm in adding the assertion.