[webkit-reviews] review denied: [Bug 14225] Make it possible to define platform-specific ResourceRequest without #ifdefs : [Attachment 15133] partial patch v2 (mac only)

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has denied MorganL <morganl.webkit at yahoo.com>'s
request for review:
Bug 14225: Make it possible to define platform-specific ResourceRequest without
#ifdefs
http://bugs.webkit.org/show_bug.cgi?id=14225

Attachment 15133: partial patch v2 (mac only)
http://bugs.webkit.org/attachment.cgi?id=15133&action=edit

------- Additional Comments from Darin Adler <darin at apple.com>
I'm a little concerned about having files with identical names but different
contents in the various platform-specific subdirectories. Up until this point
we've mostly avoided this, and it helps with build systems that have trouble
understanding identically named source files. But I don't see an elegant
alternative, so I guess we should go with it.

+#if PLATFORM(MAC)
+#include <wtf/RetainPtr.h>
+#ifdef __OBJC__
+ at class NSURLRequest;
+#else
+class NSURLRequest;
+#endif
+#endif

I don't think that ResourceRequest.h in the mac subdirectory needs an #if
PLATFORM(MAC). So you can just remove the ifdef.

+#include "config.h"
+#include "ResourceRequest.h"

I think it would be slightly better to explicitly include first
"ResourceRequestBase.h" and then "ResourceRequest.h", even though that amounts
to the same thing as just including "ResourceRequest.h". I want to take our
"always includes your own header first" rule literally in cases like this.

+   
const_cast<ResourceRequest&>(asResourceRequest()).doUpdatePlatformRequest();

I believe this won't compile. It's not just a  const_cast, it's also a
downcast, so it needs to be a combination of const_cast and static_cast or a
C-style cast. Did you try compiling this?

I'd slightly prefer it if we could come up with a way to do the downcast to
ResourceRequest without a static_cast at each call site. Maybe an inline helper
function would make the syntax a little sweeter?