Bug 212000 - http/tests/ssl/applepay/ApplePayInstallmentConfiguration.https.html fails in public SDK builds
Summary: http/tests/ssl/applepay/ApplePayInstallmentConfiguration.https.html fails in ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-17 12:29 PDT by Andy Estes
Modified: 2020-05-18 22:39 PDT (History)
16 users (show)

See Also:


Attachments
Patch (25.86 KB, patch)
2020-05-17 18:08 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (25.87 KB, patch)
2020-05-17 19:16 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (25.99 KB, patch)
2020-05-17 19:42 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch (25.89 KB, patch)
2020-05-17 19:44 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch for landing (65.35 KB, patch)
2020-05-18 10:37 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch for landing (65.34 KB, patch)
2020-05-18 11:59 PDT, Andy Estes
no flags Details | Formatted Diff | Diff
Patch for landing (1.68 KB, patch)
2020-05-18 22:35 PDT, Peng Liu
ews-feeder: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2020-05-17 12:29:18 PDT
https://build.webkit.org/results/Apple-Catalina-Release-WK2-Tests/r261793%20(5612)/results.html

>CONSOLE MESSAGE: InvalidAccessError: "8" is not a supported version.

This is because HAVE(PASSKIT_INSTALLMENTS) is 0 in public SDK builds.
Comment 1 Andy Estes 2020-05-17 12:43:08 PDT
Skipped test in r261795: <https://trac.webkit.org/changeset/261795>
Comment 2 Radar WebKit Bug Importer 2020-05-17 18:02:18 PDT
<rdar://problem/63323082>
Comment 3 Andy Estes 2020-05-17 18:08:26 PDT Comment hidden (obsolete)
Comment 4 Andy Estes 2020-05-17 19:16:36 PDT Comment hidden (obsolete)
Comment 5 Andy Estes 2020-05-17 19:42:10 PDT Comment hidden (obsolete)
Comment 6 Andy Estes 2020-05-17 19:44:48 PDT
Created attachment 399617 [details]
Patch
Comment 7 youenn fablet 2020-05-17 23:57:50 PDT
Comment on attachment 399617 [details]
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.
Comment 8 Andy Estes 2020-05-18 10:32:34 PDT
Comment on attachment 399617 [details]
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?

I’ll also remove $(ENABLE_APPLE_PAY_SESSION_V9) from all the FEATURE_DEFINES variables. I don’t see any other hits for ENABLE_APPLE_PAY_SESSION_V9 in .xcconfig files.

>> 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?

For now, ENABLE_APPLE_PAY_SESSION_V9 is being defined in AdditionalFeatureDefines.h.
Comment 9 Andy Estes 2020-05-18 10:37:29 PDT Comment hidden (obsolete)
Comment 10 Andy Estes 2020-05-18 11:59:16 PDT
Created attachment 399663 [details]
Patch for landing
Comment 11 EWS 2020-05-18 19:32:55 PDT
Committed r261845: <https://trac.webkit.org/changeset/261845>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399663 [details].
Comment 12 Peng Liu 2020-05-18 22:35:49 PDT Comment hidden (obsolete)
Comment 13 Peng Liu 2020-05-18 22:35:50 PDT Comment hidden (obsolete)
Comment 14 EWS 2020-05-18 22:36:35 PDT Comment hidden (obsolete)
Comment 15 Peng Liu 2020-05-18 22:37:57 PDT Comment hidden (obsolete)
Comment 16 Peng Liu 2020-05-18 22:39:27 PDT Comment hidden (obsolete)