[Payment Request] canMakePayment() should not consider serialized payment method...
authoraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Nov 2018 18:27:35 +0000 (18:27 +0000)
committeraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 9 Nov 2018 18:27:35 +0000 (18:27 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191432

Reviewed by Dean Jackson.

LayoutTests/imported/w3c:

* web-platform-tests/payment-request/payment-request-canmakepayment-method.https-expected.txt: Added.

Source/WebCore:

In https://github.com/w3c/payment-request/pull/806, we're changing the specification of
canMakePayment() to not consider serialized payment method data when deciding if a payment
method is supported. For Apple Pay, this means we resolve to true for
"https://apple.com/apple-pay", even if an ApplePayRequest is omitted or is missing required
fields.

Added test cases to
http/tests/paymentrequest/payment-request-canmakepayment-method.https.html and
http/tests/paymentrequest/payment-request-show-method.https.html.

* Modules/paymentrequest/PaymentRequest.cpp:
(WebCore::PaymentRequest::canMakePayment):

LayoutTests:

* http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt:
* http/tests/paymentrequest/payment-request-canmakepayment-method.https.html: Updated with
changes from imported/w3c/web-platform-tests/payment-request/. Modified two tests to use
user_activation_test() rather than test_driver.bless().
* http/tests/paymentrequest/payment-request-show-method.https-expected.txt:
* http/tests/paymentrequest/payment-request-show-method.https.html: Now that canMakePayment
does not convert payment method data, added a test that ensures show() rejects with a
TypeError when Apple Pay's payment method data is invalid.
* platform/ios-wk2/TestExpectations: Un-skipped payment-request-canmakepayment-method.https.html.
* platform/mac-wk2/TestExpectations: Ditto.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238042 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt
LayoutTests/http/tests/paymentrequest/payment-request-canmakepayment-method.https.html
LayoutTests/http/tests/paymentrequest/payment-request-show-method.https-expected.txt
LayoutTests/http/tests/paymentrequest/payment-request-show-method.https.html
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-canmakepayment-method.https-expected.txt [new file with mode: 0644]
LayoutTests/platform/ios-wk2/TestExpectations
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp

index 99b381e..9899983 100644 (file)
@@ -1,5 +1,23 @@
 2018-11-09  Andy Estes  <aestes@apple.com>
 
+        [Payment Request] canMakePayment() should not consider serialized payment method data
+        https://bugs.webkit.org/show_bug.cgi?id=191432
+
+        Reviewed by Dean Jackson.
+
+        * http/tests/paymentrequest/payment-request-canmakepayment-method.https-expected.txt:
+        * http/tests/paymentrequest/payment-request-canmakepayment-method.https.html: Updated with
+        changes from imported/w3c/web-platform-tests/payment-request/. Modified two tests to use
+        user_activation_test() rather than test_driver.bless().
+        * http/tests/paymentrequest/payment-request-show-method.https-expected.txt:
+        * http/tests/paymentrequest/payment-request-show-method.https.html: Now that canMakePayment
+        does not convert payment method data, added a test that ensures show() rejects with a
+        TypeError when Apple Pay's payment method data is invalid.
+        * platform/ios-wk2/TestExpectations: Un-skipped payment-request-canmakepayment-method.https.html.
+        * platform/mac-wk2/TestExpectations: Ditto.
+
+2018-11-09  Andy Estes  <aestes@apple.com>
+
         [Payment Request] PaymentResponse.details should be updated when the user accepts a rpayment retry
         https://bugs.webkit.org/show_bug.cgi?id=191440
 
index 41acef6..ba3031e 100644 (file)
@@ -1,10 +1,5 @@
 
-PASS If request.[[state]] is "created", then return a promise that resolves to true for known method. 
 PASS If request.[[state]] is "interactive", then return a promise rejected with an "InvalidStateError" DOMException. 
 PASS If request.[[state]] is "closed", then return a promise rejected with an "InvalidStateError" DOMException. 
-PASS If payment method identifier and serialized parts are supported, resolve promise with true. 
-PASS If a payment method identifier is supported but its serialized parts are not, resolve promise with false. 
-PASS If payment method identifier is unknown, resolve promise with false. 
-PASS Optionally, at the user agent's discretion, return a promise rejected with a "NotAllowedError" DOMException. 
 PASS Return a promise that resolves to true when Apple Pay is available, even if there isn't an active card. 
 
index dbe0bd2..43277ea 100644 (file)
@@ -4,7 +4,7 @@
 <!-- FIXME: Upstream this test to web-platform-tests/payment-request/. -->
 <meta charset="utf-8">
 <title>Tests for PaymentRequest.canMakePayment() method</title>
-<link rel="help" href="https://w3c.github.io/browser-payment-api/#show-method">
+<link rel="help" href="https://w3c.github.io/browser-payment-api/#canmakepayment-method">
 <script src="/js-test-resources/ui-helper.js"></script>
 <script src="/resources/payment-request.js"></script>
 <script src="/resources/testharness.js"></script>
@@ -21,14 +21,6 @@ const applePay = Object.freeze({
         countryCode: "US",
     }
 });
