PaymentRequest Mojo bindings.

third_party/WebKit/Source/modules/payments/PaymentRequest.cpp

Index: third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
diff --git a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
index fe3ab4642012884480a3a1fe8edd66b0e6bdd892..8b6f819d1746c9b18e37ca360f02c151611dfc01 100644
--- a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
+++ b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
@@ -6,13 +6,140 @@
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/JSONValuesForV8.h"
+#include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
+#include "core/EventTypeNames.h"
 #include "core/dom/DOMException.h"
 #include "core/dom/ExceptionCode.h"
+#include "core/events/Event.h"
+#include "core/events/EventQueue.h"
 #include "modules/EventTargetModulesNames.h"
+#include "modules/payments/PaymentItem.h"
+#include "modules/payments/PaymentResponse.h"
+#include "modules/payments/PaymentsValidators.h"
 #include "modules/payments/ShippingAddress.h"
+#include "modules/payments/ShippingOption.h"
+#include "mojo/public/cpp/bindings/interface_request.h"
+#include "public/platform/Platform.h"
+#include <utility>
+
+namespace mojo {
+
+template <>
+struct TypeConverter<mojom::wtf::CurrencyAmountPtr, blink::CurrencyAmount> {
+    static mojom::wtf::CurrencyAmountPtr Convert(const blink::CurrencyAmount& input)
+    {
+        mojom::wtf::CurrencyAmountPtr output = mojom::wtf::CurrencyAmount::New();
+        output->currency_code = input.currencyCode();
+        output->value = input.value();
+        return output;
+    }
+};
+
+template <>
+struct TypeConverter<mojom::wtf::PaymentItemPtr, blink::PaymentItem> {
+    static mojom::wtf::PaymentItemPtr Convert(const blink::PaymentItem& input)
+    {
+        mojom::wtf::PaymentItemPtr output = mojom::wtf::PaymentItem::New();
+        output->id = input.id();
+        output->label = input.label();
+        output->amount = mojom::wtf::CurrencyAmount::From(input.amount());
+        return output;
+    }
+};
+
+template <>
+struct TypeConverter<mojom::wtf::ShippingOptionPtr, blink::ShippingOption> {
+    static mojom::wtf::ShippingOptionPtr Convert(const blink::ShippingOption& input)
+    {
+        mojom::wtf::ShippingOptionPtr output = mojom::wtf::ShippingOption::New();
+        output->id = input.id();
+        output->label = input.label();
+        output->amount = mojom::wtf::CurrencyAmount::From(input.amount());
+        return output;
+    }
+};
+
+template <>
+struct TypeConverter<mojom::wtf::PaymentDetailsPtr, blink::PaymentDetails> {
+    static mojom::wtf::PaymentDetailsPtr Convert(const blink::PaymentDetails& input)
+    {
+        mojom::wtf::PaymentDetailsPtr output = mojom::wtf::PaymentDetails::New();
+        output->items = mojo::WTFArray<mojom::wtf::PaymentItemPtr>::New(input.items().size());
+        for (size_t i = 0; i < input.items().size(); ++i) {
+            output->items[i] = mojom::wtf::PaymentItem::From(input.items()[i]);
+        }
+        output->shipping_options = mojo::WTFArray<mojom::wtf::ShippingOptionPtr>::New(input.hasShippingOptions() ? input.shippingOptions().size() : 0);
+        if (input.hasShippingOptions()) {
+            for (size_t i = 0; i < input.shippingOptions().size(); ++i) {
+                output->shipping_options[i] = mojom::wtf::ShippingOption::From(input.shippingOptions()[i]);

[esprehn, 2016/03/25 23:40:11]

if we're going to be writing a lot of these copy ::From() loops we probably need
a templateized helper.

[Marijn Kruisselbrink, 2016/03/25 23:59:39]

See also my earlier comment; mojo::Array has these templateized helpers already,
it seems to just be an oversight that mojo::WTFArray doesn't.

[please use gerrit instead, 2016/03/29 22:15:45]

Created a helper.

+            }
+        }
+        return output;
+    }
+};
+
+template <>
+struct TypeConverter<mojom::wtf::PaymentOptionsPtr, blink::PaymentOptions> {
+    static mojom::wtf::PaymentOptionsPtr Convert(const blink::PaymentOptions& input)
+    {
+        mojom::wtf::PaymentOptionsPtr output = mojom::wtf::PaymentOptions::New();
+        output->request_shipping = input.requestShipping();
+        return output;
+    }
+};
+
+} // namespace mojo
 
 namespace blink {
+namespace {
+
+// Validates ShippingOption and PaymentItem dictionaries, which happen to have identical fields.
+template <typename T>
+bool areValidItems(HeapVector<T> items, ExceptionState& exceptionState)

[esprehn, 2016/03/25 23:40:11]

hmm what two things have the exact same fields below that require using
templates?

[please use gerrit instead, 2016/03/29 22:15:45]

See the comment for this function: "ShippingOption and PaymentItem dictionaries,
which happen to have identical fields."

+{
+    String errorMessage;
+    for (const auto& item : items) {
+        if (!item.hasId()) {
+            exceptionState.throwTypeError("Item id required");
+            return false;
+        }
+
+        if (!item.hasLabel()) {
+            exceptionState.throwTypeError("Item label required");
+            return false;
+        }
+
+        if (!item.hasAmount()) {
+            exceptionState.throwTypeError("Currency amount required");
+            return false;
+        }
+
+        if (!item.amount().hasCurrencyCode()) {
+            exceptionState.throwTypeError("Currency code required");
+            return false;
+        }
+
+        if (!item.amount().hasValue()) {
+            exceptionState.throwTypeError("Currency value required");
+            return false;
+        }
+
+        if (!isValidCurrencyCodeFormat(item.amount().currencyCode(), &errorMessage)) {
+            exceptionState.throwTypeError(errorMessage);
+            return false;
+        }
+
+        if (!isValidAmountFormat(item.amount().value(), &errorMessage)) {
+            exceptionState.throwTypeError(errorMessage);
+            return false;
+        }
+    }
+
+    return true;

[esprehn, 2016/03/25 23:40:11]

you actually don't need the boolean, you can just check hadException(), then you
can join all the if statements in here to be:

if (...) {

} else if (...) {

} ...

and once you hit one you'll then fall out of the function and the caller checks
hadException(). 

Then you name this function something like validateFoo()

you could even just return exceptionState.hadException() on the last line if you
still want the boolean.

[Marijn Kruisselbrink, 2016/03/25 23:59:39]

of course you'd still need to return as soon as you come across an individual
invalid item. Otherwise ExceptionState will end up holding the last thrown error
rather than the first.

[please use gerrit instead, 2016/03/29 22:15:45]

Done. Note that "else if" statements are not suitable, because I am checking
"item.hasAmount()" not "item.hasValidAmount()". Having an amount does not
preclude checking the label as well, for example.

+}
+
+} // namespace
 
 // static
 PaymentRequest* PaymentRequest::create(ScriptState* scriptState, const Vector<String>& supportedMethods, const PaymentDetails& details, ExceptionState& exceptionState)
@@ -38,11 +165,27 @@ PaymentRequest::~PaymentRequest()
 
 ScriptPromise PaymentRequest::show(ScriptState* scriptState)
 {
-    return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(NotSupportedError, "Not implemented."));
+    if (m_showResolver)
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Already called show() once"));
+
+    DCHECK(!m_paymentProvider.is_bound());
+    blink::Platform::current()->connectToRemoteService(mojo::GetProxy(&m_paymentProvider));
+    // TODO(rouslan): Handle connection errors.

[esprehn, 2016/03/25 23:40:11]

What's the correct way to handle this? Often todo's like this end up sticking
around for very long periods of time.

[please use gerrit instead, 2016/03/29 22:15:45]

Handling it now. It was not very well documented, but reading the source code
helped me out here.

+    m_paymentProvider->SetClient(m_clientBinding.CreateInterfacePtrAndBind());
+    m_paymentProvider->Show(std::move(m_supportedMethods), mojom::wtf::PaymentDetails::From(m_details), mojom::wtf::PaymentOptions::From(m_options), m_stringifiedData.isNull() ? "" : m_stringifiedData);
+
+    m_showResolver = ScriptPromiseResolver::create(scriptState);
+    return m_showResolver->promise();
 }
 
