[webkit-reviews] review granted: [Bug 17269] Deobfuscate CanvasRenderingContext2D.cpp : [Attachment 19029] Round 1 patch, add ImageData abstraction

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 11 09:14:25 PST 2008


Darin Adler <darin at apple.com> has granted Oliver Hunt <oliver at apple.com>'s
request for review:
Bug 17269: Deobfuscate CanvasRenderingContext2D.cpp
http://bugs.webkit.org/show_bug.cgi?id=17269

Attachment 19029: Round 1 patch, add ImageData abstraction
http://bugs.webkit.org/attachment.cgi?id=19029&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I think this is OK as an incremental step, but the entire thing needs to be
behind an abstraction later. I'm not sure this is helpful.

 67	ImageDataRef createPlatformImage() const;

Given the name of this, maybe the object should be PlatformImageRef.

 76 #if !PLATFORM(QT)
9177	   m_platformImage(0)
9278	 ,
9379 #endif

You could avoid this #if by writing:

    : m_platformImage()

It would initialize to 0 in the non-Qt case and call the default initializer in
the Qt case.

I see no reason not to wrap these objects in a class so you don't need all the
Qt strangeness and comments about not liking what Qt is doing. I don't think
that adding a data type like this is a good as doing that.

r=me, despite my doubts about your approach here.


More information about the webkit-reviews mailing list