PaymentRequest: Introduce PaymentMethodData.

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 075ba88ceae0eabe8e7eb55071a72e7edfbd0d9b..79d106312fea98478a660ab4695405a3971dfb5e 100644
--- a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
+++ b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
@@ -35,6 +35,8 @@ using blink::mojom::blink::PaymentDetails;
 using blink::mojom::blink::PaymentDetailsPtr;
 using blink::mojom::blink::PaymentItem;
 using blink::mojom::blink::PaymentItemPtr;
+using blink::mojom::blink::PaymentMethodData;
+using blink::mojom::blink::PaymentMethodDataPtr;
 using blink::mojom::blink::PaymentOptions;
 using blink::mojom::blink::PaymentOptionsPtr;
 using blink::mojom::blink::ShippingOption;
@@ -98,6 +100,18 @@ struct TypeConverter<PaymentOptionsPtr, blink::PaymentOptions> {
     }
 };
 
+template <>
+struct TypeConverter<PaymentMethodDataPtr, blink::PaymentMethodData> {

[Marijn Kruisselbrink, 2016/06/01 18:20:24]

I believe these days typemaps and StructTraits are preferred over TypeConverters
(https://www.chromium.org/developers/design-documents/mojo/type-mapping). That
way you could just pass blink::PaymentMethodData instances directly to/from the
mojom generated blink bindings. But other than for urls and origins there aren't
really any blink specific typemaps in the code-base yet. Also depending on what
happens with PaymentMethodData and the ScriptValue thing the typemaps would end
up being a bit different...

And maybe it makes more sense to just keep this as a TypeConverter for now, and
separately clean up all these TypeConverters to be StructTraits later.

[please use gerrit instead, 2016/06/01 19:20:21]

Let's make TypeConverters into typemaps or StrucTraits all in one go. I have
this on my schedule.

+    static PaymentMethodDataPtr Convert(const blink::PaymentMethodData& input)
+    {
+        PaymentMethodDataPtr output = PaymentMethodData::New();
+        output->supported_methods = Vector<WTF::String>(input.supportedMethods());
+        if (input.hasData() && !input.data().isEmpty())
+            output->data = blink::JSONObject::cast(blink::toJSONValue(input.data().context(), input.data().v8Value()))->toJSONString();
+        return output;
+    }
+};
+
 } // namespace mojo
 
 namespace blink {
@@ -174,21 +188,43 @@ void validatePaymentDetails(const PaymentDetails& details, ExceptionState& excep
     }
 }
 
-} // namespace
-
-PaymentRequest* PaymentRequest::create(ScriptState* scriptState, const Vector<String>& supportedMethods, const PaymentDetails& details, ExceptionState& exceptionState)
+void validatePaymentMethodData(const HeapVector<PaymentMethodData>& methodData, ExceptionState& exceptionState)
 {
-    return new PaymentRequest(scriptState, supportedMethods, details, PaymentOptions(), ScriptValue(), exceptionState);
+    if (methodData.isEmpty()) {
+        exceptionState.throwTypeError("Must specify at least one payment method identifier");
+        return;
+    }
+
+    for (const auto& md : methodData) {
+        if (md.supportedMethods().isEmpty()) {
+            exceptionState.throwTypeError("Must specify at least one payment method identifier");
+            return;
+        }
+
+        if (!md.data().isEmpty()) {
+            RefPtr<JSONValue> value = toJSONValue(md.data().context(), md.data().v8Value());
+            if (!value) {
+                exceptionState.throwTypeError("Unable to parse payment method specific data");
+                return;
+            }
+            if (!value->isNull() && value->getType() != JSONValue::TypeObject) {
+                exceptionState.throwTypeError("Data should be a JSON-serializable object");
+                return;
+            }
+        }
+    }
 }
 
-PaymentRequest* PaymentRequest::create(ScriptState* scriptState, const Vector<String>& supportedMethods, const PaymentDetails& details, const PaymentOptions& options, ExceptionState& exceptionState)
+} // namespace
+
+PaymentRequest* PaymentRequest::create(ScriptState* scriptState, const HeapVector<PaymentMethodData>& methodData, const PaymentDetails& details, ExceptionState& exceptionState)
 {
-    return new PaymentRequest(scriptState, supportedMethods, details, options, ScriptValue(), exceptionState);
+    return new PaymentRequest(scriptState, methodData, details, PaymentOptions(), exceptionState);
 }
 