-void PaymentRequest::abort()
+void PaymentRequest::abort(ExceptionState& exceptionState)
 {
+    if (!m_showResolver) {
+        exceptionState.throwDOMException(InvalidStateError, "Never called show(), so nothing to abort");
+        return;
+    }
+
+    m_paymentProvider->Abort();
 }
 
 const AtomicString& PaymentRequest::interfaceName() const
@@ -55,11 +198,24 @@ ExecutionContext* PaymentRequest::getExecutionContext() const
     return m_scriptState->getExecutionContext();
 }
 
+ScriptPromise PaymentRequest::complete(ScriptState* scriptState, bool success)
+{
+    if (m_completeResolver)
+        return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(InvalidStateError, "Already called complete() once"));
+
+    m_completeResolver = ScriptPromiseResolver::create(scriptState);
+    m_paymentProvider->Complete(success);
+
+    return m_completeResolver->promise();
+}
+
 DEFINE_TRACE(PaymentRequest)
 {
     visitor->trace(m_details);
     visitor->trace(m_options);
     visitor->trace(m_shippingAddress);
+    visitor->trace(m_showResolver);
+    visitor->trace(m_completeResolver);
     RefCountedGarbageCollectedEventTargetWithInlineData<PaymentRequest>::trace(visitor);
 }
 
