[Payment Request] Implement error checking for show(), abort(), and canMakePayment()
authoraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Aug 2017 16:59:49 +0000 (16:59 +0000)
committeraestes@apple.com <aestes@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 22 Aug 2017 16:59:49 +0000 (16:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=175789

Reviewed by Brady Eidson.
LayoutTests/imported/w3c:

* web-platform-tests/payment-request/payment-request-abort-method.https-expected.txt:
* web-platform-tests/payment-request/payment-request-show-method.https-expected.txt:

Source/WebCore:

Implement many of the exceptions and promise rejections specified for PaymentRequest's
show(), abort(), and canMakePayment() methods. Also implement basic state tracking.

* Modules/paymentrequest/PaymentRequest.cpp:
(WebCore::PaymentRequest::create): Changed serializedMethodData from a
HashMap<String, String> to a Vector<PaymentRequest::Method>.
(WebCore::PaymentRequest::PaymentRequest):
(WebCore::PaymentRequest::show): Added promise rejection for invalid state, updated the
state to Interactive, stored the promise in m_showPromise, and dispatched finishShowing().
(WebCore::PaymentRequest::finishShowing): Added JSON parsing of payment method serialized
data and exception propagation. If there are no exceptions, rejected m_showPromise with
NotSupportedError since we don't yet support any payment methods.
(WebCore::PaymentRequest::abort): Added promise rejection for invalid state and stored the
promise in m_abortPromise. Dispatched a lambda to update the state to Closed, reject
m_showPromise, and resolve m_abortPromise.
(WebCore::PaymentRequest::canMakePayment): Added promise rejection for invalid state and
stored the promise in m_canMakePaymentPromise. Dispatched a lambda to resolve
m_canMakePaymentPromise with false since we don't yet support any payment methods.
* Modules/paymentrequest/PaymentRequest.h:
* Modules/paymentrequest/PaymentRequest.idl: Annotated abort() with MayThrowException.

LayoutTests:

Stopped marking payment-request-abort-method.https.html and payment-request-show-method.https.html  as flaky.

* platform/ios-wk2/TestExpectations:
* platform/mac-wk2/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/imported/w3c/ChangeLog
LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https-expected.txt
LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https-expected.txt
LayoutTests/platform/ios-wk2/TestExpectations
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp
Source/WebCore/Modules/paymentrequest/PaymentRequest.h
Source/WebCore/Modules/paymentrequest/PaymentRequest.idl

index f453e0b..5d17d15 100644 (file)
@@ -1,3 +1,15 @@
+2017-08-22  Andy Estes  <aestes@apple.com>
+
+        [Payment Request] Implement error checking for show(), abort(), and canMakePayment()
+        https://bugs.webkit.org/show_bug.cgi?id=175789
+
+        Reviewed by Brady Eidson.
+        
+        Stopped marking payment-request-abort-method.https.html and payment-request-show-method.https.html  as flaky.
+
+        * platform/ios-wk2/TestExpectations:
+        * platform/mac-wk2/TestExpectations:
+
 2017-08-22  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, enable http/wpt/beacon/contentextensions on Mac WK2 only.
index a4d2380..e19c817 100644 (file)
@@ -1,3 +1,13 @@
+2017-08-22  Andy Estes  <aestes@apple.com>
+
+        [Payment Request] Implement error checking for show(), abort(), and canMakePayment()
+        https://bugs.webkit.org/show_bug.cgi?id=175789
+
+        Reviewed by Brady Eidson.
+
+        * web-platform-tests/payment-request/payment-request-abort-method.https-expected.txt:
+        * web-platform-tests/payment-request/payment-request-show-method.https-expected.txt:
+
 2017-08-21  Youenn Fablet  <youenn@apple.com>
 
         [Cache API] Add support for Cache.add/addAll
index c5382e6..e93f80e 100644 (file)
@@ -1,6 +1,6 @@
-CONSOLE MESSAGE: Unhandled Promise Rejection: NotSupportedError: Not implemented
+CONSOLE MESSAGE: Unhandled Promise Rejection: NotSupportedError: The operation is not supported.
 
-FAIL Throws if the promise [[state]] is not "interactive" assert_throws: function "function () { throw e }" threw object "NotSupportedError: Not implemented" that is not a DOMException InvalidStateError: property "code" is equal to 9, expected 11
-FAIL Calling abort must not change the [[state]] until after "interactive" assert_throws: function "function () { throw e }" threw object "NotSupportedError: Not implemented" that is not a DOMException InvalidStateError: property "code" is equal to 9, expected 11
-FAIL calling .abort() causes acceptPromise to reject and closes the request. assert_true: Unexpected promise rejection: Not implemented expected true got false
+PASS Throws if the promise [[state]] is not "interactive" 
+PASS Calling abort must not change the [[state]] until after "interactive" 
+FAIL calling .abort() causes acceptPromise to reject and closes the request. assert_throws: function "function () { throw e }" threw object "NotSupportedError: The operation is not supported." that is not a DOMException AbortError: property "code" is equal to 9, expected 20
 
index 757807b..a77a125 100644 (file)
@@ -1,8 +1,5 @@
-CONSOLE MESSAGE: Unhandled Promise Rejection: NotSupportedError: Not implemented
-CONSOLE MESSAGE: Unhandled Promise Rejection: NotSupportedError: Not implemented
+CONSOLE MESSAGE: Unhandled Promise Rejection: NotSupportedError: The operation is not supported.
 
-Harness Error (FAIL), message = Not implemented
-
-FAIL Throws if the promise [[state]] is not "created" assert_throws: function "function () { throw e }" threw object "NotSupportedError: Not implemented" that is not a DOMException InvalidStateError: property "code" is equal to 9, expected 11
-FAIL If the user agent's "payment request is showing" boolean is true, then return a promise rejected with an "AbortError" DOMException. assert_throws: function "function () { throw e }" threw object "NotSupportedError: Not implemented" that is not a DOMException AbortError: property "code" is equal to 9, expected 20
+FAIL Throws if the promise [[state]] is not "created" assert_throws: function "function () { throw e }" threw object "NotSupportedError: The operation is not supported." that is not a DOMException AbortError: property "code" is equal to 9, expected 20
+FAIL If the user agent's "payment request is showing" boolean is true, then return a promise rejected with an "AbortError" DOMException. assert_throws: function "function () { throw e }" threw object "NotSupportedError: The operation is not supported." that is not a DOMException AbortError: property "code" is equal to 9, expected 20
 
index 3fcaf59..57a9917 100644 (file)
@@ -34,8 +34,6 @@ webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpayment
 webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpaymentrequest/removing-allowpaymentrequest.https.sub.html [ Skip ]
 webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpaymentrequest/setting-allowpaymentrequest-timing.https.sub.html [ Skip ]
 webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpaymentrequest/setting-allowpaymentrequest.https.sub.html [ Skip ]
-webkit.org/b/175753 imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https.html [ Pass Failure ]
-webkit.org/b/175753 imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https.html [ Pass Failure ]
 
 #//////////////////////////////////////////////////////////////////////////////////////////
 # End platform-specific directories.
index eff99aa..1322c50 100644 (file)
@@ -37,8 +37,6 @@ webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpayment
 webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpaymentrequest/removing-allowpaymentrequest.https.sub.html [ Skip ]
 webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpaymentrequest/setting-allowpaymentrequest-timing.https.sub.html [ Skip ]
 webkit.org/b/175611 imported/w3c/web-platform-tests/payment-request/allowpaymentrequest/setting-allowpaymentrequest.https.sub.html [ Skip ]
-webkit.org/b/175753 imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https.html [ Pass Failure ]
-webkit.org/b/175753 imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https.html [ Pass Failure ]
 
 #//////////////////////////////////////////////////////////////////////////////////////////
 # End platform-specific directories.
index 11bc0b0..9fc694e 100644 (file)
@@ -1,3 +1,31 @@
+2017-08-22  Andy Estes  <aestes@apple.com>
+
+        [Payment Request] Implement error checking for show(), abort(), and canMakePayment()
+        https://bugs.webkit.org/show_bug.cgi?id=175789
+
+        Reviewed by Brady Eidson.
+
+        Implement many of the exceptions and promise rejections specified for PaymentRequest's
+        show(), abort(), and canMakePayment() methods. Also implement basic state tracking.
+
+        * Modules/paymentrequest/PaymentRequest.cpp:
+        (WebCore::PaymentRequest::create): Changed serializedMethodData from a
+        HashMap<String, String> to a Vector<PaymentRequest::Method>.
+        (WebCore::PaymentRequest::PaymentRequest):
+        (WebCore::PaymentRequest::show): Added promise rejection for invalid state, updated the
+        state to Interactive, stored the promise in m_showPromise, and dispatched finishShowing().
+        (WebCore::PaymentRequest::finishShowing): Added JSON parsing of payment method serialized
+        data and exception propagation. If there are no exceptions, rejected m_showPromise with
+        NotSupportedError since we don't yet support any payment methods.
+        (WebCore::PaymentRequest::abort): Added promise rejection for invalid state and stored the
+        promise in m_abortPromise. Dispatched a lambda to update the state to Closed, reject
+        m_showPromise, and resolve m_abortPromise.
+        (WebCore::PaymentRequest::canMakePayment): Added promise rejection for invalid state and
+        stored the promise in m_canMakePaymentPromise. Dispatched a lambda to resolve
+        m_canMakePaymentPromise with false since we don't yet support any payment methods.
+        * Modules/paymentrequest/PaymentRequest.h:
+        * Modules/paymentrequest/PaymentRequest.idl: Annotated abort() with MayThrowException.
+
 2017-08-22  Brent Fulgham  <bfulgham@apple.com> and Pranjal Jumde  <pjumde@apple.com>
 
         Disable access to secure cookies if an HTTPS site loads mixed content
index e45fc66..80feb9c 100644 (file)
 #include "PaymentDetailsInit.h"
 #include "PaymentMethodData.h"
 #include "PaymentOptions.h"
+#include "ScriptController.h"
 #include <JavaScriptCore/JSONObject.h>
 #include <JavaScriptCore/ThrowScope.h>
 #include <wtf/ASCIICType.h>
+#include <wtf/RunLoop.h>
 #include <wtf/UUID.h>
 
 namespace WebCore {
@@ -169,7 +171,8 @@ ExceptionOr<Ref<PaymentRequest>> PaymentRequest::create(Document& document, Vect
     if (methodData.isEmpty())
         return Exception { TypeError, ASCIILiteral("At least one payment method is required.") };
 
-    HashMap<String, String> serializedMethodData;
+    Vector<Method> serializedMethodData;
+    serializedMethodData.reserveInitialCapacity(methodData.size());
     for (auto& paymentMethod : methodData) {
         if (paymentMethod.supportedMethods.isEmpty())
             return Exception { TypeError, ASCIILiteral("supportedMethods must be specified.") };
@@ -181,7 +184,7 @@ ExceptionOr<Ref<PaymentRequest>> PaymentRequest::create(Document& document, Vect
             if (scope.exception())
                 return Exception { ExistingExceptionError };
         }
-        serializedMethodData.add(paymentMethod.supportedMethods, WTFMove(serializedData));
+        serializedMethodData.uncheckedAppend({ paymentMethod.supportedMethods, WTFMove(serializedData) });
     }
 
     auto exception = checkAndCanonicalizeTotal(details.total.amount);
@@ -240,7 +243,7 @@ ExceptionOr<Ref<PaymentRequest>> PaymentRequest::create(Document& document, Vect
     return adoptRef(*new PaymentRequest(document, WTFMove(options), WTFMove(details), WTFMove(serializedModifierData), WTFMove(serializedMethodData), WTFMove(selectedShippingOption)));
 }
 
-PaymentRequest::PaymentRequest(Document& document, PaymentOptions&& options, PaymentDetailsInit&& details, Vector<String>&& serializedModifierData, HashMap<String, String>&& serializedMethodData, String&& selectedShippingOption)
+PaymentRequest::PaymentRequest(Document& document, PaymentOptions&& options, PaymentDetailsInit&& details, Vector<String>&& serializedModifierData, Vector<Method>&& serializedMethodData, String&& selectedShippingOption)
     : ActiveDOMObject { &document }
     , m_options { WTFMove(options) }
     , m_details { WTFMove(details) }
@@ -255,21 +258,88 @@ PaymentRequest::~PaymentRequest()
 {
 }
 
-void PaymentRequest::show(DOMPromiseDeferred<IDLInterface<PaymentResponse>>&& promise)
+// https://www.w3.org/TR/payment-request/#show()-method
+void PaymentRequest::show(ShowPromise&& promise)
 {
-    promise.reject(Exception { NotSupportedError, ASCIILiteral("Not implemented") });
+    // FIXME: Reject promise with SecurityError if show() was not triggered by a user gesture.
+    // Find a way to do this without breaking the payment-request web platform tests.
+
+    if (m_state != State::Created) {
+        promise.reject(Exception { InvalidStateError });
+        return;
+    }
+
+    // FIXME: Reject promise with AbortError if PaymentCoordinator already has an active session.
+
+    m_state = State::Interactive;
+    ASSERT(!m_showPromise);
+    m_showPromise = WTFMove(promise);
+
+    // The spec requires these steps to be run after returning `promise` to the caller.
+    RunLoop::main().dispatch([this, protectedThis = makeRef(*this)] {
+        finishShowing();
+    });
+}
+
+void PaymentRequest::finishShowing()
+{
+    ASSERT(m_showPromise);
+
+    for (auto& paymentMethod : m_serializedMethodData) {
+        auto scope = DECLARE_THROW_SCOPE(scriptExecutionContext()->vm());
+        JSC::JSValue data = JSONParse(scriptExecutionContext()->execState(), paymentMethod.serializedData);
+        if (scope.exception()) {
+            m_showPromise->reject(Exception { ExistingExceptionError });
+            return;
+        }
+
+        // FIXME: If there is a payment handler that can support this payment method, allow it to
+        // convert the serialized data (propagating any exceptions that might be thrown) and add it
+        // to a list of handlers.
+        UNUSED(data);
+    }
+
+    // FIXME: If the list of handlers is non-empty, present the payment UI instead of rejecting.
+    m_showPromise->reject(Exception { NotSupportedError });
 }
 
-void PaymentRequest::abort(DOMPromiseDeferred<void>&& promise)
+// https://www.w3.org/TR/payment-request/#abort()-method
+ExceptionOr<void> PaymentRequest::abort(AbortPromise&& promise)
 {
-    promise.reject(Exception { NotSupportedError, ASCIILiteral("Not implemented") });
+    if (m_state != State::Interactive)
+        return Exception { InvalidStateError };
+
+    ASSERT(m_showPromise);
+    ASSERT(!m_abortPromise);
+    m_abortPromise = WTFMove(promise);
+
+    // The spec requires these steps to be run after returning `promise` to the caller.
+    RunLoop::main().dispatch([this, protectedThis = makeRef(*this)] {
+        m_state = State::Closed;
+        m_showPromise->reject(Exception { AbortError });
+        m_abortPromise->resolve();
+    });
+
+    return { };
 }
 
-void PaymentRequest::canMakePayment(DOMPromiseDeferred<IDLBoolean>&& promise)
+// https://www.w3.org/TR/payment-request/#canmakepayment()-method
+void PaymentRequest::canMakePayment(CanMakePaymentPromise&& promise)
 {
-    promise.reject(Exception { NotSupportedError, ASCIILiteral("Not implemented") });
+    if (m_state != State::Created) {
+        promise.reject(Exception { InvalidStateError });
+        return;
+    }
+
+    m_canMakePaymentPromise = WTFMove(promise);
+
+    // The spec requires these steps to be run after returning `promise` to the caller.
+    RunLoop::main().dispatch([this, protectedThis = makeRef(*this)] {
+        // FIXME: Resolve the promise with true if we can support any of the payment methods in m_serializedMethodData.
+        m_canMakePaymentPromise->resolve(false);
+    });
 }
-    
+
 const String& PaymentRequest::id() const
 {
     return m_details.id;
index d243815..3e2afd9 100644 (file)
@@ -44,12 +44,16 @@ struct PaymentMethodData;
 
 class PaymentRequest final : public RefCounted<PaymentRequest>, public ActiveDOMObject, public EventTargetWithInlineData {
 public:
+    using ShowPromise = DOMPromiseDeferred<IDLInterface<PaymentResponse>>;
+    using AbortPromise = DOMPromiseDeferred<void>;
+    using CanMakePaymentPromise = DOMPromiseDeferred<IDLBoolean>;
+
     static ExceptionOr<Ref<PaymentRequest>> create(Document&, Vector<PaymentMethodData>&&, PaymentDetailsInit&&, PaymentOptions&&);
     ~PaymentRequest();
 
-    void show(DOMPromiseDeferred<IDLInterface<PaymentResponse>>&&);
-    void abort(DOMPromiseDeferred<void>&&);
-    void canMakePayment(DOMPromiseDeferred<IDLBoolean>&&);
+    void show(ShowPromise&&);
+    ExceptionOr<void> abort(AbortPromise&&);
+    void canMakePayment(CanMakePaymentPromise&&);
 
     const String& id() const;
     PaymentAddress* shippingAddress() const { return m_shippingAddress.get(); }
@@ -60,7 +64,20 @@ public:
     using RefCounted<PaymentRequest>::deref;
 
 private:
-    PaymentRequest(Document&, PaymentOptions&&, PaymentDetailsInit&&, Vector<String>&& serializedModifierData, HashMap<String, String>&& serializedMethodData, String&& selectedShippingOption);
+    enum class State {
+        Created,
+        Interactive,
+        Closed,
+    };
+
+    struct Method {
+        String supportedMethods;
+        String serializedData;
+    };
+
+    PaymentRequest(Document&, PaymentOptions&&, PaymentDetailsInit&&, Vector<String>&& serializedModifierData, Vector<Method>&& serializedMethodData, String&& selectedShippingOption);
+
+    void finishShowing();
 
     // ActiveDOMObject
     const char* activeDOMObjectName() const final { return "PaymentRequest"; }
@@ -76,9 +93,13 @@ private:
     PaymentOptions m_options;
     PaymentDetailsInit m_details;
     Vector<String> m_serializedModifierData;
-    HashMap<String, String> m_serializedMethodData;
+    Vector<Method> m_serializedMethodData;
     String m_shippingOption;
     RefPtr<PaymentAddress> m_shippingAddress;
+    State m_state { State::Created };
+    std::optional<ShowPromise> m_showPromise;
+    std::optional<AbortPromise> m_abortPromise;
+    std::optional<CanMakePaymentPromise> m_canMakePaymentPromise;
 };
 
 } // namespace WebCore
index 78c40c0..9403415 100644 (file)
@@ -33,7 +33,7 @@
     SecureContext
 ] interface PaymentRequest : EventTarget {
     Promise<PaymentResponse> show();
-    Promise<void> abort();
+    [MayThrowException] Promise<void> abort();
     Promise<boolean> canMakePayment();
 
     readonly attribute DOMString id;