[webkit-reviews] review granted: [Bug 87521] characterBreakIterator() is not safe to use reentrantly or from multiple threads : [Attachment 144118] Add and deply a NonSharedCharacterBreakIterator class

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted mitz at webkit.org's request for review:
Bug 87521: characterBreakIterator() is not safe to use reentrantly or from
multiple threads
https://bugs.webkit.org/show_bug.cgi?id=87521

Attachment 144118: Add and deply a NonSharedCharacterBreakIterator class
https://bugs.webkit.org/attachment.cgi?id=144118&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=144118&action=review


> Source/WebCore/platform/text/TextBreakIterator.h:112
> +    TextBreakIterator* get() const { return m_iterator; }

MIght want to consider the type conversion operator for this instead of an
explicit get function.

> Source/WebCore/platform/text/TextBreakIteratorICU.cpp:93
> +    bool createdIterator = m_iterator &&
weakCompareAndSwap(reinterpret_cast<void**>(&nonSharedCharacterBreakIterator),
m_iterator, 0);

The reinterpret_cast<void**> idiom is particularly unpleasant. Might be worth
looking into a refactoring to clean that up and put it in one place.

> Source/WebCore/platform/text/wince/TextBreakIteratorWinCE.cpp:247
> +	   m_iterator = new CharBreakIterator();

I normally omit the () in lines of code like this.