[webkit-reviews] review granted: [Bug 23092] Conditionalize CFNetwork-specific logic in WebKit.dll : [Attachment 26516] Update using svn copy to create new CFNet versions

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted Brent Fulgham
<bfulgham at gmail.com>'s request for review:
Bug 23092: Conditionalize CFNetwork-specific logic in WebKit.dll
https://bugs.webkit.org/show_bug.cgi?id=23092

Attachment 26516: Update using svn copy to create new CFNet versions
https://bugs.webkit.org/attachment.cgi?id=26516&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> Index: WebKit/win/WebDownload.cpp
> ===================================================================
> --- WebKit/win/WebDownload.cpp	(revision 39682)
> +++ WebKit/win/WebDownload.cpp	(working copy)
> @@ -38,12 +38,16 @@
>  #include "WebURLCredential.h"
>  #include "WebURLResponse.h"
>  
> +#include <wtf/platform.h>

I'm surprised you need to include this. I thought config.h pulled it in. In any
case, "Platform" should be capitalized, and this header should be in ASCII
order with the others (unless there's a reason why it can't be, in which case a
comment would be good).

> +#if USE(CFNETWORK)
>  #include <WebCore/AuthenticationCF.h>
> +#endif

I'm surprised this #include is still needed in WebDownload.cpp after these
changes.

> +    static const WebCore::String BundleExtension;
> +    static const UInt32 BundleMagicNumber;

I think a slightly better approach would be to make some static getter
functions to return these values. The main advantage of this is that it won't
require the BundleExtension String to be constructed when the DLL is first
loaded (a "static initializer"). Whether or not you turn these into getter
functions, they should begin with lowercase letters.

r=me