[webkit-reviews] review denied: [Bug 27511] Add WinCE specific platform/graphics files to WebCore : [Attachment 34394] 3) ImageBuffer

[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 27511: Add WinCE specific platform/graphics files to WebCore
https://bugs.webkit.org/show_bug.cgi?id=27511

Attachment 34394: 3) ImageBuffer
https://bugs.webkit.org/attachment.cgi?id=34394&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
The ChangeLog really should have more information on what these classes are
for, what they do.  Why does the WinCE port use this new "SharedBitmap" and
what is it for?  It seems ImageBufferData is just to allow ImageBuffer to be
backed by a SharedBitmap, the question is why? ;)  Likewise BufferedImage seems
to exist so that Image can be backed by a ImageBufferData.  Why wouldn't WinCE
have used the BitmapImage abstraction that other classes use?  Image exists to
abstract between SVGImage, PDFDocumentImage and BitmapImage.  Seems you have
created a duplicate/parallel BitmapImage-like subclass, no?

(Not necessarily giving objection here, just seeking more information!) :)

Need more info in the ChangeLog to perform a good review.  Also, I think given
how comment-full this bug is now, these patches really should be each on their
own bugs so they can be discussed where folks have some prayer of being able to
read all the relevant comments. :)

Indent:
+: m_data(size)
+, m_size(size)

UNUSED_ARG is what you want:
+    grayScale; // Currently not used

c++ casts:
+    const unsigned char* src = (unsigned char*)m_data.m_bitmap->bytes();

r-, mostly for the lack of ChangeLog information.