[webkit-reviews] review denied: [Bug 22116] Add some more PLATFORM(CHROMIUM) #ifdef's to WebCore (ICU difference) : [Attachment 24960] patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Jungshik Shin <jshin at chromium.org>'s
request for review:
Bug 22116: Add some more PLATFORM(CHROMIUM) #ifdef's to WebCore (ICU
difference)
https://bugs.webkit.org/show_bug.cgi?id=22116

Attachment 24960: patch 
https://bugs.webkit.org/attachment.cgi?id=24960&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
I'm going to mark this r- (to remove it from the review queue), pending further
comment from Jungshik.	Seems like 2 questions were raised above by ap and
myself:

1.  What is the harm in leaving any of these workarounds in, even though
chromium uses a fixed version of ICU?  Maybe this is very performance sensitive
code? (probably is) and thus removing these workarounds is a speedup?

2.  If we're going to conditionals these workarounds, we should probably do so
with some smarter #defines.  Here are two possible examples:


#define USE_ICU_3_X_LINE_BREAK_WORKAROUND (U_ICU_VERSION_MAJOR_NUM < 3 &&
!PLATFORM(CHROMIUM))

and the other one seems like it could just be an ICU major/minor version check
for 3.3 or later, no?  Maybe it too needs a nice
USE_ICU_3_X_EXTRA_ENCODING_ALIASES or something.