Use new CFNetwork cookie jar SPI only on El Capitan <rdar://problem/22460752>
Created attachment 260156 [details] Patch v1
Comment on attachment 260156 [details] Patch v1 r- This won't build on El Capitan without internal headers. You need a change like this to CFNetworkSPI.h: diff --git a/Source/WebCore/platform/spi/cf/CFNetworkSPI.h b/Source/WebCore/platform/spi/cf/CFNetworkSPI.h index ac8f087..34621e1 100644 --- a/Source/WebCore/platform/spi/cf/CFNetworkSPI.h +++ b/Source/WebCore/platform/spi/cf/CFNetworkSPI.h @@ -144,6 +144,14 @@ EXTERN_C CFArrayRef _CFHTTPParsedCookiesWithResponseHeaderFields(CFAllocatorRef + (void)_setSharedHTTPCookieStorage:(NSHTTPCookieStorage *)storage; @end #endif + +#if !USE(APPLE_INTERNAL_SDK) && ((TARGET_OS_IPHONE && __IPHONE_OS_VERSION_MIN_REQUIRED >= 90000) || (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100)) +@interface NSHTTPCookie () ++ (NSArray *)_parsedCookiesWithResponseHeaderFields:(NSDictionary *)headerFields forURL:(NSURL *)aURL; +@end #endif +#endif + + #endif // CFNetworkSPI_h
Comment on attachment 260156 [details] Patch v1 View in context: https://bugs.webkit.org/attachment.cgi?id=260156&action=review > Source/WebCore/platform/network/mac/CookieJarMac.mm:123 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 Also, this needs to include a PLATFORM(MAC) check to fix the build failure on iOS: /Volumes/Data/EWS/WebKit/Source/WebCore/platform/network/mac/CookieJarMac.mm:123:5: error: '__MAC_OS_X_VERSION_MIN_REQUIRED' is not defined, evaluates to 0 [-Werror,-Wundef] #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 ^ 1 error generated. #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100
Created attachment 260157 [details] Patch v2
(In reply to comment #2) > Comment on attachment 260156 [details] > Patch v1 > > r- > > This won't build on El Capitan without internal headers. You need a change > like this to CFNetworkSPI.h: > Yup, we already went over this in the radar. Needs the forward declare, but this one isn't quite right.
(In reply to comment #3) > Comment on attachment 260156 [details] > Patch v1 > > View in context: > https://bugs.webkit.org/attachment.cgi?id=260156&action=review > > > Source/WebCore/platform/network/mac/CookieJarMac.mm:123 > > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 > > Also, this needs to include a PLATFORM(MAC) check to fix the build failure > on iOS: > > /Volumes/Data/EWS/WebKit/Source/WebCore/platform/network/mac/CookieJarMac.mm: > 123:5: error: '__MAC_OS_X_VERSION_MIN_REQUIRED' is not defined, evaluates to > 0 [-Werror,-Wundef] > #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 > ^ > 1 error generated. > > #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 This is sad and always catches me by surprise - iOS uses both cookiejarmac and cookiejarcf :(
(In reply to comment #6) > (In reply to comment #3) > > Comment on attachment 260156 [details] > > Patch v1 > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=260156&action=review > > > > > Source/WebCore/platform/network/mac/CookieJarMac.mm:123 > > > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 > > > > Also, this needs to include a PLATFORM(MAC) check to fix the build failure > > on iOS: > > > > /Volumes/Data/EWS/WebKit/Source/WebCore/platform/network/mac/CookieJarMac.mm: > > 123:5: error: '__MAC_OS_X_VERSION_MIN_REQUIRED' is not defined, evaluates to > > 0 [-Werror,-Wundef] > > #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 > > ^ > > 1 error generated. > > > > #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 > > This is sad and always catches me by surprise - iOS uses both cookiejarmac > and cookiejarcf :( Actually, this is SUPREMELY surprising. How is this possible? iOS gets this exact same symbol twice - Once from CookieJarMac and once from CookieJarCFNet, yet it uses the CFNet version o_O
Created attachment 260158 [details] Patch v3 This adds the PLATFORM(MAC), but it's weird that it's needed. We need an answer to why that builds on iOS, and how it's possible that iOS gets the same symbol twice and chooses the right one. =/
Comment on attachment 260157 [details] Patch v2 View in context: https://bugs.webkit.org/attachment.cgi?id=260157&action=review r=me with the macro fixes. > Source/WebCore/platform/network/mac/CookieJarMac.mm:123 > +#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 This still needs to be changed to prevent an iOS build failure: #if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 Apparently this file is compiled on iOS but not used. > Source/WebCore/platform/spi/cf/CFNetworkSPI.h:132 > +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 This should really be: #if !USE(APPLE_INTERNAL_SDK) || PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100
Comment on attachment 260158 [details] Patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=260158&action=review r=me but consider the macro change. > Source/WebCore/platform/spi/cf/CFNetworkSPI.h:132 > +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 This should probably be: #if !USE(APPLE_INTERNAL_SDK) || PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100
(In reply to comment #8) > Created attachment 260158 [details] > Patch v3 > > This adds the PLATFORM(MAC), but it's weird that it's needed. > > We need an answer to why that builds on iOS, and how it's possible that iOS > gets the same symbol twice and chooses the right one. =/ I know why. It's because the EWS builder has the NSURLConnection path defined (so WTF_USE_CFNETWORK set to 0), while builds against an internal iOS SDK have WTF_USE_CFNETOWRK set to 1. (This needs to be fixed; I'll explain the details in radar.) Otherwise there would be linker errors with duplicate symbols.
Comment on attachment 260158 [details] Patch v3 View in context: https://bugs.webkit.org/attachment.cgi?id=260158&action=review r=me >> Source/WebCore/platform/spi/cf/CFNetworkSPI.h:132 >> +#if PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 > > This should probably be: > > #if !USE(APPLE_INTERNAL_SDK) || PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101100 Comment withdrawn after taking to Brady in person. The !USE(APPLE_INTERNAL_SDK) doesn't matter here.
Since EWS ran and since I was actually back at my computer, landed myself instead of waiting on the CQ. https://trac.webkit.org/changeset/189101
Why no tests?