[webkit-reviews] review granted: [Bug 212000] http/tests/ssl/applepay/ApplePayInstallmentConfiguration.https.html fails in public SDK builds : [Attachment 399617] Patch

[bugzilla-daemon at webkit.org]

youenn fablet <youennf at gmail.com> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 212000: http/tests/ssl/applepay/ApplePayInstallmentConfiguration.https.html
fails in public SDK builds
https://bugs.webkit.org/show_bug.cgi?id=212000

Attachment 399617: Patch

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




--- Comment #7 from youenn fablet <youennf at gmail.com> ---
Comment on attachment 399617
  --> https://bugs.webkit.org/attachment.cgi?id=399617
Patch

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

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:-67
> -

There is probably still one instance of ENABLE_APPLE_PAY_SESSION_V9 in
Features.xcconfig.
Is that expected?

> Source/WTF/wtf/PlatformEnableCocoa.h:58
> +#endif

So ENABLE(APPLE_PAY_SESSION_V9) is no longer defined which means
PaymentCoordinatorClient::supportsVersion will always return version 8.
Should we have a case with ENABLE_APPLE_PAY_SESSION_V9 equal to 1?

> Source/WebCore/PAL/pal/spi/cocoa/PassKitSPI.h:362
> +#if HAVE(PASSKIT_INSTALLMENTS)

This one is not needed given the previous HAVE(PASSKIT_INSTALLMENTS).

> Source/WebCore/PAL/pal/spi/cocoa/PassKitSPI.h:368
> +#if HAVE(PASSKIT_INSTALLMENTS)

Ditto.

> Source/WebKit/Shared/WebCoreArgumentCoders.h:883
> +#endif

We usually try to have these defined in the class header itself.
Looking at PaymentInstallmentConfigurationWebCore.h, it is using
HAVE(PASSKIT_INSTALLMENTS).
I guess it could be transitioned to ENABLE(APPLE_PAY_INSTALLMENTS) as well.