[webkit-reviews] review denied: [Bug 16979] Patch to conditionalize some CG/Cairo support in win32 : [Attachment 18731] Third revision based on Darin and Alp's comments.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Jan 28 09:16:36 PST 2008


Adam Roben <aroben at apple.com> has denied Brent Fulgham <bfulgham at gmail.com>'s
request for review:
Bug 16979: Patch to conditionalize some CG/Cairo support in win32
http://bugs.webkit.org/show_bug.cgi?id=16979

Attachment 18731: Third revision based on Darin and Alp's comments.
http://bugs.webkit.org/attachment.cgi?id=18731&action=edit

------- Additional Comments from Adam Roben <aroben at apple.com>
I think it would be nicer to use 'svn cp' when creating the *CGWin.cpp files.
For example:

svn cp FrameWin.cpp FrameCGWin.cpp

That way it'll be easier to track the changes to the methods defined in the
*CGWin.cpp files across the split in history.

The #includes in FrameCairoWin.cpp could probably be reduced quite a bit.

A number of your new methods in the *CairoWin.cpp files throughout this patch
are missing return statements.

+GraphicsContext::GraphicsContext(HDC hdc)
+//    : m_common(createGraphicsContextPrivate())
+//    , m_data(new GraphicsContextPlatformPrivate(CGContextWithHDC(hdc)))
+{
+//    CGContextRelease(m_data->m_cgContext);

We generally don't like to commit commented-out code.

I think this is very close to being ready to go in to Subversion.


More information about the webkit-reviews mailing list