Bug 136107 – Add USE(APPLE_INTERNAL_SDK)-guards around SPI in ResourceHandle code

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Daniel Bates]

Towards getting iOS WebKit to build using the public SDK we should guard SPI headers with USE(APPLE_INTERNAL_SDK) and add appropriate forward declarations in the following files:

platform/network/ResourceHandleClient.h
platform/network/ResourceHandleInternal.h
platform/network/ios/ResourceHandleIOS.mm
platform/network/mac/ResourceErrorMac.mm

[Daniel Bates]

Created attachment 236875 [details]
Patch

[Daniel Bates]

Comment on attachment 236875 [details]
Patch

Will update patch to fix Windows build issues.

[David Kilzer (:ddkilzer)]

Is this still needed after the fix for this bug?

Bug 136487: [iOS] Make WebCore build and link with public SDK

[Blaze Burg]

There is still this:

#ifndef ResourceHandleClient_h
#define ResourceHandleClient_h

...

#if USE(CFNETWORK)
#include <CFNetwork/CFURLCachePriv.h>
#include <CFNetwork/CFURLResponsePriv.h>
#endif


The private includes could probably just be replaced with #include <CFNetworkSPI.h>.

[Frédéric Wang (:fredw)]

(In reply to Brian Burg from comment #4)
> There is still this:
> 
> #ifndef ResourceHandleClient_h
> #define ResourceHandleClient_h
> 
> ...
> 
> #if USE(CFNETWORK)
> #include <CFNetwork/CFURLCachePriv.h>
> #include <CFNetwork/CFURLResponsePriv.h>
> #endif
> 
> 
> The private includes could probably just be replaced with #include
> <CFNetworkSPI.h>.

It looks like USE_CFNETWORK has been removed in r207151. Do we still want that change?

[Frédéric Wang (:fredw)]

Created attachment 325704 [details]
Patch

[Frédéric Wang (:fredw)]

(In reply to Frédéric Wang (:fredw) from comment #6)
> Created attachment 325704 [details]
> Patch

Untested patch, based on previous comments.

[Frédéric Wang (:fredw)]

Created attachment 325705 [details]
Patch

[Frédéric Wang (:fredw)]

(In reply to Brian Burg from comment #4)
> There is still this:

Actually, it seems none of the changes were actually taken. So I just uploaded an untested rebase of Daniel's patch.

[Alex Christensen]

Comment on attachment 325705 [details]
Patch

Why are we trying to make iOS build using CFURLConnection?  Right now we're using NSURLConnection on iOS WebKitLegacy and NSURLSession on WebKit, and we're moving towards using NSURLSession everywhere.  We don't intend to maintain an iOS build that uses CFURLConnection.

[Frédéric Wang (:fredw)]

(In reply to Alex Christensen from comment #10)
> Comment on attachment 325705 [details]
> Patch
> 
> Why are we trying to make iOS build using CFURLConnection?  Right now we're
> using NSURLConnection on iOS WebKitLegacy and NSURLSession on WebKit, and
> we're moving towards using NSURLSession everywhere.  We don't intend to
> maintain an iOS build that uses CFURLConnection.

Then maybe this old bug should be WONTFIX :-)

[Darin Adler]

Comment on attachment 325705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325705&action=review

> Source/WebCore/platform/network/ios/ResourceHandleIOS.mm:42
> +#if USE(APPLE_INTERNAL_SDK)
>  #import <CFNetwork/CFSocketStreamPriv.h>
>  #import <Foundation/NSURLRequestPrivate.h>
> +#else
> +#import <Foundation/NSURLRequest.h>
> +@interface NSURLRequest (Details)
> ++ (BOOL)allowsAnyHTTPSCertificateForHost:(NSString *)host;
> ++ (NSArray*)allowsSpecificHTTPSCertificateForHost:(NSString *)host;
> +@end
> +#endif
>  
> -using namespace WebCore;
> +extern "C" const CFStringRef _kCFStreamSSLTrustedLeafCertificates;

In theory this belongs in an SPI.h header; someone should move it there eventually.

> Source/WebCore/platform/network/mac/ResourceErrorMac.mm:43
> -#if PLATFORM(IOS) && USE(CFURLCONNECTION)
> +#if PLATFORM(IOS) && USE(CFURLCONNECTION) && USE(APPLE_INTERNAL_SDK)
>  #import <CFNetwork/CFSocketStreamPriv.h>
>  #endif
>  
> +#if USE(CFURLCONNECTION)
> +extern "C" {
> +const CFStringRef _kCFStreamPropertySSLClientCertificates;
> +const CFStringRef _kCFStreamPropertySSLClientCertificateState;
> +}
> +#endif

Ditto.

[Frédéric Wang (:fredw)]

Committed r224395: <https://trac.webkit.org/changeset/224395>

[Frédéric Wang (:fredw)]

Comment on attachment 325705 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=325705&action=review

>> Source/WebCore/platform/network/ios/ResourceHandleIOS.mm:42
>> +extern "C" const CFStringRef _kCFStreamSSLTrustedLeafCertificates;
> 
> In theory this belongs in an SPI.h header; someone should move it there eventually.

Right. I see other places where it's done e.g. Source/WebCore/platform/cocoa/TelephoneNumberDetectorCocoa.cpp ; maybe this can be addressed in bug 164684.

[Radar WebKit Bug Importer]

<rdar://problem/35568927>