[webkit-reviews] review denied: [Bug 136487] [iOS] Make WebCore build with public iOS SDK : [Attachment 237624] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 4 18:20:44 PDT 2014 

Andy Estes <aestes at apple.com> has denied Daniel Bates <dbates at webkit.org>'s
request for review:
Bug 136487: [iOS] Make WebCore build with public iOS SDK
https://bugs.webkit.org/show_bug.cgi?id=136487

Attachment 237624: Patch
https://bugs.webkit.org/attachment.cgi?id=237624&action=review

------- Additional Comments from Andy Estes <aestes at apple.com>
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.

More information about the webkit-reviews mailing list