- Revision
- 237134
- Author
- [email protected]
- Date
- 2018-10-15 12:40:32 -0700 (Mon, 15 Oct 2018)
Log Message
[Apple Pay] Payment authorization results with ApplePayErrors should never be considered final
https://bugs.webkit.org/show_bug.cgi?id=190559
<rdar://problem/37250908>
Reviewed by Anders Carlsson.
Source/WebCore:
When PaymentCoordinator thinks a payment authorization result is final it releases the
active ApplePaySession. The Apple Pay UI is dismissed after a receiving a final result.
However, WebCore::isFinalStateResult had the wrong idea about what was a final state,
in some cases causing PaymentCoordinator to release the active session even when the UI is
still presented. If the user authorizes payment again, the website will not receive another
paymentauthorized event, and the Apple Pay UI will eventually time out waiting for a result.
Specifically, isFinalStateResult thought that:
(1) results with STATUS_SUCCESS were always final, even if they had errors
(2) errors with code "unknown" were final
Both of these assumptions are wrong. PassKit considers any result with errors to be
non-final, even if an error has code "unknown."
Fixed WebCore::isFinalStateResult to agree with what PassKit considers to be final results.
Test: http/tests/ssl/applepay/ApplePaySessionFinalState.https.html
* Modules/applepay/ApplePaySessionPaymentRequest.cpp:
(WebCore::isFinalStateResult):
* testing/MockPaymentCoordinator.cpp:
(WebCore::MockPaymentCoordinator::completePaymentSession):
LayoutTests:
* http/tests/ssl/applepay/ApplePaySessionFinalState.https-expected.txt: Added.
* http/tests/ssl/applepay/ApplePaySessionFinalState.https.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (237133 => 237134)
--- trunk/LayoutTests/ChangeLog 2018-10-15 19:37:17 UTC (rev 237133)
+++ trunk/LayoutTests/ChangeLog 2018-10-15 19:40:32 UTC (rev 237134)
@@ -1,3 +1,14 @@
+2018-10-15 Andy Estes <[email protected]>
+
+ [Apple Pay] Payment authorization results with ApplePayErrors should never be considered final
+ https://bugs.webkit.org/show_bug.cgi?id=190559
+ <rdar://problem/37250908>
+
+ Reviewed by Anders Carlsson.
+
+ * http/tests/ssl/applepay/ApplePaySessionFinalState.https-expected.txt: Added.
+ * http/tests/ssl/applepay/ApplePaySessionFinalState.https.html: Added.
+
2018-10-15 Commit Queue <[email protected]>
Unreviewed, rolling out r237054.
Added: trunk/LayoutTests/http/tests/ssl/applepay/ApplePaySessionFinalState.https-expected.txt (0 => 237134)
--- trunk/LayoutTests/http/tests/ssl/applepay/ApplePaySessionFinalState.https-expected.txt (rev 0)
+++ trunk/LayoutTests/http/tests/ssl/applepay/ApplePaySessionFinalState.https-expected.txt 2018-10-15 19:40:32 UTC (rev 237134)
@@ -0,0 +1,10 @@
+Test that ApplePayPaymentAuthorizationResults with non-empty errors are considered non-final results, regardless of result status or error codes.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS did not crash
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/http/tests/ssl/applepay/ApplePaySessionFinalState.https.html (0 => 237134)
--- trunk/LayoutTests/http/tests/ssl/applepay/ApplePaySessionFinalState.https.html (rev 0)
+++ trunk/LayoutTests/http/tests/ssl/applepay/ApplePaySessionFinalState.https.html 2018-10-15 19:40:32 UTC (rev 237134)
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<html>
+<head>
+<meta charset="utf-8">
+<script src=""
+<script src=""
+<script src=""
+</head>
+<body>
+<script>
+
+description("Test that ApplePayPaymentAuthorizationResults with non-empty errors are considered non-final results, regardless of result status or error codes.");
+
+window.jsTestIsAsync = true;
+
+function validRequest()
+{
+ return {
+ countryCode: 'US',
+ currencyCode: 'USD',
+ supportedNetworks: ['visa', 'masterCard'],
+ merchantCapabilities: ['supports3DS'],
+ total: { label: 'Your Label', amount: '10.00' },
+ }
+}
+
+activateThen(() => {
+ var session = new ApplePaySession(2, validRequest());
+ var results = null;
+ if (window.ApplePayError) {
+ var unknownError = new ApplePayError("unknown");
+ results = [
+ { status: ApplePaySession.STATUS_SUCCESS, errors: [unknownError] },
+ { status: ApplePaySession.STATUS_FAILURE, errors: [unknownError] },
+ { status: ApplePaySession.STATUS_SUCCESS, errors: [] },
+ ];
+ } else {
+ results = [
+ ApplePaySession.STATUS_INVALID_SHIPPING_POSTAL_ADDRESS,
+ ApplePaySession.STATUS_INVALID_SHIPPING_CONTACT,
+ ApplePaySession.STATUS_SUCCESS,
+ ];
+ }
+ session._onpaymentauthorized_ = (event) => {
+ session.completePayment(results.splice(0, 1)[0]);
+ if (results.length)
+ internals.mockPaymentCoordinator.acceptPayment();
+ else {
+ testPassed("did not crash");
+ finishJSTest();
+ }
+ };
+
+ session.begin();
+ internals.mockPaymentCoordinator.acceptPayment();
+});
+</script>
+<script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (237133 => 237134)
--- trunk/Source/WebCore/ChangeLog 2018-10-15 19:37:17 UTC (rev 237133)
+++ trunk/Source/WebCore/ChangeLog 2018-10-15 19:40:32 UTC (rev 237134)
@@ -1,5 +1,38 @@
2018-10-15 Andy Estes <[email protected]>
+ [Apple Pay] Payment authorization results with ApplePayErrors should never be considered final
+ https://bugs.webkit.org/show_bug.cgi?id=190559
+ <rdar://problem/37250908>
+
+ Reviewed by Anders Carlsson.
+
+ When PaymentCoordinator thinks a payment authorization result is final it releases the
+ active ApplePaySession. The Apple Pay UI is dismissed after a receiving a final result.
+
+ However, WebCore::isFinalStateResult had the wrong idea about what was a final state,
+ in some cases causing PaymentCoordinator to release the active session even when the UI is
+ still presented. If the user authorizes payment again, the website will not receive another
+ paymentauthorized event, and the Apple Pay UI will eventually time out waiting for a result.
+
+ Specifically, isFinalStateResult thought that:
+
+ (1) results with STATUS_SUCCESS were always final, even if they had errors
+ (2) errors with code "unknown" were final
+
+ Both of these assumptions are wrong. PassKit considers any result with errors to be
+ non-final, even if an error has code "unknown."
+
+ Fixed WebCore::isFinalStateResult to agree with what PassKit considers to be final results.
+
+ Test: http/tests/ssl/applepay/ApplePaySessionFinalState.https.html
+
+ * Modules/applepay/ApplePaySessionPaymentRequest.cpp:
+ (WebCore::isFinalStateResult):
+ * testing/MockPaymentCoordinator.cpp:
+ (WebCore::MockPaymentCoordinator::completePaymentSession):
+
+2018-10-15 Andy Estes <[email protected]>
+
[Payment Request] PaymentResponse should be a ContextDestructionObserver
https://bugs.webkit.org/show_bug.cgi?id=190558
Modified: trunk/Source/WebCore/Modules/applepay/ApplePaySessionPaymentRequest.cpp (237133 => 237134)
--- trunk/Source/WebCore/Modules/applepay/ApplePaySessionPaymentRequest.cpp 2018-10-15 19:37:17 UTC (rev 237133)
+++ trunk/Source/WebCore/Modules/applepay/ApplePaySessionPaymentRequest.cpp 2018-10-15 19:40:32 UTC (rev 237134)
@@ -44,22 +44,13 @@
switch (result->status) {
case PaymentAuthorizationStatus::Success:
- return true;
+ case PaymentAuthorizationStatus::Failure:
+ return result->errors.isEmpty();
case PaymentAuthorizationStatus::PINRequired:
case PaymentAuthorizationStatus::PINIncorrect:
case PaymentAuthorizationStatus::PINLockout:
return false;
-
- case PaymentAuthorizationStatus::Failure:
- if (result->errors.isEmpty())
- return true;
-
- for (auto& error : result->errors) {
- if (error.code == PaymentError::Code::Unknown)
- return true;
- }
- return false;
}
}
Modified: trunk/Source/WebCore/testing/MockPaymentCoordinator.cpp (237133 => 237134)
--- trunk/Source/WebCore/testing/MockPaymentCoordinator.cpp 2018-10-15 19:37:17 UTC (rev 237133)
+++ trunk/Source/WebCore/testing/MockPaymentCoordinator.cpp 2018-10-15 19:40:32 UTC (rev 237134)
@@ -198,8 +198,11 @@
});
}
-void MockPaymentCoordinator::completePaymentSession(std::optional<PaymentAuthorizationResult>&&)
+void MockPaymentCoordinator::completePaymentSession(std::optional<PaymentAuthorizationResult>&& result)
{
+ if (!isFinalStateResult(result))
+ return;
+
++hideCount;
ASSERT(showCount == hideCount);
}
