Bug 136863 – [iOS] Make WebKit build with public iOS SDK

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Daniel Bates]

We should make WebKit build with the public iOS SDK.

[Daniel Bates]

Created attachment 238206 [details]
Patch

[Daniel Bates]

Created attachment 238218 [details]
Patch

Rebased patch

[Daniel Bates]

Created attachment 238261 [details]
Patch

Rebased patch

[WebKit Commit Bot]

Attachment 238261 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameNetworkingContext.mm:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Daniel Bates]

Created attachment 238262 [details]
Patch

Update NSCalenderDateSPI.h to conditionally include <Foundation/NSCalendarDate.h> when building for Mac as it is a public API header.

[WebKit Commit Bot]

Attachment 238262 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameNetworkingContext.mm:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 22 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Daniel Bates]

Created attachment 241844 [details]
Patch

[WebKit Commit Bot]

Attachment 241844 [details] did not pass style-queue:


ERROR: Source/WebKit/mac/WebCoreSupport/WebFrameNetworkingContext.mm:26:  Found header this file implements before WebCore config.h. Should be: config.h, primary header, blank line, and then alphabetically sorted.  [build/include_order] [4]
Total errors found: 1 in 24 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[David Kilzer (:ddkilzer)]

Comment on attachment 241844 [details]
Patch

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

r=me

> Source/WebCore/platform/spi/cf/CFNetworkSPI.h:50
> +#ifdef __OBJC__
> +// FIXME: As a workaround for <rdar://problem/18337182>, we conditionally enclose the header
> +// in an extern "C" linkage block to make it suitable for C++ use.
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#import <CFNetwork/CFNSURLConnection.h>
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +#endif // __OBJC__

Is the #ifdef __OBJC__ check really needed here?  Aren't all the CF* APIs work in plain C?

Or does this header include Objective-C headers without their own guards?

> Source/WebKit/mac/Misc/WebDownload.h:34
> +#if __has_include(<Foundation/NSURLDownload.h>)
>  #import <Foundation/NSURLDownload.h>
> +#else
> +@interface NSURLDownload : NSObject
> +@end
> +#endif

At minimum, it seems like we should add a comment here about why this is defined inline instead of an *SPI.h header so someone doesn't try to "clean it up" later.

Is it worth filing a radar either (1) to get internal clients off this WebKit SPI header (#irony), or (2) to make this API for all clients?

[Daniel Bates]

(In reply to comment #9)
> [...]
> > Source/WebCore/platform/spi/cf/CFNetworkSPI.h:50
> > +#ifdef __OBJC__
> > +// FIXME: As a workaround for <rdar://problem/18337182>, we conditionally enclose the header
> > +// in an extern "C" linkage block to make it suitable for C++ use.
> > +#ifdef __cplusplus
> > +extern "C" {
> > +#endif
> > +
> > +#import <CFNetwork/CFNSURLConnection.h>
> > +
> > +#ifdef __cplusplus
> > +}
> > +#endif
> > +#endif // __OBJC__
> 
> Is the #ifdef __OBJC__ check really needed here?  Aren't all the CF* APIs
> work in plain C?
> 

Yes, the #ifdef __OBJC__ is needed as a workaround for <rdar://problem/19033610>. I'll add the following comment above the #ifdef __OBJC__ line before landing:

FIXME: Remove the #ifdef __OBJC__-guard onnce we fix <rdar://problem/19033610>.


> Or does this header include Objective-C headers without their own guards?
> 

Yes, this header includes Objective-C headers without a __OBJC__ guard. See <rdar://problem/19033610> for more details.

> > Source/WebKit/mac/Misc/WebDownload.h:34
> > +#if __has_include(<Foundation/NSURLDownload.h>)
> >  #import <Foundation/NSURLDownload.h>
> > +#else
> > +@interface NSURLDownload : NSObject
> > +@end
> > +#endif
> 
> At minimum, it seems like we should add a comment here about why this is
> defined inline instead of an *SPI.h header so someone doesn't try to "clean
> it up" later.
> 

Will add a comment similar to the comment in WAKAppKitStubs.h(*):

We explicitly use __has_include() instead of the macro define WTF_USE_APPLE_INTERNAL_SDK as the condition for including the header Foundation/NSURLDownload.h to support internal Apple clients that build without header wtf/Platform.h.

(*) <http://trac.webkit.org/browser/trunk/Source/WebCore/platform/ios/wak/WAKAppKitStubs.h?rev=176105#L57>

> Is it worth filing a radar either (1) to get internal clients off this
> WebKit SPI header (#irony), or (2) to make this API for all clients?

I felt such a discussion is worth to have and filed <rdar://problem/19034131>.

[Daniel Bates]

Committed r176347: <http://trac.webkit.org/changeset/176347>