[webkit-reviews] review granted: [Bug 63674] Get webkit to compile with USE(CFNETWORK) enabled on Mac : [Attachment 99195] Patch 1 - Changes to ResourceHandle class

[bugzilla-daemon at webkit.org]

David Kilzer (ddkilzer) <ddkilzer at webkit.org> has granted Pratik Solanki
<psolanki at apple.com>'s request for review:
Bug 63674: Get webkit to compile with USE(CFNETWORK) enabled on Mac
https://bugs.webkit.org/show_bug.cgi?id=63674

Attachment 99195: Patch 1 - Changes to ResourceHandle class
https://bugs.webkit.org/attachment.cgi?id=99195&action=review

------- Additional Comments from David Kilzer (ddkilzer) <ddkilzer at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=99195&action=review

r=me

> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:214
> +    int statusCode =  msg ? CFHTTPMessageGetResponseStatusCode(msg) : 0;

Nit: Extra space after "=".

>>>> Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:220
>>>> +	      CFURLResponseSetMIMEType(cfResponse, CFSTR("text/html"));
>>> 
>>> This seemed like a change in behavior to until I looked in
ResourceHandleMac.mm and saw the NSURLResponse version of this code. A note in
the ChangeLog about why this was added would have cleared this up.
>> 
>> It'd be even nicer to share this with the Mac rather than duplicating it.
The Mac port could do this in terms of CF API and toll-free bridging should
ensure that the NSURLResponse has the right changes.
> 
> Yes thats a good idea. I can refactor the code so its shared between the two
implementations.

Make sure NSURLResponse is toll-free-bridged to CFURLResponse.	Not all of the
NSURL* types are toll-free-bridged.