@@ -68,12 +224,125 @@ PaymentRequest::PaymentRequest(ScriptState* scriptState, const Vector<String>& s
     , m_supportedMethods(supportedMethods)
     , m_details(details)
     , m_options(options)
+    , m_clientBinding(this)
 {
+    // TODO(rouslan): Also check for a top-level browsing context.
+    // https://github.com/w3c/browser-payment-api/issues/2
+    if (!scriptState->getExecutionContext()->isSecureContext()) {
+        exceptionState.throwSecurityError("Must be in a secure context");
+        return;
+    }
+
+    if (supportedMethods.isEmpty()) {
+        exceptionState.throwTypeError("Must specify at least one payment method identifier");
+        return;
+    }
+
+    if (!details.hasItems()) {
+        exceptionState.throwTypeError("Must specify items");
+        return;
+    }
+
+    if (details.items().isEmpty()) {
+        exceptionState.throwTypeError("Must specify at least one item");
+        return;
+    }
+
+    if (!areValidItems(details.items(), exceptionState))
+        return;
+
+    if (details.hasShippingOptions() && !areValidItems(details.shippingOptions(), exceptionState))
+        return;
+
     if (!data.isEmpty()) {
         RefPtr<JSONValue> value = toJSONValue(data.context(), data.v8Value());

[esprehn, 2016/03/25 23:40:11]

this only returns null if maxDepth is reached btw

[rwlbuis, 2016/03/29 18:26:43]

And I think there is a problem with the way it is not handled when it is null. I
added an infinite JSON object to payment-request-interface.html and it is not
throwing:

['Infinite JSON value for one of the payment method specific data pieces should
throw', null, function() {
    var data = { foo: false }
    data.foo = data;
    new PaymentRequest(['foo'], buildDetails(), {}, data)
}],

[please use gerrit instead, 2016/03/29 22:15:45]

Added a layout test and throwing on infinite json, which is detected here as a
null |value|.

-        if (value && value->getType() == JSONValue::TypeObject)
-            m_stringifiedData = JSONObject::cast(value)->toJSONString();
+        if (value && !value->isNull()) {
+            if (value->getType() != JSONValue::TypeObject) {
+                exceptionState.throwTypeError("Payment method specific data should be a JSON-serializable object");
+                return;
+            }
+
+            RefPtr<JSONObject> jsonData = JSONObject::cast(value);
+            for (const auto& paymentMethodSpecificKeyValue : *jsonData) {
+                if (!supportedMethods.contains(paymentMethodSpecificKeyValue.key)) {
+                    exceptionState.throwTypeError("'" + paymentMethodSpecificKeyValue.key + "' should match one of the payment method identifiers");

[esprehn, 2016/03/25 23:40:11]

hmm, if we have real requirements on the |data| why isn't it a dictionary?

[Marijn Kruisselbrink, 2016/03/25 23:59:40]

a WebIDL dictionary has a fixed, hardcoded set of keys. The |data| here doesn't
have a fixed set of allowed keys. It just has the requirement that its keys
match values in the supportedMethods parameter (not sure why the spec even has
that separate supportedMethods parameter though, it seems like it could just
reuse the details object to also get the supported methods).

[please use gerrit instead, 2016/03/29 22:15:45]

Correct.
I think the idea is that you can get credit cards by saying
PaymentRequest(["visa", "amex", mastercard"]) as opposed to
PaymentRequest({"visa": null, "amex": null, "mastercard": null}). That's when
you don't have any special data to pass into the API.

+                    return;
+                }
+                if (!paymentMethodSpecificKeyValue.value || paymentMethodSpecificKeyValue.value->isNull() || paymentMethodSpecificKeyValue.value->getType() != JSONValue::TypeObject) {

[esprehn, 2016/03/25 23:40:11]

ditto, all these null checks are kind of suspect. Should toJSONValue just return
JSONValue::null() instead?

[rwlbuis, 2016/03/29 19:30:46]

Independent of this, note that getType() != TypeObject also is false when
JSONValue is of null type, so I think you can skip the isNull check with the
code as-is.

[please use gerrit instead, 2016/03/29 22:15:45]

Done.

[please use gerrit instead, 2016/03/29 22:15:45]

Removed the "!paymentMethodSpecificKeyValue.value" check, because toJSONValue()
checks this for us already.

Also removed the "paymentMethodSpecificKeyValue.value->isNull()" check, because
that's covered by the "paymentMethodSpecificKeyValue.value->getType() !=
JSONValue::TypeObject" check already.

+                    exceptionState.throwTypeError("Data for '" + paymentMethodSpecificKeyValue.key + "' should be a JSON-serializable object");
+                    return;
+                }
+            }
+
+            m_stringifiedData = jsonData->toJSONString();

[esprehn, 2016/03/25 23:40:11]

why can't we reuse the data? This code is doing string -> json -> string

[Marijn Kruisselbrink, 2016/03/25 23:59:40]

Where is it doing that? It starts as ScriptValue, and is converted to JSONObject
to get a string out of it. I suppose arguably it could do the validation on the
ScriptValue directly rather than converting to a JSONObject first, but it isn't
doing any extra conversions.

[please use gerrit instead, 2016/03/29 22:15:45]

Acknowledged.

+        }
+    }
+
+    if (details.hasShippingOptions() && details.shippingOptions().size() == 1)

[esprehn, 2016/03/25 23:40:11]

what happens if you specified more than one? It seems like we just ignore it
entirely. Is that right?

[Marijn Kruisselbrink, 2016/03/25 23:59:40]

I was confused here for a bit too, all the options are stored in m_details. This
just sets the currently selected option if only one option was passed.

[please use gerrit instead, 2016/03/29 22:15:45]

If multiple options are available, the user has to pick one in browser UI. Then
onShippingOptionChange() will set the value of this member variable.
Correct. Added a comment to that respect.

+        m_shippingOption = details.shippingOptions()[0].id();
+}
+
+void PaymentRequest::OnShippingAddressChange(mojom::wtf::ShippingAddressPtr address)
+{
+    DCHECK(m_showResolver);
+    DCHECK(!m_completeResolver);
+
+    // TODO(rouslan): Should the merchant website be notified of invalid shipping address
+    // from the browser or the payment app?
+    String errorMessage;
+    if (!isValidRegionCodeFormat(address->region_code, &errorMessage)
+        || !isValidLanguageCodeFormat(address->language_code, &errorMessage)
+        || !isValidScriptCodeFormat(address->script_code, &errorMessage)) {
+        m_showResolver->reject(DOMException::create(SyntaxError, errorMessage));
+        return;
     }
+
+    if (address->language_code.isEmpty() && !address->script_code.isEmpty()) {
+        m_showResolver->reject(DOMException::create(SyntaxError, "If language code is empty, then script code should also be empty"));
+        return;
+    }
+
+    m_shippingAddress = new ShippingAddress(std::move(address));
+    RefPtrWillBeRawPtr<Event> event = Event::create(EventTypeNames::shippingaddresschange);
+    event->setTarget(this);
+    getExecutionContext()->getEventQueue()->enqueueEvent(event);
+}
+
+void PaymentRequest::OnShippingOptionChange(const String& shippingOptionId)
+{
+    DCHECK(m_showResolver);
+    DCHECK(!m_completeResolver);
+    m_shippingOption = shippingOptionId;
+    RefPtrWillBeRawPtr<Event> event = Event::create(EventTypeNames::shippingoptionchange);
+    event->setTarget(this);
+    getExecutionContext()->getEventQueue()->enqueueEvent(event);
+}
+
+void PaymentRequest::OnPaymentResponse(mojom::wtf::PaymentResponsePtr response)
+{
+    DCHECK(m_showResolver);
+    DCHECK(!m_completeResolver);
+    m_showResolver->resolve(new PaymentResponse(std::move(response), this));
+}
+
+void PaymentRequest::OnError()
+{
+    if (m_completeResolver) {
+        m_completeResolver->reject(DOMException::create(SyntaxError, "Request cancelled"));
+        return;
+    }
+    DCHECK(m_showResolver);
+    m_showResolver->reject(DOMException::create(SyntaxError, "Request cancelled"));
+    m_clientBinding.Close();
+    m_paymentProvider.reset();
+}
+
+void PaymentRequest::OnComplete()
+{
+    DCHECK(m_completeResolver);
+    m_completeResolver->resolve();
+    m_clientBinding.Close();
+    m_paymentProvider.reset();
 }
 
 } // namespace blink