-const invalidApplePay = Object.freeze({
-    supportedMethods: "https://apple.com/apple-pay",
-    data: {
-    }
-});
-const secondInvalidApplePay = Object.freeze({
-    supportedMethods: "https://apple.com/apple-pay",
-});
 const defaultMethods = Object.freeze([applePay]);
 const defaultDetails = Object.freeze({
   total: {
@@ -40,26 +32,7 @@ const defaultDetails = Object.freeze({
   },
 });
 
-promise_test(async t => {
-  const request = new PaymentRequest(defaultMethods, defaultDetails);
-  try {
-    assert_true(
-      await request.canMakePayment(),
-      `canMakePaymentPromise should be true`
-    );
-    assert_true(
-      await request.canMakePayment(),
-      `canMakePaymentPromise should be true`
-    );
-  } catch (err) {
-    assert_equals(
-      err.name,
-      "NotAllowedError",
-      "if it throws, then it must be a NotAllowedError."
-    );
-  }
-}, `If request.[[state]] is "created", then return a promise that resolves to true for known method.`);
-
+// FIXME: This can be removed in favor of the test in imported/w3c/web-platform-tests/payment-request once we support test_driver.bless().
 user_activation_test(async t => {
   const request = new PaymentRequest(defaultMethods, defaultDetails);
   const acceptPromise = request.show(); // Sets state to "interactive"
@@ -78,11 +51,12 @@ user_activation_test(async t => {
   }
   // The state should be "closed"
   await promise_rejects(t, "InvalidStateError", request.canMakePayment());
-}, `If request.[[state]] is "interactive", then return a promise rejected with an "InvalidStateError" DOMException.`);
+}, 'If request.[[state]] is "interactive", then return a promise rejected with an "InvalidStateError" DOMException.');
 
+// FIXME: This can be removed in favor of the test in imported/w3c/web-platform-tests/payment-request once we support test_driver.bless().
 user_activation_test(async t => {
   const request = new PaymentRequest(defaultMethods, defaultDetails);
-  const acceptPromise = request.show(); // The state is now "interactive"
+  const acceptPromise = request.show(); // Sets state to "interactive"
   acceptPromise.catch(() => {}); // no-op, just to silence unhandled rejection in devtools.
   await request.abort(); // The state is now "closed"
   await promise_rejects(t, "InvalidStateError", request.canMakePayment());
@@ -99,93 +73,7 @@ user_activation_test(async t => {
       "must be an InvalidStateError."
     );
   }
-}, `If request.[[state]] is "closed", then return a promise rejected with an "InvalidStateError" DOMException.`);
-
-promise_test(async t => {
-  const request = new PaymentRequest(defaultMethods, defaultDetails);
-  assert_true(await request.canMakePayment(), "basic-card should be supported");
-}, `If payment method identifier and serialized parts are supported, resolve promise with true.`);
-
-promise_test(async t => {
-  const request = new PaymentRequest([invalidApplePay, secondInvalidApplePay], defaultDetails);
-  assert_false(await request.canMakePayment(), "Apple Pay with invalid data should not be supported");
-}, `If a payment method identifier is supported but its serialized parts are not, resolve promise with false.`);
-
-promise_test(async t => {
-  const unsupportedMethods = [
-    "this-is-not-supported",
-    "https://not.supported",
-    "e",
-    "n6jzof05mk2g4lhxr-u-q-w1-c-i-pa-ty-bdvs9-ho-ae7-p-md8-s-wq3-h-qd-e-q-sa",
-    "a-b-q-n-s-pw0",
-    "m-u",
-    "s-l5",
-    "k9-f",
-    "m-l",
-    "u4-n-t",
-    "i488jh6-g18-fck-yb-v7-i",
-    "x-x-t-t-c34-o",
-    "https://wpt",
-    "https://wpt.fyi/",
-    "https://wpt.fyi/payment",
-    "https://wpt.fyi/payment-request",
-    "https://wpt.fyi/payment-request?",
-    "https://wpt.fyi/payment-request?this=is",
-    "https://wpt.fyi/payment-request?this=is&totally",
-    "https://wpt.fyi:443/payment-request?this=is&totally",
-    "https://wpt.fyi:443/payment-request?this=is&totally#fine",
-    "https://:@wpt.fyi:443/payment-request?this=is&totally#👍",
-    " \thttps://wpt\n ",
-    "https://xn--c1yn36f",
-    "https://點看",
-  ];
-  for (const method of unsupportedMethods) {
-    try {
-      const request = new PaymentRequest(
-        [{ supportedMethods: method }],
-        defaultDetails
-      );
-      assert_false(
-        await request.canMakePayment(),
-        `method "${method}" must not be supported`
-      );
-    } catch (err) {
-      assert_true(
-        false,
-        `Unexpected exception testing method ${method}, expected false. See error console.`
-      );
-    }
-  }
-}, `If payment method identifier is unknown, resolve promise with false.`);
-
-promise_test(async t => {
-  // This test might never actually hit its assertion, but that's allowed.
-  const request = new PaymentRequest(defaultMethods, defaultDetails);
-  for (let i = 0; i < 1000; i++) {
-    try {
-      await request.canMakePayment();
-    } catch (err) {
-      assert_equals(
-        err.name,
-        "NotAllowedError",
-        "if it throws, then it must be a NotAllowedError."
-      );
-      break;
-    }
-  }
-  for (let i = 0; i < 1000; i++) {
-    try {
-      await new PaymentRequest(defaultMethods, defaultDetails).canMakePayment();
-    } catch (err) {
-      assert_equals(
-        err.name,
-        "NotAllowedError",
-        "if it throws, then it must be a NotAllowedError."
-      );
-      break;
-    }
-  }
-}, `Optionally, at the user agent's discretion, return a promise rejected with a "NotAllowedError" DOMException.`);
+}, 'If request.[[state]] is "closed", then return a promise rejected with an "InvalidStateError" DOMException.');
 
 promise_test(async t => {
   internals.mockPaymentCoordinator.setCanMakePaymentsWithActiveCard(false);
index 9c2ce29..836869e 100644 (file)
@@ -3,6 +3,7 @@ PASS Must be possible to construct a payment request
 PASS Throws if the promise [[state]] is not "created" 
 PASS If the user agent's "payment request is showing" boolean is true, then return a promise rejected with an "AbortError" DOMException. 
 PASS If payment method consultation produces no supported method of payment, then return a promise rejected with a "NotSupportedError" DOMException. 
+PASS If data conversion results in an error, then return a promise rejected with a "TypeError" DOMException. 
 PASS If the user aborts the payment request algorithm, then return a promise rejected with an "AbortError" DOMException. 
 PASS If the user aborts the payment request algorithm while details are updating, then reject the accept promise with an "AbortError" when the details settle. 
 PASS A request is updated when show()'s detail promise resolves. 
index 537c129..28b635e 100644 (file)
@@ -70,6 +70,14 @@ user_activation_test(async t => {
 }, `If payment method consultation produces no supported method of payment, then return a promise rejected with a "NotSupportedError" DOMException.`);
 
 user_activation_test(async t => {
+  const request = new PaymentRequest(
+    [{ supportedMethods: "https://apple.com/apple-pay" }],
+    defaultDetails);
+  const acceptPromise = request.show();
+  await promise_rejects(t, new TypeError, acceptPromise);
+}, `If data conversion results in an error, then return a promise rejected with a "TypeError" DOMException.`);
+
+user_activation_test(async t => {
   const request = new PaymentRequest(defaultMethods, defaultDetails);
   const acceptPromise = request.show(); // Sets state to "interactive"
   internals.mockPaymentCoordinator.cancelPayment();
index 0912bde..afe71f6 100644 (file)
@@ -1,3 +1,12 @@
+2018-11-09  Andy Estes  <aestes@apple.com>
+
+        [Payment Request] canMakePayment() should not consider serialized payment method data
+        https://bugs.webkit.org/show_bug.cgi?id=191432
+
+        Reviewed by Dean Jackson.
+
+        * web-platform-tests/payment-request/payment-request-canmakepayment-method.https-expected.txt: Added.
+
 2018-11-09  Jer Noble  <jer.noble@apple.com>
 
         [Cocoa] Fix failing imported/w3c/web-platform-tests/media-source/mediasource-changetype-play.html test
diff --git a/LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-canmakepayment-method.https-expected.txt b/LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-canmakepayment-method.https-expected.txt
new file mode 100644 (file)
index 0000000..9f2d9d5
--- /dev/null
@@ -0,0 +1,9 @@
+If you find a buggy test, please file a bug and tag one of the suggested reviewers.
+
+PASS If payment method identifier are supported, resolve promise with true. 
+FAIL If request.[[state]] is "interactive", then return a promise rejected with an "InvalidStateError" DOMException. promise_test: Unhandled rejection with value: object "ReferenceError: Can't find variable: test_driver"
+FAIL If request.[[state]] is "closed", then return a promise rejected with an "InvalidStateError" DOMException. promise_test: Unhandled rejection with value: object "ReferenceError: Can't find variable: test_driver"
+PASS If request.[[state]] is "created", then return a promise that resolves to true for known method. 
+PASS All methods are unsupported 
+PASS Mix of supported and unsupported methods, at least one method is supported. 
+
index 8456158..7bf6600 100644 (file)
@@ -42,7 +42,6 @@ webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpayment
 
 # skip in favor of tests in http/tests/paymentrequest
 imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https.html [ WontFix ]
-imported/w3c/web-platform-tests/payment-request/payment-request-canmakepayment-method.https.html [ WontFix ]
 imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https.html [ WontFix ]
 imported/w3c/web-platform-tests/payment-request/rejects_if_not_active.https.html [ WontFix ]
 
index afd369c..4b30754 100644 (file)
@@ -47,7 +47,6 @@ webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpayment
 
 # skip in favor of tests in http/tests/paymentrequest
 imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https.html [ WontFix ]
-imported/w3c/web-platform-tests/payment-request/payment-request-canmakepayment-method.https.html [ WontFix ]
 imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https.html [ WontFix ]
 imported/w3c/web-platform-tests/payment-request/rejects_if_not_active.https.html [ WontFix ]
 
index c10117d..1b5d495 100644 (file)
@@ -1,5 +1,25 @@
 2018-11-09  Andy Estes  <aestes@apple.com>
 
+        [Payment Request] canMakePayment() should not consider serialized payment method data
+        https://bugs.webkit.org/show_bug.cgi?id=191432
+
+        Reviewed by Dean Jackson.
+
+        In https://github.com/w3c/payment-request/pull/806, we're changing the specification of
+        canMakePayment() to not consider serialized payment method data when deciding if a payment
+        method is supported. For Apple Pay, this means we resolve to true for
+        "https://apple.com/apple-pay", even if an ApplePayRequest is omitted or is missing required
+        fields.
+
+        Added test cases to
+        http/tests/paymentrequest/payment-request-canmakepayment-method.https.html and
+        http/tests/paymentrequest/payment-request-show-method.https.html.
+
+        * Modules/paymentrequest/PaymentRequest.cpp:
+        (WebCore::PaymentRequest::canMakePayment):
+
+2018-11-09  Andy Estes  <aestes@apple.com>
+
         [Payment Request] PaymentResponse.details should be updated when the user accepts a retried payment
         https://bugs.webkit.org/show_bug.cgi?id=191440
 
index 3fecdea..9709068 100644 (file)
@@ -501,25 +501,11 @@ void PaymentRequest::canMakePayment(Document& document, CanMakePaymentPromise&&
         return;
     }
 
-    auto scope = DECLARE_CATCH_SCOPE(document.execState()->vm());
     for (auto& paymentMethod : m_serializedMethodData) {
-        auto data = parse(document, paymentMethod.serializedData);
-        ASSERT(!!scope.exception() == data.hasException());
-        if (data.hasException()) {
-            scope.clearException();
-            continue;
-        }
-
         auto handler = PaymentHandler::create(document, *this, paymentMethod.identifier);
         if (!handler)
             continue;
 
-        auto exception = handler->convertData(data.releaseReturnValue());
-        if (exception.hasException()) {
-            scope.clearException();
-            continue;
-        }
-
         handler->canMakePayment([promise = WTFMove(promise)](bool canMakePayment) mutable {
             promise.resolve(canMakePayment);
         });