Bug 173371 - Build WebKit with High Sierra (Seed 1)
Summary: Build WebKit with High Sierra (Seed 1)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: Other
Hardware: Mac macOS 10.13
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-06-14 10:26 PDT by Jonathan Bedard
Modified: 2017-06-15 13:18 PDT (History)
7 users (show)

See Also:


Attachments
Patch (7.48 KB, patch)
2017-06-14 11:49 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (7.45 KB, patch)
2017-06-14 13:21 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (5.79 KB, patch)
2017-06-14 15:57 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (5.92 KB, patch)
2017-06-15 08:44 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2017-06-14 10:26:22 PDT
WebKit should build with High Sierra.
Comment 1 Radar WebKit Bug Importer 2017-06-14 11:04:40 PDT
<rdar://problem/32770710>
Comment 2 Jonathan Bedard 2017-06-14 11:49:09 PDT
Created attachment 312910 [details]
Patch
Comment 3 Build Bot 2017-06-14 11:51:08 PDT
Attachment 312910 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/cocoa/PassKitSPI.h:54:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:62:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:63:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Andy Estes 2017-06-14 13:17:31 PDT
Comment on attachment 312910 [details]
Patch

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

> Source/WebKit2/ChangeLog:12
> +        (-[WKPaymentAuthorizationViewControllerDelegate paymentAuthorizationViewController:didAuthorizePayment:completion:]):
> +        The current paymentAuthorizationViewController: didAuthorizePayment:handler uses a handler which accepts a
> +        PKPaymentAuthorizationResult instead of a PKPaymentAuthorizationStatus, which is required by
> +        PKPaymentAuthorizationViewControllerDelegate.

This isn't right. The required method in PKPaymentAuthorizationViewControllerDelegate is the one that takes a PKPaymentAuthorizationResult. The one that takes a PKPaymentAuthorizationStatus is deprecated in High Sierra.
Comment 5 Jonathan Bedard 2017-06-14 13:21:01 PDT
Created attachment 312917 [details]
Patch
Comment 6 Andy Estes 2017-06-14 13:23:00 PDT
Comment on attachment 312910 [details]
Patch

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

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:259
> +- (_Nonnull id)initWithStatus:(PKPaymentAuthorizationStatus)status errors:(nullable NSArray<NSError *> *)errors;

The return type should be instancetype.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:264
> +- (_Nonnull id)initWithPaymentSummaryItems:(nonnull NSArray<PKPaymentSummaryItem *> *)paymentSummaryItems;

Ditto.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:268
> +- (_Nonnull id)initWithPaymentSummaryItems:(nonnull NSArray<PKPaymentSummaryItem *> *)summaryItems shippingMethods:(nonnull NSArray<PKShippingMethod *> *)shippingMethods;

Ditto.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:269
> +- (_Nonnull id)initWithErrors:(nullable NSArray<NSError *> *)errors paymentSummaryItems:(nonnull NSArray<PKPaymentSummaryItem *> *)summaryItems shippingMethods:(nonnull NSArray<PKShippingMethod *> *)shippingMethods;

Ditto.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:289
> +- (void) setRequiredShippingContactFields:(nonnull NSSet*) contactInformation;
> +- (void) setRequiredBillingContactFields:(nonnull NSSet*) contactInformation;

No space after the return type.
Comment 7 Build Bot 2017-06-14 13:25:37 PDT
Attachment 312917 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/cocoa/PassKitSPI.h:54:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:62:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:63:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 6 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 8 Jiewen Tan 2017-06-14 13:29:04 PDT
Comment on attachment 312917 [details]
Patch

Good for WebCrypto part.
Comment 9 Jonathan Bedard 2017-06-14 15:57:38 PDT
Created attachment 312924 [details]
Patch
Comment 10 Jonathan Bedard 2017-06-14 15:59:22 PDT
Attachment 312924 [details] should address Andy's comments in comment 4 and comment 5.
Comment 11 Build Bot 2017-06-14 16:00:18 PDT
Attachment 312924 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/cocoa/PassKitSPI.h:54:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:62:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:63:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 12 Andy Estes 2017-06-14 16:50:15 PDT
Comment on attachment 312924 [details]
Patch

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

> Source/WebCore/platform/spi/cocoa/AVKitSPI.h:241
> +- (id)initWithTitle:(nonnull NSString *)title type:(AVTouchBarMediaSelectionOptionType)type;

instancetype, not id

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:265
> +- (_Nonnull instancetype)initWithStatus:(PKPaymentAuthorizationStatus)status errors:(nullable NSArray<NSError *> *)errors;

I don't think the _Nonnull is right. In general, Objective-C initializers can return nil, and I don't see this annotation in the system header.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:270
> +- (_Nonnull instancetype)initWithPaymentSummaryItems:(nonnull NSArray<PKPaymentSummaryItem *> *)paymentSummaryItems;

Ditto.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:273
> +@interface PKPaymentRequestShippingContactUpdate

PKPaymentRequestShippingContactUpdate inherits from PKPaymentRequestUpdate.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:275
> +- (_Nonnull instancetype)initWithPaymentSummaryItems:(nonnull NSArray<PKPaymentSummaryItem *> *)summaryItems shippingMethods:(nonnull NSArray<PKShippingMethod *> *)shippingMethods;
> +- (_Nonnull instancetype)initWithErrors:(nullable NSArray<NSError *> *)errors paymentSummaryItems:(nonnull NSArray<PKPaymentSummaryItem *> *)summaryItems shippingMethods:(nonnull NSArray<PKShippingMethod *> *)shippingMethods;

Ditto about _Nonnull.

> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:295
> +- (void)setRequiredShippingContactFields:(nonnull NSSet*) contactInformation;
> +- (void)setRequiredBillingContactFields:(nonnull NSSet*) contactInformation;

No space between the argument type and name.
Comment 13 Andy Estes 2017-06-14 16:56:11 PDT
Comment on attachment 312924 [details]
Patch

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

>> Source/WebCore/platform/spi/cocoa/PassKitSPI.h:295
>> +- (void)setRequiredBillingContactFields:(nonnull NSSet*) contactInformation;
> 
> No space between the argument type and name.

Also, yes space between NSSet and *.
Comment 14 Jonathan Bedard 2017-06-15 08:44:17 PDT
Created attachment 312985 [details]
Patch
Comment 15 Build Bot 2017-06-15 08:47:12 PDT
Attachment 312985 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/spi/cocoa/PassKitSPI.h:55:  Place brace on its own line for function definitions.  [whitespace/braces] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:62:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/crypto/CommonCryptoUtilities.h:63:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 WebKit Commit Bot 2017-06-15 09:57:20 PDT
Comment on attachment 312985 [details]
Patch

Clearing flags on attachment: 312985

Committed r218336: <http://trac.webkit.org/changeset/218336>
Comment 17 WebKit Commit Bot 2017-06-15 09:57:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 18 Jonathan Bedard 2017-06-15 13:18:36 PDT
Follow-up fix in <https://trac.webkit.org/changeset/218352/webkit>.