[webkit-reviews] review granted: [Bug 187024] Enable WebKit iOS 12 build : [Attachment 343626] Patch

[bugzilla-daemon at webkit.org]

David Kilzer (:ddkilzer) <ddkilzer at webkit.org> has granted Jonathan Bedard
<jbedard at apple.com>'s request for review:
Bug 187024: Enable WebKit iOS 12 build
https://bugs.webkit.org/show_bug.cgi?id=187024

Attachment 343626: Patch

https://bugs.webkit.org/attachment.cgi?id=343626&action=review




--- Comment #10 from David Kilzer (:ddkilzer) <ddkilzer at webkit.org> ---
Comment on attachment 343626
  --> https://bugs.webkit.org/attachment.cgi?id=343626
Patch

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

r=me

> Source/WTF/wtf/spi/darwin/XPCSPI.h:107
>  extern const struct _xpc_type_s _xpc_type_dictionary;
>  extern const struct _xpc_type_s _xpc_type_error;
>  extern const struct _xpc_type_s _xpc_type_string;
> +extern const struct _xpc_type_s _xpc_type_connection;
> +extern const struct _xpc_type_s _xpc_type_endpoint;

Nit: Alphabetize.

> Source/WTF/wtf/spi/darwin/XPCSPI.h:139
>  void xpc_dictionary_set_fd(xpc_object_t, const char* key, int fd);
>  void xpc_dictionary_set_string(xpc_object_t, const char* key, const char*
string);
>  void xpc_dictionary_set_uint64(xpc_object_t, const char* key, uint64_t
value);
> -void xpc_dictionary_set_value(xpc_object_t, const char*key, xpc_object_t
value);
> +void xpc_dictionary_set_value(xpc_object_t, const char *key, xpc_object_t
value);

Nit: Please put the space to the right of the '*'.

> Source/WTF/wtf/spi/darwin/XPCSPI.h:164
> +void xpc_array_append_value(xpc_object_t xarray, xpc_object_t value);
> +xpc_object_t xpc_data_create(const void* bytes, size_t length);
> +xpc_object_t xpc_dictionary_get_array(xpc_object_t xdict, const char* key);
> +const void * xpc_data_get_bytes_ptr(xpc_object_t xdata);
> +size_t xpc_data_get_length(xpc_object_t xdata);
> +xpc_object_t xpc_array_get_value(xpc_object_t xarray, size_t index);

NitL Alphabetize.

> Source/WebKit/Platform/spi/ios/PDFKitSPI.h:53
> ++ (void) createHostView: (void(^)( PDFHostViewController* hostViewController
)) callback forExtensionIdentifier: (NSString*) extensionIdentifier;
> +- (void) setDelegate: (id<PDFHostViewControllerDelegate>) delegate;
> +- (void) setDocumentData: (NSData*) data withScrollView: (UIScrollView*)
scrollView;
> +
> +- (void) findString: (NSString*) string withOptions:
(NSStringCompareOptions) options;
> +- (void) cancelFindString;
> +- (void) focusOnSearchResultAtIndex: (NSUInteger) searchIndex;
> +
> +- (NSInteger) currentPageIndex;
> +- (NSInteger) pageCount;
> +- (UIView*) pageNumberIndicator;
> +- (void) goToPageIndex: (NSInteger) pageIndex;

Nit:  No spaces between the colon and the left parenthesis please:

- (void) goToPageIndex:(NSInteger) pageIndex;

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:976
> +typedef NS_ENUM(NSInteger, UICompositingMode) {
> +    UICompositingModeNormal,
> +    UICompositingModeMultiply,
> +    UICompositingModeScreen,
> +    UICompositingModeOverlay,
> +    UICompositingModeDarken,
> +    UICompositingModeLighten,
> +    UICompositingModeColorDodge,
> +    UICompositingModeColorBurn,
> +    UICompositingModeSoftLight,
> +    UICompositingModeHardLight,
> +    UICompositingModeDifference,
> +    UICompositingModeExclusion,
> +    UICompositingModeClear,
> +    UICompositingModeCopy,
> +    UICompositingModeSourceIn,
> +    UICompositingModeSourceOut,
> +    UICompositingModeSourceAtop,
> +    UICompositingModeDestination,
> +    UICompositingModeDestinationOver,
> +    UICompositingModeDestinationIn,
> +    UICompositingModeDestinationOut,
> +    UICompositingModeDestinationAtop,
> +    UICompositingModeXOR,
> +    UICompositingModePlusDarker,
> +    UICompositingModePlusLighter,
> +};

Do we need all of these?  I guess we need placeholders if we don't declare them
all.

>
WebKitLibraries/WebKitPrivateFrameworkStubs/iOS/12/AppSupport.framework/AppSupp
ort.tbd:4
> +  - armv7
> +  - armv7s

Are you sure we need armv7 and armv7s anymore?	Do we need armv7k for watchOS?

>
WebKitLibraries/WebKitPrivateFrameworkStubs/iOS/12/AppSupport.framework/AppSupp
ort.tbd:12
> +	 - armv7
> +	 - armv7s

Ditto.