Changeset 221020 in webkit

Timestamp:

Aug 22, 2017 9:59:49 AM (7 years ago)

Author:

aestes@apple.com

Message:

[Payment Request] Implement error checking for show(), abort(), and canMakePayment()
​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:

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/ChangeLog (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https-expected.txt (modified) (1 diff)
• LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https-expected.txt (modified) (1 diff)
• LayoutTests/platform/ios-wk2/TestExpectations (modified) (1 diff)
• LayoutTests/platform/mac-wk2/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp (modified) (5 diffs)
• Source/WebCore/Modules/paymentrequest/PaymentRequest.h (modified) (3 diffs)
• Source/WebCore/Modules/paymentrequest/PaymentRequest.idl (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/imported/w3c/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-abort-method.https-expected.txt

@@ -1 @@
-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
 

trunk/LayoutTests/imported/w3c/web-platform-tests/payment-request/payment-request-show-method.https-expected.txt

@@ -1 @@
-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: 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
 
-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
-

trunk/LayoutTests/platform/ios-wk2/TestExpectations

@@ -35 +35 @@
 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 ]
 
 #//////////////////////////////////////////////////////////////////////////////////////////

trunk/LayoutTests/platform/mac-wk2/TestExpectations

@@ -38 +38 @@
 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 ]
 
 #//////////////////////////////////////////////////////////////////////////////////////////

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/Modules/paymentrequest/PaymentRequest.cpp

@@ -35 +35 @@
 #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>
 
@@ -170 +172 @@
         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())
@@ -182 +185 @@
                 return Exception { ExistingExceptionError };
         }
-        serializedMethodData.add(paymentMethod.supportedMethods, WTFMove(serializedData));
+        serializedMethodData.uncheckedAppend({ paymentMethod.supportedMethods, WTFMove(serializedData) });
     }
 
@@ -241 +244 @@
 }
 
-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) }
@@ -256 +259 @@
 }
 
-void PaymentRequest::show(DOMPromiseDeferred<IDLInterface<PaymentResponse>>&& promise)
-{
-    promise.reject(Exception { NotSupportedError, ASCIILiteral("Not implemented") });
-}
-
-void PaymentRequest::abort(DOMPromiseDeferred<void>&& promise)
-{
-    promise.reject(Exception { NotSupportedError, ASCIILiteral("Not implemented") });
-}
-
-void PaymentRequest::canMakePayment(DOMPromiseDeferred<IDLBoolean>&& promise)
-{
-    promise.reject(Exception { NotSupportedError, ASCIILiteral("Not implemented") });
-}
-    
+// https://www.w3.org/TR/payment-request/#show()-method
+void PaymentRequest::show(ShowPromise&& promise)
+{
+    // 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 });
+}
+
+// https://www.w3.org/TR/payment-request/#abort()-method
+ExceptionOr<void> PaymentRequest::abort(AbortPromise&& promise)
+{
+    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 { };
+}
+
+// https://www.w3.org/TR/payment-request/#canmakepayment()-method
+void PaymentRequest::canMakePayment(CanMakePaymentPromise&& promise)
+{
+    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
 {

trunk/Source/WebCore/Modules/paymentrequest/PaymentRequest.h

@@ -45 +45 @@
 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;
@@ -61 +65 @@
 
 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
@@ -77 +94 @@
     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;
 };
 

trunk/Source/WebCore/Modules/paymentrequest/PaymentRequest.idl

@@ -34 +34 @@
 ] interface PaymentRequest : EventTarget {
     Promise<PaymentResponse> show();
-    Promise<void> abort();
+    [MayThrowException] Promise<void> abort();
     Promise<boolean> canMakePayment();