[webkit-reviews] review denied: [Bug 17484] Add Windows (Cairo) support in WebKit.dll : [Attachment 19268] Patch to conditionalize some CG/Cairo calls in WebKit proper.

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied Brent Fulgham <bfulgham at gmail.com>'s
request for review:
Bug 17484: Add Windows (Cairo) support in WebKit.dll
http://bugs.webkit.org/show_bug.cgi?id=17484

Attachment 19268: Patch to conditionalize some CG/Cairo calls in WebKit proper.
http://bugs.webkit.org/attachment.cgi?id=19268&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
Good general direction. Some comments:

 #include "config.h"
+#include "NotImplemented.h"
 #include "WebDragClient.h"

This is wrong. The first include after "config.h" should be the file's own
header "WebDragClient.h". There should be a blank line after that before the
rest of the includes.

+#else
+    notImplemented ();
+#endif

No space before the parenthesis.

 void WebFrame::dispatchDidReceiveAuthenticationChallenge(DocumentLoader*
loader, unsigned long identifier, const AuthenticationChallenge& challenge)
 {
+#if USE(CFNETWORK)

Not answering the challenge at all will simply make the browser hang. You
should probably do something better than that. Or is there some reason this is
OK?

+#if PLATFORM(CG)

It seems strange to just conditionalize the printing code without adding a call
to notImplemented(). How is this different from the createDragImage case?

+#if !PLATFORM(CAIRO)
 #if !defined(NDEBUG) && defined(USE_DEBUG_SAFARI_THEME)

I think this is backwards. It should be #if PLATFORM(CG), since SafariTheme
relies on CG. In the future we might need to add a conditional about whether
SafariTheme is being included, since someone with CG still might not want
SafariTheme. I definitely don't think that "CAIRO" is a good way to say "no
SafariTheme".

The purpose of WebKitGraphics is to give a way to draw text if you can't
compile C++ code. There's no reason to have it if the context is a
WebCore::GraphicsContext object. I suggest leaving these functions out entirely
for non-CG platforms.