[webkit-reviews] review denied: [Bug 27371] [WINCE] Add WinCE specific platform/text files to WebCore : [Attachment 38902] modified as Eric suggested

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Yong Li <yong.li at torchmobile.com>'s
request for review:
Bug 27371: [WINCE] Add WinCE specific platform/text files to WebCore
https://bugs.webkit.org/show_bug.cgi?id=27371

Attachment 38902: modified as Eric suggested
https://bugs.webkit.org/attachment.cgi?id=38902&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
Style:
 247	 static WordBreakIterator *iterator = 0;

Also, these lines can be combined:
247	static WordBreakIterator *iterator = 0;
 248	 if (!iterator)
 249	     iterator = new WordBreakIterator;

static WordBreakIterator* iterator = new WordBreakIterator;

that will only run once on the function's first run.  Likewise you could just
use:

static WordBreakIterator iterator; and return &iterator instead.  Although you
might need to use the DEFINE_STATIC_LOCAL macro.

Please add a contructor for your BreakIterators sine they all seem to share the
same initialization code.

Longer variable names are preferrd:
int textBreakFirst(TextBreakIterator* bi)
 298 {
 299	 return bi->first();
 300 }

Why WebCore::?
 420	     WebCore::decode(result, m_encoding.name(), bytes, length, &left,
m_decodeBuffer.isEmpty(), sawInvalidChar);

Why L'' here?
 427	     result.append(L'?');