supportedCountries only works on ApplePaySession, not PaymentRequest …

…(affects Shopify) https://bugs.webkit.org/show_bug.cgi?id=259940 rdar://113557607 Reviewed by Alex Christensen. The `supportedCountries` property only seems to be reflected in a payment request when it is set in an ApplePayJS request, and not the W3C Payment Request API. This is because we end up calling `WebCore::convertAndValidate()` multiple times for `ApplePayRequestBase` in the Payment Request flow, which means we end up `WTFMove`-ing the `supportedCountries` vector on the first call and said vector is empty on subsequent entries into the function, so we ultimately set it to an empty vector. The multiple calls to `WebCore::convertAndValidate()` is an architectural issue that we should address (https://bugs.webkit.org/show_bug.cgi?id=260050), but in the meantime we opt to fix the issue by copying the `supportedCountries` vector before moving it at the `setSupportedCountries()` callsite in `WebCore::convertAndValidate()`. We also modify `WebCore::convertAndValidate()`'s signature to expect a const L-value reference to `ApplePayRequestBase` instead, which should mitigate against accidental data corruption. This patch also introduces a layout test (and some associated Internals plumbing) that improves our test coverage for the `supportedCountries` attribute in payment requests. * LayoutTests/http/tests/paymentrequest/paymentrequest-supportedCountries.https-expected.txt: Added. * LayoutTests/http/tests/paymentrequest/paymentrequest-supportedCountries.https.html: Added. * LayoutTests/http/tests/resources/payment-request.js: (async validPaymentMethod): * Source/WebCore/Modules/applepay/ApplePayRequestBase.cpp: (WebCore::convertAndValidate): * Source/WebCore/Modules/applepay/ApplePayRequestBase.h: * Source/WebCore/testing/MockPaymentCoordinator.cpp: (WebCore::MockPaymentCoordinator::showPaymentUI): * Source/WebCore/testing/MockPaymentCoordinator.h: * Source/WebCore/testing/MockPaymentCoordinator.idl: Canonical link: https://commits.webkit.org/266829@main
WebKit · Aug 11, 2023 · e5afaf9 · e5afaf9
1 parent 43d0b6d
commit e5afaf9
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 4 deletions.
diff --git a/LayoutTests/http/tests/paymentrequest/paymentrequest-supportedCountries.https-expected.txt b/LayoutTests/http/tests/paymentrequest/paymentrequest-supportedCountries.https-expected.txt
@@ -0,0 +1,4 @@
+
+PASS Should not have a default value for `supportedCountries`.
+PASS Should propagate `supportedCountries` if provided.
+
diff --git a/LayoutTests/http/tests/paymentrequest/paymentrequest-supportedCountries.https.html b/LayoutTests/http/tests/paymentrequest/paymentrequest-supportedCountries.https.html
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Tests for providing `supportedCountries` in `data` as part of `PaymentMethodData`.</title>
+<script src="/js-test-resources/ui-helper.js"></script>
+<script src="/resources/payment-request.js"></script>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<body>
+<script>
+setup({ explicit_done: true, explicit_timeout: true });
+
+user_activation_test(async (test) => {
+    let request = new PaymentRequest([validPaymentMethod()], validPaymentDetails());
+    request.addEventListener("merchantvalidation", (event) => {
+        event.complete({ });
+    });
+    request.addEventListener("shippingaddresschange", (event) => {
+        assert_array_equals(internals.mockPaymentCoordinator.supportedCountries, [], "check that `supportedCountries` is still empty after an update");
+        event.updateWith({ });
+        internals.mockPaymentCoordinator.acceptPayment();
+    });
+
+    let response = await request.show();
+
+    assert_array_equals(internals.mockPaymentCoordinator.supportedCountries, [], "check that `supportedCountries` is still empty after the payment is accepted");
+
+    await response.complete("success");
+}, "Should not have a default value for `supportedCountries`.");
+
+user_activation_test(async (test) => {
+    const method = validPaymentMethod();
+    method.data.supportedCountries = ['CA', 'GB', 'US'];
+
+    let request = new PaymentRequest([method], validPaymentDetails());
+    request.addEventListener("merchantvalidation", (event) => {
+        event.complete({ });
+    });
+    request.addEventListener("shippingaddresschange", (event) => {
+        assert_array_equals(internals.mockPaymentCoordinator.supportedCountries, method.data.supportedCountries, "check that `supportedCountries` still matches after an update");
+        event.updateWith({ });
+        internals.mockPaymentCoordinator.acceptPayment();
+    });
+
+    let response = await request.show();
+
+    assert_array_equals(internals.mockPaymentCoordinator.supportedCountries, method.data.supportedCountries, "check that `supportedCountries` still matches after the payment is accepted");
+
+    await response.complete("success");
+}, "Should propagate `supportedCountries` if provided.");
+done();
+</script>
diff --git a/LayoutTests/http/tests/resources/payment-request.js b/LayoutTests/http/tests/resources/payment-request.js
@@ -25,7 +25,7 @@ function validPaymentMethod()
     return {
         supportedMethods: 'https://apple.com/apple-pay',
         data: {
-            version: 2,
+            version: 3,
             merchantIdentifier: '',
             countryCode: 'US',
             supportedNetworks: ['visa', 'masterCard'],

diff --git a/Source/WebCore/Modules/applepay/ApplePayRequestBase.cpp b/Source/WebCore/Modules/applepay/ApplePayRequestBase.cpp
@@ -58,7 +58,7 @@ static ExceptionOr<Vector<String>> convertAndValidate(Document& document, unsign
     return WTFMove(result);
 }
 
-ExceptionOr<ApplePaySessionPaymentRequest> convertAndValidate(Document& document, unsigned version, ApplePayRequestBase& request, const PaymentCoordinator& paymentCoordinator)
+ExceptionOr<ApplePaySessionPaymentRequest> convertAndValidate(Document& document, unsigned version, const ApplePayRequestBase& request, const PaymentCoordinator& paymentCoordinator)
 {
     if (!version || !paymentCoordinator.supportsVersion(document, version))
         return Exception { InvalidAccessError, makeString('"', version, "\" is not a supported version.") };
@@ -103,7 +103,7 @@ ExceptionOr<ApplePaySessionPaymentRequest> convertAndValidate(Document& document
     result.setApplicationData(request.applicationData);
 
     if (version >= 3)
-        result.setSupportedCountries(WTFMove(request.supportedCountries));
+        result.setSupportedCountries(Vector { request.supportedCountries });
 
 #if ENABLE(APPLE_PAY_INSTALLMENTS)
     if (request.installmentConfiguration) {

diff --git a/Source/WebCore/Modules/applepay/ApplePayRequestBase.h b/Source/WebCore/Modules/applepay/ApplePayRequestBase.h
@@ -73,7 +73,7 @@ struct ApplePayRequestBase {
 #endif
 };
 
-ExceptionOr<ApplePaySessionPaymentRequest> convertAndValidate(Document&, unsigned version, ApplePayRequestBase&, const PaymentCoordinator&);
+ExceptionOr<ApplePaySessionPaymentRequest> convertAndValidate(Document&, unsigned version, const ApplePayRequestBase&, const PaymentCoordinator&);
 
 } // namespace WebCore
 

diff --git a/Source/WebCore/testing/MockPaymentCoordinator.cpp b/Source/WebCore/testing/MockPaymentCoordinator.cpp
@@ -114,6 +114,7 @@ bool MockPaymentCoordinator::showPaymentUI(const URL&, const Vector<URL>&, const
 {
     if (request.shippingContact().pkContact())
         m_shippingAddress = request.shippingContact().toApplePayPaymentContact(request.version());
+    m_supportedCountries = request.supportedCountries();
     m_shippingMethods = request.shippingMethods();
     m_requiredBillingContactFields = request.requiredBillingContactFields();
     m_requiredShippingContactFields = request.requiredShippingContactFields();

diff --git a/Source/WebCore/testing/MockPaymentCoordinator.h b/Source/WebCore/testing/MockPaymentCoordinator.h
@@ -71,6 +71,7 @@ class MockPaymentCoordinator final : public PaymentCoordinatorClient {
     const Vector<ApplePayLineItem>& lineItems() const { return m_lineItems; }
     const Vector<MockPaymentError>& errors() const { return m_errors; }
     const Vector<ApplePayShippingMethod>& shippingMethods() const { return m_shippingMethods; }
+    const Vector<String>& supportedCountries() const { return m_supportedCountries; }
     const MockPaymentContactFields& requiredBillingContactFields() const { return m_requiredBillingContactFields; }
     const MockPaymentContactFields& requiredShippingContactFields() const { return m_requiredShippingContactFields; }
 
@@ -144,6 +145,7 @@ class MockPaymentCoordinator final : public PaymentCoordinatorClient {
     Vector<ApplePayLineItem> m_lineItems;
     Vector<MockPaymentError> m_errors;
     Vector<ApplePayShippingMethod> m_shippingMethods;
+    Vector<String> m_supportedCountries;
     HashSet<String, ASCIICaseInsensitiveHash> m_availablePaymentNetworks;
     MockPaymentContactFields m_requiredBillingContactFields;
     MockPaymentContactFields m_requiredShippingContactFields;

diff --git a/Source/WebCore/testing/MockPaymentCoordinator.idl b/Source/WebCore/testing/MockPaymentCoordinator.idl
@@ -44,6 +44,7 @@
     readonly attribute sequence<ApplePayShippingMethod> shippingMethods;
     readonly attribute MockPaymentContactFields requiredBillingContactFields;
     readonly attribute MockPaymentContactFields requiredShippingContactFields;
+    readonly attribute sequence<DOMString> supportedCountries;
 
     readonly attribute ApplePaySetupConfiguration setupConfiguration;