[Apple Pay] Support granular errors in PaymentDetailsUpdate
Created attachment 350717 [details] Patch
Created attachment 350899 [details] Patch
Created attachment 350903 [details] Patch
Comment on attachment 350903 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=350903&action=review > Source/WebCore/Modules/applepay/ApplePayErrorCode.h:2 > + * Copyright (C) 2017-2018 Apple Inc. All rights reserved. 2018? > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:315 > +static inline void appendShippingContactInvalidError(String&& message, std::optional<PaymentError::ContactField>&& contactField, Vector<PaymentError>& errors) If ContactField is an enum, I believe we go with std::optional<> without &&. > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:321 > +Vector<PaymentError> ApplePayPaymentHandler::computeErrors(String&& error, AddressErrors&& addressErrors, PayerErrorFields&& payerErrors, JSC::Strong<JSC::JSObject>&& paymentMethodErrors) const Does paymentMethodErrors need to be a JSC::Strong, can we make it a JSC::JSObject& instead? > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:357 > + errors.append({ applePayError->code(), applePayError->message(), applePayError->contactField() }); We can probably move applePayError->message()
(In reply to youenn fablet from comment #4) > Comment on attachment 350903 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=350903&action=review > > > Source/WebCore/Modules/applepay/ApplePayErrorCode.h:2 > > + * Copyright (C) 2017-2018 Apple Inc. All rights reserved. > > 2018? Yeah I'll remove this. > > > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:315 > > +static inline void appendShippingContactInvalidError(String&& message, std::optional<PaymentError::ContactField>&& contactField, Vector<PaymentError>& errors) > > If ContactField is an enum, I believe we go with std::optional<> without &&. Ok. > > > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:321 > > +Vector<PaymentError> ApplePayPaymentHandler::computeErrors(String&& error, AddressErrors&& addressErrors, PayerErrorFields&& payerErrors, JSC::Strong<JSC::JSObject>&& paymentMethodErrors) const > > Does paymentMethodErrors need to be a JSC::Strong, can we make it a > JSC::JSObject& instead? We can make it a JSC::JSObject*. It might be null. > > > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:357 > > + errors.append({ applePayError->code(), applePayError->message(), applePayError->contactField() }); > > We can probably move applePayError->message() Yes. Thanks for reviewing!
(In reply to Andy Estes from comment #5) > (In reply to youenn fablet from comment #4) > > Comment on attachment 350903 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=350903&action=review > > > > > Source/WebCore/Modules/applepay/ApplePayErrorCode.h:2 > > > + * Copyright (C) 2017-2018 Apple Inc. All rights reserved. > > > > 2018? > > Yeah I'll remove this. > > > > > > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:315 > > > +static inline void appendShippingContactInvalidError(String&& message, std::optional<PaymentError::ContactField>&& contactField, Vector<PaymentError>& errors) > > > > If ContactField is an enum, I believe we go with std::optional<> without &&. > > Ok. > > > > > > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:321 > > > +Vector<PaymentError> ApplePayPaymentHandler::computeErrors(String&& error, AddressErrors&& addressErrors, PayerErrorFields&& payerErrors, JSC::Strong<JSC::JSObject>&& paymentMethodErrors) const > > > > Does paymentMethodErrors need to be a JSC::Strong, can we make it a > > JSC::JSObject& instead? > > We can make it a JSC::JSObject*. It might be null. > > > > > > Source/WebCore/Modules/applepay/paymentrequest/ApplePayPaymentHandler.cpp:357 > > > + errors.append({ applePayError->code(), applePayError->message(), applePayError->contactField() }); > > > > We can probably move applePayError->message() > > Yes. Actually we don't need to move, because since message() returns a temporary the compiler will elide the copy.
Created attachment 350969 [details] Patch
Comment on attachment 350969 [details] Patch Clearing flags on attachment: 350969 Committed r236552: <https://trac.webkit.org/changeset/236552>
All reviewed patches have been landed. Closing bug.
<rdar://problem/44833449>
The new test: http/tests/ssl/applepay/ApplePayShippingAddressChangeEventErrorsV3.https.html added in: https://trac.webkit.org/changeset/236552/webkit is flakey failure. History: https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=http%2Ftests%2Fssl%2Fapplepay%2FApplePayShippingAddressChangeEventErrorsV3.https.html Diff: --- /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/http/tests/ssl/applepay/ApplePayShippingAddressChangeEventErrorsV3.https-expected.txt +++ /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/http/tests/ssl/applepay/ApplePayShippingAddressChangeEventErrorsV3.https-actual.txt @@ -1,3 +1,4 @@ +CONSOLE MESSAGE: Unhandled Promise Rejection: InvalidStateError: The object is in an invalid state. Test specifying Apple Pay errors in response to the shippingaddresschange event. On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
(In reply to Truitt Savell from comment #11) > The new test: > http/tests/ssl/applepay/ApplePayShippingAddressChangeEventErrorsV3.https.html > > added in: > https://trac.webkit.org/changeset/236552/webkit > > is flakey failure. > > History: > https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard. > html#showAllRuns=true&tests=http%2Ftests%2Fssl%2Fapplepay%2FApplePayShippingA > ddressChangeEventErrorsV3.https.html > > Diff: > --- > /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/ > http/tests/ssl/applepay/ApplePayShippingAddressChangeEventErrorsV3.https- > expected.txt > +++ > /Volumes/Data/slave/highsierra-debug-tests-wk2/build/layout-test-results/ > http/tests/ssl/applepay/ApplePayShippingAddressChangeEventErrorsV3.https- > actual.txt > @@ -1,3 +1,4 @@ > +CONSOLE MESSAGE: Unhandled Promise Rejection: InvalidStateError: The object > is in an invalid state. > Test specifying Apple Pay errors in response to the shippingaddresschange > event. > > On success, you will see a series of "PASS" messages, followed by "TEST > COMPLETE". Tracked by bug #190650.