[webkit-reviews] review granted: [Bug 25709] Skia/Cairo image decoders should be merged : [Attachment 31073] part three

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has granted Peter Kasting <pkasting at google.com>'s
request for review:
Bug 25709: Skia/Cairo image decoders should be merged
https://bugs.webkit.org/show_bug.cgi?id=25709

Attachment 31073: part three
https://bugs.webkit.org/attachment.cgi?id=31073&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
+	   http://code.google.com/p/chromium/issues/detail?id=3643.

Can we just add the test as part of this change?  (I don't need to see the
test, but you could land it with this.)

Would be nice to eventually add a setColor function which took an IntPoint and
a Color instead of:
+	     m_frameBufferCache.first().setRGBA(m_coord.x(), m_coord.y(), red,
green, blue, alpha);

obviously not for this patch. :)

It's not clear to me from the ChangeLog why the removal of these lines:
-	     buffer->setStatus(RGBA32Buffer::FrameComplete);

is correct.  But I trust you.

Very sad that setSize() doesn't take an IntSize :)

I'm assuming this:
-		       buffer->setRGBA(bitmap.getAddr32(x, y), 0, 0, 0, 0);
is not needed because x/y are already 32-bit checked or some such?  Unclear why
the change.

I am not the right person to review this technically.  But I don't think that
person exists in the set of webkit reviewers.  I trust here that you've checked
any needed parts with people who do have Skia knowledge.

You have my rubber stamp.

Thank you for making this so small.