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

• Status: RESOLVED
• Resolution: FIXED
• Component: Platform

[Daniel Bates]

Towards building the iOS WebKit port using the the public iOS SDK, we need to build WebCore for iOS using the public iOS SDK.

[Daniel Bates]

Created attachment 237563 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:33:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:34:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  __CG_DEPRECATED_ENUMERATOR is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cui/CUICatalogSPI.h:34:  Extra space before [  [whitespace/braces] [5]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 23 in 80 files


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

[Daniel Bates]

Created attachment 237570 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:33:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:34:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  __CG_DEPRECATED_ENUMERATOR is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cui/CUICatalogSPI.h:34:  Extra space before [  [whitespace/braces] [5]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 23 in 81 files


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

[Daniel Bates]

Created attachment 237579 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:33:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:34:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  __CG_DEPRECATED_ENUMERATOR is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cui/CUICatalogSPI.h:34:  Extra space before [  [whitespace/braces] [5]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 23 in 82 files


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

[Daniel Bates]

Created attachment 237624 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:33:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:34:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/dispatch/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ct/CTFontSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cui/CUICatalogSPI.h:34:  Extra space before [  [whitespace/braces] [5]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGContextSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  __CG_DEPRECATED_ENUMERATOR is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 23 in 84 files


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

[Andy Estes]

Comment on attachment 237624 [details]
Patch

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

I think this goes a little overboard with the directory structure under platform/spi/, so I'd like to see another pass at this.

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:5758
> +		CE4F6D2D19B774D500BFC64A /* AVPlayerControllerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D2919B774D500BFC64A /* AVPlayerControllerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D2E19B774D500BFC64A /* AVPlayerViewControllerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D2A19B774D500BFC64A /* AVPlayerViewControllerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D2F19B774D500BFC64A /* AVValueTimingSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D2B19B774D500BFC64A /* AVValueTimingSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D3019B774D500BFC64A /* AVVideoLayerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D2C19B774D500BFC64A /* AVVideoLayerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D3419B774F600BFC64A /* CALayerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3219B774F600BFC64A /* CALayerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D3519B774F600BFC64A /* CATiledLayerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3319B774F600BFC64A /* CATiledLayerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4119B7751200BFC64A /* CGColorTransformSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3719B7751200BFC64A /* CGColorTransformSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4219B7751200BFC64A /* CGContextGStateSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3819B7751200BFC64A /* CGContextGStateSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4319B7751200BFC64A /* CGContextSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3919B7751200BFC64A /* CGContextSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4419B7751200BFC64A /* CGFloatSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3A19B7751200BFC64A /* CGFloatSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4519B7751200BFC64A /* CGFontGlyphSupportSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3B19B7751200BFC64A /* CGFontGlyphSupportSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4619B7751200BFC64A /* CGFontInfoSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3C19B7751200BFC64A /* CGFontInfoSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4719B7751200BFC64A /* CGFontRenderingSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3D19B7751200BFC64A /* CGFontRenderingSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4819B7751200BFC64A /* CGFontUnicodeSupportSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3E19B7751200BFC64A /* CGFontUnicodeSupportSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4919B7751200BFC64A /* CGImageSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D3F19B7751200BFC64A /* CGImageSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D4A19B7751200BFC64A /* CGSRegionSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D4019B7751200BFC64A /* CGSRegionSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5719B7757B00BFC64A /* CTFontDescriptorSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D4C19B7757B00BFC64A /* CTFontDescriptorSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5819B7757B00BFC64A /* CTFontSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D4D19B7757B00BFC64A /* CTFontSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5919B7757B00BFC64A /* CUICatalogSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D4F19B7757B00BFC64A /* CUICatalogSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5A19B7757B00BFC64A /* CUIStyleEffectConfigurationSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D5019B7757B00BFC64A /* CUIStyleEffectConfigurationSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5B19B7757B00BFC64A /* MobileGestaltSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D5219B7757B00BFC64A /* MobileGestaltSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5C19B7757B00BFC64A /* MPAVRoutingControllerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D5319B7757B00BFC64A /* MPAVRoutingControllerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5D19B7757B00BFC64A /* QLPreviewConverterSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D5519B7757B00BFC64A /* QLPreviewConverterSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D5E19B7757B00BFC64A /* QuickLookSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D5619B7757B00BFC64A /* QuickLookSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D6419B775F800BFC64A /* DispatchSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D5F19B775F800BFC64A /* DispatchSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D6519B775F800BFC64A /* dyldSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D6019B775F800BFC64A /* dyldSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D6619B775F800BFC64A /* NSFileManagerSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D6119B775F800BFC64A /* NSFileManagerSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D6719B775F800BFC64A /* NSGeometrySPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D6219B775F800BFC64A /* NSGeometrySPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D6819B775F800BFC64A /* NSPointerFunctionsSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D6319B775F800BFC64A /* NSPointerFunctionsSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };
> +		CE4F6D6A19B7854E00BFC64A /* IOPMLibSPI.h in Headers */ = {isa = PBXBuildFile; fileRef = CE4F6D6919B7854E00BFC64A /* IOPMLibSPI.h */; settings = {ATTRIBUTES = (Private, ); }; };

Does every single one of these new headers actually need to be Private? I didn't check every one of them, but at least one doesn't seem to be used outside of WebCore (CGFontUnicodeSupportSPI.h).

> Source/WebCore/WebCore.xcodeproj/project.pbxproj:16206
> +				CE4F6D2819B774D500BFC64A /* avkit */,
> +				CE4F6D3119B774F600BFC64A /* ca */,
>  				65086DA619AC1719009AF46B /* cf */,
> +				CE4F6D3619B7751200BFC64A /* cg */,
>  				653EF83719A043AE0052202C /* cocoa */,
> +				CE4F6D4B19B7757B00BFC64A /* ct */,
> +				CE4F6D4E19B7757B00BFC64A /* cui */,
> +				CE4F6D5119B7757B00BFC64A /* ios */,
> +				CE4F6D5419B7757B00BFC64A /* quicklook */,

These groups/directories are too fine-grained in my opinion, since the header names themselves already indicate the framework or library they come from. I would think simply having cocoa (for shared Mac and iOS SPI), mac, and ios directories would be sufficient.

> Source/WebCore/platform/spi/cg/CGColorTransformSPI.h:40
> +typedef const struct CGColorTransform *CGColorTransformRef;
> +extern "C" {
> +CGColorTransformRef CGColorTransformCreate(CGColorSpaceRef, CFDictionaryRef attributes);
> +CGColorRef CGColorTransformConvertColor(CGColorTransformRef, CGColorRef, CGColorRenderingIntent);
> +}

Are these declarations necessary if USE(APPLE_INTERNAL_SDK) is true? They seem identical to what's in the SPI header.

> Source/WebCore/platform/spi/cg/CGContextGStateSPI.h:41
> +extern "C" {
> +CGFloat CGContextGetLineWidth(CGContextRef);
> +void CGContextSetCTM(CGContextRef, CGAffineTransform);
> +CGColorRef CGContextGetFillColorAsColor(CGContextRef);
> +CGCompositeOperation CGContextGetCompositeOperation(CGContextRef);
> +void CGContextSetCompositeOperation(CGContextRef, CGCompositeOperation);
> +}

Ditto.

> Source/WebCore/platform/spi/cg/CGContextSPI.h:40
> +extern "C" CGAffineTransform CGContextGetBaseCTM(CGContextRef);

Ditto.

> Source/WebCore/platform/spi/cg/CGFontGlyphSupportSPI.h:37
> +extern "C" bool CGFontGetGlyphAdvancesForStyle(CGFontRef font, const CGAffineTransform* , CGFontRenderingStyle, const CGGlyph glyphs[], size_t count, CGSize advances[]);

Ditto.

> Source/WebCore/platform/spi/cg/CGFontInfoSPI.h:66
> +extern "C" {
> +const CGFontHMetrics* CGFontGetHMetrics(CGFontRef);
> +bool CGFontGetDescriptor(CGFontRef, CGFontDescriptor*);
> +const char* CGFontGetPostScriptName(CGFontRef);
> +CFStringRef CGFontCopyFamilyName(CGFontRef);
> +bool CGFontIsFixedPitch(CGFontRef);
> +}

Ditto.

> Source/WebCore/platform/spi/cg/CGFontUnicodeSupportSPI.h:35
> +extern "C" void CGFontGetGlyphsForUnichars(CGFontRef, const UniChar[], CGGlyph[], size_t count);

Ditto.

> Source/WebCore/platform/spi/cg/CGImageSPI.h:41
> +extern "C" void CGImageSetCachingFlags(CGImageRef, CGImageCachingFlags);

Ditto.

> Source/WebCore/platform/spi/cg/CGSRegionSPI.h:43
> +extern "C" {
> +CGSRegionEnumeratorObj CGSRegionEnumerator(CGRegionRef);
> +CGRect *CGSNextRect(const CGSRegionEnumeratorObj);
> +CGError CGSReleaseRegionEnumerator(const CGSRegionEnumeratorObj);
> +}

Ditto.

> Source/WebCore/platform/spi/cocoa/DispatchSPI.h:42
> +DISPATCH_EXPORT const struct dispatch_source_type_s _dispatch_source_type_memorystatus;

Ditto.

> Source/WebCore/platform/spi/cocoa/IOPMLibSPI.h:44
> +extern "C" {
> +IOReturn IOPMAssertionCreateWithDescription(CFStringRef assertionType, CFStringRef name, CFStringRef details,
> +    CFStringRef humanReadableReason, CFStringRef localizationBundlePath, CFTimeInterval timeout, CFStringRef timeoutAction, IOPMAssertionID *);
> +IOReturn IOPMAssertionRelease(IOPMAssertionID);
> +}

Ditto.

> Source/WebCore/platform/spi/cocoa/dyldSPI.h:39
> +extern "C" uint32_t dyld_get_program_sdk_version();

Ditto.

> Source/WebCore/platform/spi/ct/CTFontDescriptorSPI.h:57
> +extern "C" {
> +CTFontDescriptorRef CTFontDescriptorCreateWithTextStyle(CFStringRef style, CFStringRef size, CFStringRef language);
> +CTFontDescriptorRef CTFontDescriptorCreateForUIType(CTFontUIFontType, CGFloat size, CFStringRef language);
> +bool CTFontDescriptorIsSystemUIFont(CTFontDescriptorRef);
> +}

Ditto.

> Source/WebCore/platform/spi/ct/CTFontSPI.h:47
> +extern "C" {
> +CTFontRef CTFontCreatePhysicalFontForCharactersWithLanguage(CTFontRef, const UTF16Char* characters, CFIndex length, CFStringRef language, CFIndex* coveredLength);
> +bool CTFontIsAppleColorEmoji(CTFontRef);
> +bool CTFontDescriptorIsSystemUIFont(CTFontDescriptorRef);
> +CTFontRef CTFontCreateForCSS(CFStringRef name, uint16_t weight, CTFontSymbolicTraits, CGFloat size);
> +}

Ditto.

> Source/WebCore/platform/spi/dispatch/DispatchSPI.h:42
> +DISPATCH_EXPORT const struct dispatch_source_type_s _dispatch_source_type_memorystatus;

Ditto.

> Source/WebCore/platform/spi/ios/MobileGestaltSPI.h:37
> +extern "C" CFTypeRef MGCopyAnswer(CFStringRef question, CFDictionaryRef options);

Ditto.

[Andy Estes]

(In reply to comment #9)
> (From update of attachment 237624 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=237624&action=review
> 
> > Source/WebCore/WebCore.xcodeproj/project.pbxproj:16206
> > +				CE4F6D2819B774D500BFC64A /* avkit */,
> > +				CE4F6D3119B774F600BFC64A /* ca */,
> >  				65086DA619AC1719009AF46B /* cf */,
> > +				CE4F6D3619B7751200BFC64A /* cg */,
> >  				653EF83719A043AE0052202C /* cocoa */,
> > +				CE4F6D4B19B7757B00BFC64A /* ct */,
> > +				CE4F6D4E19B7757B00BFC64A /* cui */,
> > +				CE4F6D5119B7757B00BFC64A /* ios */,
> > +				CE4F6D5419B7757B00BFC64A /* quicklook */,
> 
> These groups/directories are too fine-grained in my opinion, since the header names themselves already indicate the framework or library they come from. I would think simply having cocoa (for shared Mac and iOS SPI), mac, and ios directories would be sufficient.

Actually, I guess it's more nuanced than this. For instance, having a separate cf/ directory makes sense since that's used on Windows. So I'm not against having separate directories for frameworks that exist outside of Mac/iOS. I was primarily objecting to having things like CoreUI and QuickLook in their own directories.

[Daniel Bates]

Created attachment 238066 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/cocoa/dyldSPI.h:43:  dyld_get_program_sdk_version is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:33:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:34:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:39:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:40:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:41:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:46:  _dispatch_source_type_memorystatus is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cocoa/MachVMSPI.h:41:  mach_vm_allocate is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cocoa/MachVMSPI.h:42:  mach_vm_deallocate is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cocoa/MachVMSPI.h:43:  mach_vm_map is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cocoa/MachVMSPI.h:45:  mach_vm_protect is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cocoa/MachVMSPI.h:46:  mach_vm_region is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 23 in 94 files


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

[Maciej Stachowiak]

Comment on attachment 238066 [details]
Patch

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

The only essential issue is explaining the IntRect.h change since it is not obviously related. Plus there are many optional comments. r- for now but looking good.

> Source/WebCore/platform/graphics/IntRect.h:45
> +typedef CGRect NSRect;

This change does not appear to be explained in the ChangeLog.

> Source/WebCore/platform/ios/wak/WAKAppKitStubs.h:37
> -#import <Foundation/NSGeometry.h>
> +#import <WebCore/NSGeometrySPI.h>

Why this change? NSGeometry is public API, at least on Mac. Is that not the case on iOS?

> Source/WebCore/platform/spi/ca/CALayerSPI.h:30
> +#import <CoreGraphics/CoreGraphics.h>
> +#import <QuartzCore/QuartzCore.h>

Does CALayerPrivate.h include these headers? I don't think it does, so it might be better to include them unconditionally, for consistency in what this header pulls in.

> Source/WebCore/platform/spi/ca/CATiledLayerSPI.h:32
> +#import <CoreGraphics/CoreGraphics.h>
> +#import <QuartzCore/QuartzCore.h>

Does CALayerPrivate.h include these headers? I don't think it does, so it might be better to include them unconditionally, for consistency in what this header pulls in.

> Source/WebCore/platform/spi/cg/CGColorTransformSPI.h:40
> +#ifdef __cplusplus
> +extern "C" {
> +#endif

Maybe it would be worthwhile making an EXTERN_C macro in a WTF header (Compiler.h maybe?) that expands to extern "C" for C++, and nothing or just extern for C, so you could avoid these repeated ifdef blocks. This comment is optional, it applies to a bunch of files but I won't mention it every time.

> Source/WebCore/platform/spi/cg/CGColorTransformSPI.h:43
> +extern CGColorRef CGColorTransformConvertColor(CGColorTransformRef, CGColorRef, CGColorRenderingIntent);
> +extern CGColorTransformRef CGColorTransformCreate(CGColorSpaceRef, CFDictionaryRef attributes);

I'm not sure the "extern" specifiers here do anything. Again, this might apply to a bunch of files.

> Source/WebCore/platform/spi/cg/CGContextGStateSPI.h:27
> +#ifndef CGContextGStateSPI_h
> +#define CGContextGStateSPI_h

I know CGContext SPI calls in reality are split among multiple files, but maybe we should put them all in one WebCore SPI header, since the split is not super meaningful.

> Source/WebCore/platform/spi/cg/CGFloatSPI.h:38
> +    if (sizeof(value) == sizeof(float))

Seems like it would be better to make this an explicit #if on CGFLOAT_IS_DOUBLE. I know the compiler will optimize out the dead path but it seems lame to rely on that, and with the #if you would not need any of the casts. Heck you could just #define CGRound to be round when CG_FLOAT_IS_DOUBLE and roundf otherwise.

Similar comments apply to almost all the other functions in this file.

> Source/WebCore/platform/spi/cg/CGFloatSPI.h:45
> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L

I strongly suspect all compilers we care about support C99, so I am not sure the conditionality is needed.

> Source/WebCore/platform/spi/cocoa/MachVMSPI.h:33
> +#if (TARGET_OS_MAC && !TARGET_OS_IPHONE) || (defined(WTF_USE_APPLE_INTERNAL_SDK) && WTF_USE_APPLE_INTERNAL_SDK)

Why write it out this way instead of if PLATFORM(MAC) || USE(APPLE_INTERNAL_SDK), as in the previous file?

> Source/WebCore/platform/spi/cocoa/NSFileManagerSPI.h:29
> +#import <Foundation/Foundation.h>

Probably should include Foundation.h unconditionally.

> Source/WebCore/platform/spi/ios/AVPlayerControllerSPI.h:31
> +#import <AVKit/AVKit.h>
> +#import <Foundation/Foundation.h>
> +#import <UIKit/UIKit.h>

These should probably be imported unconditionally.

> Source/WebCore/platform/spi/ios/AVPlayerViewControllerSPI.h:33
> +#import <AVKit/AVKit.h>
> +#import <Foundation/Foundation.h>
> +#import <QuartzCore/QuartzCore.h>
> +#import <UIKit/UIKit.h>

Should probably import these unconditionally.

> Source/WebCore/platform/spi/ios/AVValueTimingSPI.h:29
> +#import <Foundation/Foundation.h>

Again.

> Source/WebCore/platform/spi/ios/AVVideoLayerSPI.h:31
> +#import <AVKit/AVKit.h>
> +#import <CoreGraphics/CoreGraphics.h>
> +#import <Foundation/Foundation.h>

And here.

> Source/WebCore/platform/spi/ios/CTFontDescriptorSPI.h:54
> +extern const CFStringRef kCTUIFontTextStyleShortHeadline CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleShortBody CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleShortSubhead CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleShortFootnote CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleShortCaption1 CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleTallBody CT_AVAILABLE(10_10, 7_0);
> +
> +extern const CFStringRef kCTUIFontTextStyleHeadline CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleBody CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleSubhead CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleFootnote CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleCaption1 CT_AVAILABLE(10_10, 7_0);
> +extern const CFStringRef kCTUIFontTextStyleCaption2 CT_AVAILABLE(10_10, 7_0);
> +
> +extern const CFStringRef kCTFontDescriptorTextStyleEmphasized CT_AVAILABLE(10_10, 7_0);

I don't think you should include the CT_AVAILABLE tags here.

> Source/WebCore/platform/spi/ios/CUICatalogSPI.h:31
> +#import <CoreGraphics/CoreGraphics.h>
> +#import <CoreText/CoreText.h>
> +#import <Foundation/Foundation.h>

These includes should probably be unconditional.

> Source/WebCore/platform/spi/ios/QLPreviewConverterSPI.h:29
> +#import <Foundation/Foundation.h>

Probably should include Foundation.h unconditionally.

> Source/WebCore/platform/spi/ios/QuickLookSPI.h:29
> +#import <Foundation/Foundation.h>

Unconditional.

[Daniel Bates]

Comment on attachment 238066 [details]
Patch

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

>> Source/WebCore/platform/graphics/IntRect.h:45
>> +typedef CGRect NSRect;
> 
> This change does not appear to be explained in the ChangeLog.

I'll revert this change.

I made the change because I thought it was heavy handed to either use the C preprocessor to substitute CGRect for every occurrence of NSRect when a file includes IntRect.h or include NSGeometrySPI.h. The former seemed heavy handed compared to a typedef that better integrates with the compiler's type checking machinery. The latter seemed heavy handed given that we are only interested in a single definition from NSGeometrySPI.h and, obviously, a modification to NSGeometrySPI.h would cause IntRect.h and its dependents to be compiled again (although it's unlikely NSGeometrySPI.h would change much and there is value in centralizing the definition of NSRect; maybe we should do this?).

>> Source/WebCore/platform/ios/wak/WAKAppKitStubs.h:37
>> +#import <WebCore/NSGeometrySPI.h>
> 
> Why this change? NSGeometry is public API, at least on Mac. Is that not the case on iOS?

Yes, that is not the case on iOS. NSGeometry.h is a private API on iOS as of the time of writing.

>> Source/WebCore/platform/spi/ca/CALayerSPI.h:30
>> +#import <QuartzCore/QuartzCore.h>
> 
> Does CALayerPrivate.h include these headers? I don't think it does, so it might be better to include them unconditionally, for consistency in what this header pulls in.

No, CALayerPrivate.h doesn't include these headers. Moreover, it's sufficient to unconditionally include the public SDK header CALayer.h. Will update patch.

>> Source/WebCore/platform/spi/ca/CATiledLayerSPI.h:32
>> +#import <QuartzCore/QuartzCore.h>
> 
> Does CALayerPrivate.h include these headers? I don't think it does, so it might be better to include them unconditionally, for consistency in what this header pulls in.

No, CALayerPrivate.h doesn't include these headers. Moreover, it's sufficient to unconditionally include the header CATiledLayer.h instead of <CoreGraphics/CoreGraphics.h>, <QuartzCore/QuartzCore.h>, and <Foundation/Foundation.h>. Will update patch.

>> Source/WebCore/platform/spi/cg/CGColorTransformSPI.h:40
>> +#endif
> 
> Maybe it would be worthwhile making an EXTERN_C macro in a WTF header (Compiler.h maybe?) that expands to extern "C" for C++, and nothing or just extern for C, so you could avoid these repeated ifdef blocks. This comment is optional, it applies to a bunch of files but I won't mention it every time.

Will add macro EXTERN_C to Compiler.h and update these files to make use of it.

>> Source/WebCore/platform/spi/cg/CGColorTransformSPI.h:43
>> +extern CGColorTransformRef CGColorTransformCreate(CGColorSpaceRef, CFDictionaryRef attributes);
> 
> I'm not sure the "extern" specifiers here do anything. Again, this might apply to a bunch of files.

Using your suggestion above, I'll substitute EXTERN_C for extern in this file and the other files included in the patch.

>> Source/WebCore/platform/spi/cg/CGContextGStateSPI.h:27
>> +#define CGContextGStateSPI_h
> 
> I know CGContext SPI calls in reality are split among multiple files, but maybe we should put them all in one WebCore SPI header, since the split is not super meaningful.

Will coalesce the contents of files CGContextGStateSPI.h and CGContextSPI.h into a single file, Source/WebCore/platform/spi/cg/CGContextSPI.h.

>> Source/WebCore/platform/spi/cg/CGFloatSPI.h:38
>> +    if (sizeof(value) == sizeof(float))
> 
> Seems like it would be better to make this an explicit #if on CGFLOAT_IS_DOUBLE. I know the compiler will optimize out the dead path but it seems lame to rely on that, and with the #if you would not need any of the casts. Heck you could just #define CGRound to be round when CG_FLOAT_IS_DOUBLE and roundf otherwise.
> 
> Similar comments apply to almost all the other functions in this file.

I'll simplify the implementation of this function and the other functions in this file using #if CGFLOAT_IS_DOUBLE. While I could use the C preprocessor to substitute CGRound for the appropriate math.h function, it seems less error prone to define actual functions so that the compiler can type check its usage (maybe we shouldn't be concerned about incorrect usage that would be caught by the type checking machinery in the compiler?). The compiler should optimize away the function call (if applicable). Let me know if you prefer using the C preprocessor for these functions.

>> Source/WebCore/platform/spi/cg/CGFloatSPI.h:45
>> +#if defined(__STDC_VERSION__) && __STDC_VERSION__ >= 199901L
> 
> I strongly suspect all compilers we care about support C99, so I am not sure the conditionality is needed.

Will remove C99-guarded code.

>> Source/WebCore/platform/spi/cocoa/MachVMSPI.h:33
>> +#if (TARGET_OS_MAC && !TARGET_OS_IPHONE) || (defined(WTF_USE_APPLE_INTERNAL_SDK) && WTF_USE_APPLE_INTERNAL_SDK)
> 
> Why write it out this way instead of if PLATFORM(MAC) || USE(APPLE_INTERNAL_SDK), as in the previous file?

I don't recall my reason for writing this out. Will fix.

>> Source/WebCore/platform/spi/cocoa/NSFileManagerSPI.h:29
>> +#import <Foundation/Foundation.h>
> 
> Probably should include Foundation.h unconditionally.

It's sufficient to unconditionally include <Foundation/NSFileManager.h>. Will update patch.

>> Source/WebCore/platform/spi/ios/AVPlayerControllerSPI.h:31
>> +#import <UIKit/UIKit.h>
> 
> These should probably be imported unconditionally.

Will fix.

>> Source/WebCore/platform/spi/ios/AVPlayerViewControllerSPI.h:33
>> +#import <UIKit/UIKit.h>
> 
> Should probably import these unconditionally.

Will fix.

>> Source/WebCore/platform/spi/ios/AVValueTimingSPI.h:29
>> +#import <Foundation/Foundation.h>
> 
> Again.

Will fix.

>> Source/WebCore/platform/spi/ios/AVVideoLayerSPI.h:31
>> +#import <Foundation/Foundation.h>
> 
> And here.

Will fix.

>> Source/WebCore/platform/spi/ios/CTFontDescriptorSPI.h:54
>> +extern const CFStringRef kCTFontDescriptorTextStyleEmphasized CT_AVAILABLE(10_10, 7_0);
> 
> I don't think you should include the CT_AVAILABLE tags here.

Will remove.

>> Source/WebCore/platform/spi/ios/CUICatalogSPI.h:31
>> +#import <Foundation/Foundation.h>
> 
> These includes should probably be unconditional.

Will fix.

>> Source/WebCore/platform/spi/ios/QLPreviewConverterSPI.h:29
>> +#import <Foundation/Foundation.h>
> 
> Probably should include Foundation.h unconditionally.

Will fix.

>> Source/WebCore/platform/spi/ios/QuickLookSPI.h:29
>> +#import <Foundation/Foundation.h>
> 
> Unconditional.

Will fix.

[Daniel Bates]

Created attachment 238134 [details]
Patch

[WebKit Commit Bot]

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


ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:33:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:34:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:43:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGFontRenderingSPI.h:44:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:36:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:37:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/ios/CTFontSPI.h:38:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:48:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:49:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cocoa/DispatchSPI.h:50:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
ERROR: Source/WebCore/platform/spi/cg/CGImageSPI.h:35:  enum members should use InterCaps with an initial capital letter.  [readability/enum_casing] [4]
Total errors found: 16 in 97 files


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

[David Kilzer (:ddkilzer)]

Comment on attachment 238134 [details]
Patch

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

r=me with a couple comments

> Source/WebCore/platform/spi/ios/CUICatalogSPI.h:37
> +@interface CUICatalog : NSObject
> +
> +@end

NIt: Any reason to leave a blank line here?  Could be removed in other files as well.  Not necessary to fix to land the patch, though.

> Source/WebKit/mac/MigrateHeaders.make:245
> -WEBCORE_HEADER_REPLACE_RULES = -e s/\<WebCore/\<WebKitLegacy/ -e s/DOMDOMImplementation/DOMImplementation/ -e "s/(^ *)WEBCORE_EXPORT /\1/"
> -WEBCORE_HEADER_MIGRATE_CMD = sed -E $(WEBCORE_HEADER_REPLACE_RULES) $< > $@
> +WEBCORE_HEADER_REPLACE_RULES = 's/<WebCore/<WebKitLegacy/ if m|<WebCore/([^>]+)>| && $$1 !~ /SPI.h$$/; s/DOMDOMImplementation/DOMImplementation/; s/^(\s*)WEBCORE_EXPORT /$$1/;'
> +WEBCORE_HEADER_MIGRATE_CMD = perl -p -e $(WEBCORE_HEADER_REPLACE_RULES) $< > $@

Do you have to double-up the backslash in "(\s*)" to "(\\s*)"?

Did you do a recursive diff (diff -urN) between a DerivedSources directory with clean builds including this change and a clean build without this change to make sure there were no unexpected differences?  (Or something similar.)

[Daniel Bates]

(In reply to comment #17)
> (From update of attachment 238134 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=238134&action=review
> > Source/WebCore/platform/spi/ios/CUICatalogSPI.h:37
> > +@interface CUICatalog : NSObject
> > +
> > +@end
> 
> NIt: Any reason to leave a blank line here?  Could be removed in other files as well.  Not necessary to fix to land the patch, though.
> 

I'll remove the empty line.

We don't have a documented convention for the presence/omission of an empty line as the body of an empty @interface declaration as both styles are used in our code base. From briefly searching the code and disregarding the proposed SPI files, the majority of empty @interface declarations omit an empty line for the body. We should look to standardize a convention for writing an empty @interface declaration and update the WebKit Code Style Guidelines.

For completeness, one instance of declaring an empty @interface with a empty line for its body is in <http://trac.webkit.org/browser/trunk/Source/WebKit2/UIProcess/API/Cocoa/WKNavigation.h?rev=171626>. One instance of declaring an empty @interface without an empty line for the body is in <http://trac.webkit.org/browser/trunk/Source/WebKit2/WebProcess/InjectedBundle/API/Cocoa/WKWebProcessPlugInFrameInternal.h?rev=163629>.

> > Source/WebKit/mac/MigrateHeaders.make:245
> > -WEBCORE_HEADER_REPLACE_RULES = -e s/\<WebCore/\<WebKitLegacy/ -e s/DOMDOMImplementation/DOMImplementation/ -e "s/(^ *)WEBCORE_EXPORT /\1/"
> > -WEBCORE_HEADER_MIGRATE_CMD = sed -E $(WEBCORE_HEADER_REPLACE_RULES) $< > $@
> > +WEBCORE_HEADER_REPLACE_RULES = 's/<WebCore/<WebKitLegacy/ if m|<WebCore/([^>]+)>| && $$1 !~ /SPI.h$$/; s/DOMDOMImplementation/DOMImplementation/; s/^(\s*)WEBCORE_EXPORT /$$1/;'
> > +WEBCORE_HEADER_MIGRATE_CMD = perl -p -e $(WEBCORE_HEADER_REPLACE_RULES) $< > $@
> 
> Do you have to double-up the backslash in "(\s*)" to "(\\s*)"?
> 

From briefly reading <http://www.gnu.org/software/make/manual/make.html>, it isn't necessary to escape the backslash in "\s" with respect to how we are using it in MigrateHeaders.make. This behavior is consistent with my observation of the preservation of the backslash character in "\s" when running make on a Makefile with the following contents:

WEBCORE_HEADER_REPLACE_RULES = 's/<WebCore/<WebKitLegacy/ if m|<WebCore/([^>]+)>| && $$1 !~ /SPI.h$$/; s/DOMDOMImplementation/DOMImplementation/; s/^(\s*)WEBCORE_EXPORT /$$1/;'
all : ; @echo $(WEBCORE_HEADER_REPLACE_RULES)

The output was, as expected:

s/<WebCore/<WebKitLegacy/ if m|<WebCore/([^>]+)>| && $1 !~ /SPI.h$/; s/DOMDOMImplementation/DOMImplementation/; s/^(\s*)WEBCORE_EXPORT /$1/;

> Did you do a recursive diff (diff -urN) between a DerivedSources directory with clean builds including this change and a clean build without this change to make sure there were no unexpected differences?  (Or something similar.)

Notice that the script MigrateHeaders.make only affects the files in WebKitLegacy.framework/PrivateHeaders.

I ensured that there were no unexpected differences made to the files in WebKitLegacy.framework/PrivateHeaders between a build without the proposed patch and with the proposed patch.

[Daniel Bates]

Committed r173695: <http://trac.webkit.org/changeset/173695>

[Daniel Bates]

Reverted r173695 for reason:

Broke building third-party Legacy WebKit apps; will investigate offline

Committed r173710: <http://trac.webkit.org/changeset/173710>

[Daniel Bates]

I chose to separate out the addition of the SPI wrapper header NSGeometrySPI.h from the patch (attachment #238134 [details]) into its own bug (bug #137536) so as to make it straightforward to review its correctness.

[Daniel Bates]

Created attachment 241803 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 241803 [details]
Patch

r=me!

[Daniel Bates]

Comment on attachment 241803 [details]
Patch

Clearing flags on attachment: 241803

Committed r176278: <http://trac.webkit.org/changeset/176278>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.