-PaymentRequest* PaymentRequest::create(ScriptState* scriptState, const Vector<String>& supportedMethods, const PaymentDetails& details, const PaymentOptions& options, const ScriptValue& data, ExceptionState& exceptionState)
+PaymentRequest* PaymentRequest::create(ScriptState* scriptState, const HeapVector<PaymentMethodData>& methodData, const PaymentDetails& details, const PaymentOptions& options, ExceptionState& exceptionState)
 {
-    return new PaymentRequest(scriptState, supportedMethods, details, options, data, exceptionState);
+    return new PaymentRequest(scriptState, methodData, details, options, exceptionState);
 }
 
 PaymentRequest::~PaymentRequest()
@@ -207,8 +243,7 @@ ScriptPromise PaymentRequest::show(ScriptState* scriptState)
     scriptState->domWindow()->frame()->serviceRegistry()->connectToRemoteService(mojo::GetProxy(&m_paymentProvider));
     m_paymentProvider.set_connection_error_handler(createBaseCallback(bind(&PaymentRequest::OnError, WeakPersistentThisPointer<PaymentRequest>(this))));
     m_paymentProvider->SetClient(m_clientBinding.CreateInterfacePtrAndBind());
-    m_paymentProvider->Show(std::move(m_supportedMethods), mojom::blink::PaymentDetails::From(m_details), mojom::blink::PaymentOptions::From(m_options), m_stringifiedData.isNull() ? "" : m_stringifiedData);
-
+    m_paymentProvider->Show(mojo::WTFArray<blink::mojom::blink::PaymentMethodDataPtr>::From(m_methodData), mojom::blink::PaymentDetails::From(m_details), mojom::blink::PaymentOptions::From(m_options));
     m_showResolver = ScriptPromiseResolver::create(scriptState);
     return m_showResolver->promise();
 }
@@ -286,6 +321,7 @@ void PaymentRequest::onUpdatePaymentDetailsFailure(const ScriptValue& error)
 
 DEFINE_TRACE(PaymentRequest)
 {
+    visitor->trace(m_methodData);
     visitor->trace(m_details);
     visitor->trace(m_options);
     visitor->trace(m_shippingAddress);
@@ -295,12 +331,19 @@ DEFINE_TRACE(PaymentRequest)
     ContextLifecycleObserver::trace(visitor);
 }
 
-PaymentRequest::PaymentRequest(ScriptState* scriptState, const Vector<String>& supportedMethods, const PaymentDetails& details, const PaymentOptions& options, const ScriptValue& data, ExceptionState& exceptionState)
+PaymentRequest::PaymentRequest(ScriptState* scriptState, const HeapVector<PaymentMethodData>& methodData, const PaymentDetails& details, const PaymentOptions& options, ExceptionState& exceptionState)
     : ContextLifecycleObserver(scriptState->getExecutionContext())
     , ActiveScriptWrappable(this)
     , m_options(options)
     , m_clientBinding(this)
 {
+    validatePaymentMethodData(methodData, exceptionState);
+    if (exceptionState.hadException())
+        return;
+    m_methodData = methodData;
+
+    // TODO(rouslan): Also check for a top-level browsing context.
+    // https://github.com/w3c/browser-payment-api/issues/2

[please use gerrit instead, 2016/06/01 19:20:21]

Bad merge? This comment should be gone now.

     if (!scriptState->getExecutionContext()->isSecureContext()) {
         exceptionState.throwSecurityError("Must be in a secure context");
         return;
@@ -311,45 +354,11 @@ PaymentRequest::PaymentRequest(ScriptState* scriptState, const Vector<String>& s
         return;
     }
 
-    if (supportedMethods.isEmpty()) {
-        exceptionState.throwTypeError("Must specify at least one payment method identifier");
-        return;
-    }
-    m_supportedMethods = supportedMethods;
-
     validatePaymentDetails(details, exceptionState);
     if (exceptionState.hadException())
         return;
     m_details = details;
 
-    if (!data.isEmpty()) {
-        RefPtr<JSONValue> value = toJSONValue(data.context(), data.v8Value());
-        if (!value) {
-            exceptionState.throwTypeError("Unable to parse payment method specific data");
-            return;
-        }
-        if (!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");
-                    return;
-                }
-                if (paymentMethodSpecificKeyValue.value->getType() != JSONValue::TypeObject) {
-                    exceptionState.throwTypeError("Data for '" + paymentMethodSpecificKeyValue.key + "' should be a JSON-serializable object");
-                    return;
-                }
-            }
-
-            m_stringifiedData = jsonData->toJSONString();
-        }
-    }
-
     // Set the currently selected option if only one option is passed and shipping is requested.
     if (options.requestShipping() && details.hasShippingOptions() && details.shippingOptions().size() == 1)
         m_shippingOption = details.shippingOptions().begin()->id();