Summary: | [iOS] Support using Foundation networking code | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Daniel Bates <dbates> | ||||
Component: | Platform | Assignee: | Daniel Bates <dbates> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | aestes, benjamin, cmarcelo, commit-queue, ddkilzer, japhet, mjs, psolanki | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | iPhone / iPad | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Daniel Bates
2014-09-02 15:27:05 PDT
Created attachment 237522 [details]
Patch
Attachment 237522 [details] did not pass style-queue:
ERROR: Source/WebCore/platform/spi/cf/CFNetworkConnectionCacheSPI.h:33: enum members should use InterCaps with an initial capital letter. [readability/enum_casing] [4]
Total errors found: 1 in 16 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 237522 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=237522&action=review I think I would prefer the platform name come first e.g. #if PLATFORM(IOS) && !USE(CFNETWORK) instead of #if !USE(CFNETWORK) && PLATFORM(IOS) Seems easier to read for me. But what you have is fine as well if that is what we use in other code. > Source/WebCore/loader/ResourceLoader.cpp:583 > +#if USE(CF_NETWORK) && PLATFORM(IOS) This should be USE(CFNETWORK) > Source/WebCore/platform/network/mac/ResourceErrorMac.mm:38 > #import <Foundation/NSURLError.h> Not part of your patch, but I don't think we need this include anymore. Foundation.h seems to include NSURLError.h. (In reply to comment #3) > (From update of attachment 237522 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=237522&action=review > > I think I would prefer the platform name come first e.g. > > #if PLATFORM(IOS) && !USE(CFNETWORK) > > instead of > > #if !USE(CFNETWORK) && PLATFORM(IOS) > > Seems easier to read for me. But what you have is fine as well if that is what we use in other code. > Will update the patch such that we query PLATFORM(IOS) before USE(CFNETWORK). > > Source/WebCore/loader/ResourceLoader.cpp:583 > > +#if USE(CF_NETWORK) && PLATFORM(IOS) > > This should be USE(CFNETWORK) > Notice that ResourceLoader.cpp is compiled in a Windows build by <http://trac.webkit.org/export/173191/trunk/Source/WebCore/WebCore.vcxproj/WebCore.vcxproj>. Also ResourceHandleMac.mm and ResourceHandleCFNet.cpp both call ResourceHandleClient::connectionProperties() and ResourceLoader::connectionProperties() overrides ResourceHandleClient::connectionProperties(). So, this macro guard should read: #if PLATFORM(IOS) > > Source/WebCore/platform/network/mac/ResourceErrorMac.mm:38 > > #import <Foundation/NSURLError.h> > > Not part of your patch, but I don't think we need this include anymore. Foundation.h seems to include NSURLError.h. Will remove header <Foundation/NSURLError.h> before landing. Committed r173192: <http://trac.webkit.org/changeset/173192> Committed Windows build fix in <http://trac.webkit.org/changeset/173196>. |