Attachment 349095 Details for Bug 189386 – Patch

[patch] Patch

bug-189386-20180906171306.patch (text/plain), 19.83 KB, created by Andy Estes on 2018-09-06 17:13:07 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Andy Estes

Created: 2018-09-06 17:13:07 PDT

Size: 19.83 KB

patch

obsolete

>Subversion Revision: 235626
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 8a8b6c14ad9a41efb2cd33d4b9425eef24c3c5ab..d3d4816910e475dfa2b5d085997f8a23a1becca0 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,36 @@
>+2018-09-06  Andy Estes  <aestes@apple.com>
>+
>+        [Apple Pay] Dispatch a paymentmethodchange event when the payment method changes
>+        https://bugs.webkit.org/show_bug.cgi?id=189386
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Implemented the "payment method changed" algorithm as defined in the Payment Request API W3C
>+        Editor's Draft of 05 September 2018.
>+
>+        Payment Request says that the user agent MAY run this algorithm when the payment method
>+        changes. In our case, we only wish to dispatch this event when a listener is registered for
>+        it. Since PassKit requires merchants to respond to this event by calling updateWith() within
>+        30 seconds, firing the event unconditionally would break compatibility with existing clients.
>+
>+        For merchants that do not listen for this event, they can continue to use modifiers to
>+        update details based on the selected payment method type.
>+
>+        Test: http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https.html
>+
>+        * Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:
>+        (WebCore::toJSDictionary):
>+        (WebCore::ApplePayPaymentHandler::didAuthorizePayment):
>+        (WebCore::ApplePayPaymentHandler::didSelectPaymentMethod):
>+        * Modules/paymentrequest/PaymentMethodChangeEvent.cpp:
>+        (WebCore::PaymentMethodChangeEvent::PaymentMethodChangeEvent):
>+        * Modules/paymentrequest/PaymentMethodChangeEvent.h:
>+        * Modules/paymentrequest/PaymentRequest.cpp:
>+        (WebCore::PaymentRequest::paymentMethodChanged):
>+        * Modules/paymentrequest/PaymentRequest.h:
>+        * Modules/paymentrequest/PaymentRequestUpdateEvent.cpp:
>+        (WebCore::PaymentRequestUpdateEvent::updateWith):
>+
> 2018-09-03  Dean Jackson  <dino@apple.com>
> 
>         Move SystemPreview code from WebKitAdditions to WebKit
>diff --git a/Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp b/Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp
>index 98ba84712c111031a54c4eef3bcb2a05584ffa4c..56bff152d2d51b11c8acf129e7ee9e89783a5827 100644
>--- a/Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp
>+++ b/Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp
>@@ -37,6 +37,7 @@
> #include "EventNames.h"
> #include "Frame.h"
> #include "JSApplePayPayment.h"
>+#include "JSApplePayPaymentMethod.h"
> #include "JSApplePayRequest.h"
> #include "LinkIconCollector.h"
> #include "MerchantValidationEvent.h"
>@@ -440,14 +441,19 @@ static Ref<PaymentAddress> convert(const ApplePayPaymentContact& contact)
>     return PaymentAddress::create(contact.countryCode, contact.addressLines.value_or(Vector<String>()), contact.administrativeArea, contact.locality, contact.subLocality, contact.postalCode, String(), String(), contact.localizedName, contact.phoneNumber);
> }
> 
>+template<typename T>
>+static JSC::Strong<JSC::JSObject> toJSDictionary(JSC::ExecState& execState, const T& value)
>+{
>+    JSC::JSLockHolder lock { &execState };
>+    return { execState.vm(), asObject(toJS<IDLDictionary<T>>(execState, *JSC::jsCast<JSDOMGlobalObject*>(execState.lexicalGlobalObject()), value)) };
>+}
>+
> void ApplePayPaymentHandler::didAuthorizePayment(const Payment& payment)
> {
>     ASSERT(!m_isUpdating);
> 
>     auto applePayPayment = payment.toApplePayPayment(version());
>-    auto& execState = *document().execState();
>-    auto lock = JSC::JSLockHolder { &execState };
>-    auto details = JSC::Strong<JSC::JSObject> { execState.vm(), asObject(toJS<IDLDictionary<ApplePayPayment>>(execState, *JSC::jsCast<JSDOMGlobalObject*>(execState.lexicalGlobalObject()), applePayPayment)) };
>+    auto details = toJSDictionary(*document().execState(), applePayPayment);
>     const auto& shippingContact = applePayPayment.shippingContact.value_or(ApplePayPaymentContact());
>     m_paymentRequest->accept(WTF::get<URL>(m_identifier).string(), WTFMove(details), convert(shippingContact), shippingContact.localizedName, shippingContact.emailAddress, shippingContact.phoneNumber);
> }
>@@ -473,8 +479,11 @@ void ApplePayPaymentHandler::didSelectPaymentMethod(const PaymentMethod& payment
>     ASSERT(!m_isUpdating);
>     m_isUpdating = true;
> 
>-    m_selectedPaymentMethodType = paymentMethod.toApplePayPaymentMethod().type;
>-    m_paymentRequest->paymentMethodChanged();
>+    auto applePayPaymentMethod = paymentMethod.toApplePayPaymentMethod();
>+    m_selectedPaymentMethodType = applePayPaymentMethod.type;
>+
>+    auto details = toJSDictionary(*document().execState(), applePayPaymentMethod);
>+    m_paymentRequest->paymentMethodChanged(WTF::get<URL>(m_identifier).string(), WTFMove(details));
> }
> 
> void ApplePayPaymentHandler::didCancelPaymentSession()
>diff --git a/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp b/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp
>index 60082b9c1e6e9197cf46c5ad6dbf93281f0b090c..34e85f631f62f0e932a9d1c8dfb21eee6bfa178f 100644
>--- a/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp
>+++ b/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.cpp
>@@ -44,6 +44,13 @@ PaymentMethodChangeEvent::PaymentMethodChangeEvent(const AtomicString& type, con
> {
> }
> 
>+PaymentMethodChangeEvent::PaymentMethodChangeEvent(const AtomicString& type, PaymentRequest& request, const String& methodName, JSC::Strong<JSC::JSObject>&& methodDetails)
>+    : PaymentRequestUpdateEvent { type, request }
>+    , m_methodName { methodName }
>+    , m_methodDetails { WTFMove(methodDetails) }
>+{
>+}
>+
> } // namespace WebCore
> 
> #endif // ENABLE(PAYMENT_REQUEST)
>diff --git a/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h b/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h
>index 42754d4f42740ad3598fe5a5433d50e61c68636f..fc40ef113d93647737dc5f8c808c4f9d30f952f5 100644
>--- a/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h
>+++ b/Source/WebCore/Modules/paymentrequest/PaymentMethodChangeEvent.h
>@@ -54,6 +54,7 @@ public:
> 
> private:
>     PaymentMethodChangeEvent(const AtomicString& type, const PaymentMethodChangeEventInit&);
>+    PaymentMethodChangeEvent(const AtomicString& type, PaymentRequest&, const String& methodName, JSC::Strong<JSC::JSObject>&& methodDetails);
> 
>     String m_methodName;
>     JSC::Strong<JSC::JSObject> m_methodDetails;
>diff --git a/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp b/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp
>index 24cb5ee1b9af5743683c0e66bef46bc2b1bb013b..7c5989c8d21ac0452080a942a5bb9e333b813bd0 100644
>--- a/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp
>+++ b/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp
>@@ -38,6 +38,7 @@
> #include "PaymentCurrencyAmount.h"
> #include "PaymentDetailsInit.h"
> #include "PaymentHandler.h"
>+#include "PaymentMethodChangeEvent.h"
> #include "PaymentMethodData.h"
> #include "PaymentOptions.h"
> #include "PaymentRequestUpdateEvent.h"
>@@ -552,10 +553,14 @@ void PaymentRequest::shippingOptionChanged(const String& shippingOption)
>     });
> }
> 
>-void PaymentRequest::paymentMethodChanged()
>+void PaymentRequest::paymentMethodChanged(const String& methodName, JSC::Strong<JSC::JSObject>&& methodDetails)
> {
>-    whenDetailsSettled([this, protectedThis = makeRefPtr(this)] {
>-        m_activePaymentHandler->detailsUpdated(UpdateReason::PaymentMethodChanged, { });
>+    whenDetailsSettled([this, protectedThis = makeRefPtr(this), methodName, methodDetails = WTFMove(methodDetails)]() mutable {
>+        auto& eventName = eventNames().paymentmethodchangeEvent;
>+        if (hasEventListeners(eventName))
>+            dispatchEvent(PaymentMethodChangeEvent::create(eventName, *this, methodName, WTFMove(methodDetails)));
>+        else
>+            m_activePaymentHandler->detailsUpdated(UpdateReason::PaymentMethodChanged, { });
>     });
> }
> 
>diff --git a/Source/WebCore/Modules/paymentrequest/PaymentRequest.h b/Source/WebCore/Modules/paymentrequest/PaymentRequest.h
>index 7457ad7382dac8efe05fa7251a77e4c1d0b42fa5..b23b7187d4be6e06b1b918318c80392faeba0e15 100644
>--- a/Source/WebCore/Modules/paymentrequest/PaymentRequest.h
>+++ b/Source/WebCore/Modules/paymentrequest/PaymentRequest.h
>@@ -87,7 +87,7 @@ public:
> 
>     void shippingAddressChanged(Ref<PaymentAddress>&&);
>     void shippingOptionChanged(const String& shippingOption);
>-    void paymentMethodChanged();
>+    void paymentMethodChanged(const String& methodName, JSC::Strong<JSC::JSObject>&& methodDetails);
>     ExceptionOr<void> updateWith(UpdateReason, Ref<DOMPromise>&&);
>     ExceptionOr<void> completeMerchantValidation(Event&, Ref<DOMPromise>&&);
>     void accept(const String& methodName, JSC::Strong<JSC::JSObject>&& details, Ref<PaymentAddress>&& shippingAddress, const String& payerName, const String& payerEmail, const String& payerPhone);
>diff --git a/Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEvent.cpp b/Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEvent.cpp
>index 0eb72ba3a57f3235c8b8fc28da0b2f258f6ec772..61aec48d2223aadf59dd4329c152e3f8e496479f 100644
>--- a/Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEvent.cpp
>+++ b/Source/WebCore/Modules/paymentrequest/PaymentRequestUpdateEvent.cpp
>@@ -36,12 +36,14 @@ namespace WebCore {
> PaymentRequestUpdateEvent::PaymentRequestUpdateEvent(const AtomicString& type, const PaymentRequestUpdateEventInit& eventInit)
>     : Event { type, eventInit, IsTrusted::No }
> {
>+    ASSERT(!isTrusted());
> }
> 
> PaymentRequestUpdateEvent::PaymentRequestUpdateEvent(const AtomicString& type, PaymentRequest& paymentRequest)
>     : Event { type, CanBubble::No, IsCancelable::No }
>     , m_paymentRequest { &paymentRequest }
> {
>+    ASSERT(isTrusted());
> }
> 
> PaymentRequestUpdateEvent::~PaymentRequestUpdateEvent() = default;
>@@ -51,6 +53,8 @@ ExceptionOr<void> PaymentRequestUpdateEvent::updateWith(Ref<DOMPromise>&& detail
>     if (!isTrusted())
>         return Exception { InvalidStateError };
> 
>+    ASSERT(m_paymentRequest);
>+
>     if (m_waitForUpdate)
>         return Exception { InvalidStateError };
> 
>@@ -62,6 +66,8 @@ ExceptionOr<void> PaymentRequestUpdateEvent::updateWith(Ref<DOMPromise>&& detail
>         reason = PaymentRequest::UpdateReason::ShippingAddressChanged;
>     else if (type() == eventNames().shippingoptionchangeEvent)
>         reason = PaymentRequest::UpdateReason::ShippingOptionChanged;
>+    else if (type() == eventNames().paymentmethodchangeEvent)
>+        reason = PaymentRequest::UpdateReason::PaymentMethodChanged;
>     else {
>         ASSERT_NOT_REACHED();
>         return Exception { TypeError };
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index aa3dd20faebf3a80b938a995ced0e33b68f3aaf7..6a882b82d6619d21fe277b7ee0912630c3e8e743 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-09-06  Andy Estes  <aestes@apple.com>
>+
>+        [Apple Pay] Dispatch a paymentmethodchange event when the payment method changes
>+        https://bugs.webkit.org/show_bug.cgi?id=189386
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https-expected.txt: Added.
>+        * http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https.html: Added.
>+
> 2018-09-03  Dean Jackson  <dino@apple.com>
> 
>         Move SystemPreview code from WebKitAdditions to WebKit
>diff --git a/LayoutTests/http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https-expected.txt b/LayoutTests/http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..df583f6a42b1a91ed69c61a53b3c4720d41641d0
>--- /dev/null
>+++ b/LayoutTests/http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https-expected.txt
>@@ -0,0 +1,34 @@
>+CONSOLE MESSAGE: Unhandled Promise Rejection: AbortError: The operation was aborted.
>+Test PaymentMethodChangeEvent with Apple Pay.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+Testing that PaymentMethodChangeEvent fires after selecting a 'credit' payment method type.
>+PASS event.methodName is 'https://apple.com/apple-pay'
>+PASS event.methodDetails.type is 'credit'
>+
>+Testing that PaymentMethodChangeEvent.updateWith() updates payment details after selecting a 'credit' payment method type.
>+PASS event.methodName is 'https://apple.com/apple-pay'
>+PASS event.methodDetails.type is 'credit'
>+PASS internals.mockPaymentCoordinator.total.label is 'Total'
>+PASS internals.mockPaymentCoordinator.total.amount is '15.00'
>+PASS internals.mockPaymentCoordinator.lineItems.length is 1
>+PASS internals.mockPaymentCoordinator.lineItems[0].label is 'Item'
>+PASS internals.mockPaymentCoordinator.lineItems[0].amount is '10.00'
>+
>+Testing that PaymentMethodChangeEvent.updateWith() applies modifiers after selecting a 'credit' payment method type.
>+PASS event.methodName is 'https://apple.com/apple-pay'
>+PASS event.methodDetails.type is 'credit'
>+PASS internals.mockPaymentCoordinator.total.label is 'Credit total override'
>+PASS internals.mockPaymentCoordinator.total.amount is '20.00'
>+PASS internals.mockPaymentCoordinator.lineItems.length is 2
>+PASS internals.mockPaymentCoordinator.lineItems[0].label is 'Item'
>+PASS internals.mockPaymentCoordinator.lineItems[0].amount is '10.00'
>+PASS internals.mockPaymentCoordinator.lineItems[1].label is 'Credit surcharge'
>+PASS internals.mockPaymentCoordinator.lineItems[1].amount is '10.00'
>+
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https.html b/LayoutTests/http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..a7156f5d4ea1818f107c19d7588918afb86e8e5e
>--- /dev/null
>+++ b/LayoutTests/http/tests/ssl/applepay/ApplePayPaymentMethodChangeEvent.https.html
>@@ -0,0 +1,162 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta charset="utf-8">
>+<script src="/js-test-resources/ui-helper.js"></script>
>+<script src="/resources/js-test-pre.js"></script>
>+<script src="/resources/payment-request.js"></script>
>+</head>
>+<body>
>+<script>
>+
>+description("Test PaymentMethodChangeEvent with Apple Pay.");
>+
>+window.jsTestIsAsync = true;
>+
>+function validPaymentMethod() {
>+    return {
>+        supportedMethods: 'https://apple.com/apple-pay',
>+        data: {
>+            version: 2,
>+            merchantIdentifier: '',
>+            countryCode: 'US',
>+            supportedNetworks: ['visa', 'masterCard'],
>+            merchantCapabilities: ['supports3DS'],
>+        },
>+    }
>+}
>+
>+function validPaymentDetails() {
>+    return {
>+        total: {
>+            label: 'Total',
>+            amount: {
>+                currency: 'USD',
>+                value: '10.00',
>+            },
>+        },
>+        displayItems: [{
>+            label: 'Item',
>+            amount: {
>+                currency: 'USD',
>+                value: '10.00',
>+            },
>+        }],
>+    }
>+}
>+
>+async function runTests() {
>+    await new Promise((resolve, reject) => {
>+        debug("Testing that PaymentMethodChangeEvent fires after selecting a 'credit' payment method type.");
>+
>+        activateThen(() => {
>+            var paymentRequest = new PaymentRequest([validPaymentMethod()], validPaymentDetails());
>+            try {
>+                paymentRequest.onpaymentmethodchange = (event) => {
>+                    shouldBe("event.methodName", "'https://apple.com/apple-pay'");
>+                    shouldBe("event.methodDetails.type", "'credit'");
>+                    paymentRequest.abort();
>+                    resolve();
>+                };
>+
>+                paymentRequest.show();
>+                internals.mockPaymentCoordinator.changePaymentMethod({ type: 'credit' });
>+            } catch (error) {
>+            }
>+        });
>+    });
>+    debug("");
>+
>+    await new Promise((resolve, reject) => {
>+        debug("Testing that PaymentMethodChangeEvent.updateWith() updates payment details after selecting a 'credit' payment method type.");
>+
>+        activateThen(() => {
>+            var paymentRequest = new PaymentRequest([validPaymentMethod()], validPaymentDetails());
>+
>+            paymentRequest.onpaymentmethodchange = (event) => {
>+                shouldBe("event.methodName", "'https://apple.com/apple-pay'");
>+                shouldBe("event.methodDetails.type", "'credit'");
>+
>+                var detailsUpdate = validPaymentDetails();
>+                detailsUpdate.total.amount.value = '15.00';
>+                event.updateWith(detailsUpdate);
>+                internals.mockPaymentCoordinator.acceptPayment();
>+            };
>+
>+            paymentRequest.show().then((response) => {
>+                shouldBe("internals.mockPaymentCoordinator.total.label", "'Total'");
>+                shouldBe("internals.mockPaymentCoordinator.total.amount", "'15.00'");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems.length", "1");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems[0].label", "'Item'");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems[0].amount", "'10.00'");
>+                response.complete("success");
>+                resolve();
>+            });
>+
>+            internals.mockPaymentCoordinator.changePaymentMethod({ type: 'credit' });
>+        });
>+    });
>+    debug("");
>+
>+    await new Promise((resolve, reject) => {
>+        debug("Testing that PaymentMethodChangeEvent.updateWith() applies modifiers after selecting a 'credit' payment method type.");
>+
>+        activateThen(() => {
>+            var paymentRequest = new PaymentRequest([validPaymentMethod()], validPaymentDetails());
>+
>+            paymentRequest.onpaymentmethodchange = (event) => {
>+                shouldBe("event.methodName", "'https://apple.com/apple-pay'");
>+                shouldBe("event.methodDetails.type", "'credit'");
>+
>+                var detailsUpdate = validPaymentDetails();
>+                detailsUpdate.total.amount.value = '15.00';
>+                detailsUpdate.modifiers = [{
>+                    supportedMethods: 'https://apple.com/apple-pay',
>+                    total: {
>+                        label: 'Credit total override',
>+                        amount: {
>+                            currency: 'USD',
>+                            value: '20.00',
>+                        },
>+                    },
>+                    additionalDisplayItems: [{
>+                        label: 'Credit surcharge',
>+                        amount: {
>+                            currency: 'USD',
>+                            value: '10.00',
>+                        },
>+                    }],
>+                    data: {
>+                        paymentMethodType: 'credit',
>+                    },
>+                }];
>+
>+                event.updateWith(detailsUpdate);
>+                internals.mockPaymentCoordinator.acceptPayment();
>+            };
>+
>+            paymentRequest.show().then((response) => {
>+                shouldBe("internals.mockPaymentCoordinator.total.label", "'Credit total override'");
>+                shouldBe("internals.mockPaymentCoordinator.total.amount", "'20.00'");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems.length", "2");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems[0].label", "'Item'");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems[0].amount", "'10.00'");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems[1].label", "'Credit surcharge'");
>+                shouldBe("internals.mockPaymentCoordinator.lineItems[1].amount", "'10.00'");
>+                response.complete("success");
>+                resolve();
>+            });
>+
>+            internals.mockPaymentCoordinator.changePaymentMethod({ type: 'credit' });
>+        });
>+    });
>+    debug("");
>+
>+    finishJSTest();
>+}
>+
>+runTests();
>+</script>
>+<script src="/resources/js-test-post.js"></script>
>+</body>
>+</html>

Actions: View | Formatted Diff | Diff

Attachments on bug 189386: 349095 | 349259 | 349261 | 349262 | 349263 | 349264 | 349265