Add data parameter to payment details modifier.

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 10dc9a605e61d45e4a642548af1f2ae46f80e0c5..603cf1f0f4691bfee30409e7427d725e30ba4674 100644
--- a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
+++ b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
@@ -35,26 +35,20 @@
 #include "public/platform/Platform.h"
 #include "public/platform/WebTraceLocation.h"
 #include "wtf/HashSet.h"
+#include <stddef.h>
 #include <utility>
 
 using payments::mojom::blink::ActivePaymentQueryResult;
 using payments::mojom::blink::PaymentAddressPtr;
 using payments::mojom::blink::PaymentCurrencyAmount;
 using payments::mojom::blink::PaymentCurrencyAmountPtr;
-using payments::mojom::blink::PaymentDetails;
-using payments::mojom::blink::PaymentDetailsModifier;
 using payments::mojom::blink::PaymentDetailsModifierPtr;
 using payments::mojom::blink::PaymentDetailsPtr;
-using payments::mojom::blink::PaymentDetailsPtr;
 using payments::mojom::blink::PaymentErrorReason;
-using payments::mojom::blink::PaymentItem;
 using payments::mojom::blink::PaymentItemPtr;
-using payments::mojom::blink::PaymentMethodData;
 using payments::mojom::blink::PaymentMethodDataPtr;
-using payments::mojom::blink::PaymentOptions;
 using payments::mojom::blink::PaymentOptionsPtr;
 using payments::mojom::blink::PaymentResponsePtr;
-using payments::mojom::blink::PaymentShippingOption;
 using payments::mojom::blink::PaymentShippingOptionPtr;
 using payments::mojom::blink::PaymentShippingType;
 
@@ -76,7 +70,7 @@ struct TypeConverter<PaymentCurrencyAmountPtr, blink::PaymentCurrencyAmount> {
 template <>
 struct TypeConverter<PaymentItemPtr, blink::PaymentItem> {
   static PaymentItemPtr Convert(const blink::PaymentItem& input) {
-    PaymentItemPtr output = PaymentItem::New();
+    PaymentItemPtr output = payments::mojom::blink::PaymentItem::New();
     output->label = input.label();
     output->amount = PaymentCurrencyAmount::From(input.amount());
     output->pending = input.pending();
@@ -88,7 +82,8 @@ template <>
 struct TypeConverter<PaymentShippingOptionPtr, blink::PaymentShippingOption> {
   static PaymentShippingOptionPtr Convert(
       const blink::PaymentShippingOption& input) {
-    PaymentShippingOptionPtr output = PaymentShippingOption::New();
+    PaymentShippingOptionPtr output =
+        payments::mojom::blink::PaymentShippingOption::New();
     output->id = input.id();
     output->label = input.label();
     output->amount = PaymentCurrencyAmount::From(input.amount());
@@ -98,65 +93,9 @@ struct TypeConverter<PaymentShippingOptionPtr, blink::PaymentShippingOption> {
 };
 
 template <>
-struct TypeConverter<PaymentDetailsModifierPtr, blink::PaymentDetailsModifier> {
-  static PaymentDetailsModifierPtr Convert(
-      const blink::PaymentDetailsModifier& input) {
-    PaymentDetailsModifierPtr output = PaymentDetailsModifier::New();
-    output->supported_methods = input.supportedMethods();
-
-    if (input.hasTotal())
-      output->total = PaymentItem::From(input.total());
-
-    if (input.hasAdditionalDisplayItems()) {
-      for (size_t i = 0; i < input.additionalDisplayItems().size(); ++i) {
-        output->additional_display_items.append(
-            PaymentItem::From(input.additionalDisplayItems()[i]));
-      }
-    }
-    return output;
-  }
-};
-
-template <>
-struct TypeConverter<PaymentDetailsPtr, blink::PaymentDetails> {
-  static PaymentDetailsPtr Convert(const blink::PaymentDetails& input) {
-    PaymentDetailsPtr output = PaymentDetails::New();
-    output->total = PaymentItem::From(input.total());
-
-    if (input.hasDisplayItems()) {
-      for (size_t i = 0; i < input.displayItems().size(); ++i) {
-        output->display_items.append(
-            PaymentItem::From(input.displayItems()[i]));
-      }
-    }
-
-    if (input.hasShippingOptions()) {
-      for (size_t i = 0; i < input.shippingOptions().size(); ++i) {
-        output->shipping_options.append(
-            PaymentShippingOption::From(input.shippingOptions()[i]));
-      }
-    }
-
-    if (input.hasModifiers()) {
-      for (size_t i = 0; i < input.modifiers().size(); ++i) {
-        output->modifiers.append(
-            PaymentDetailsModifier::From(input.modifiers()[i]));
-      }
-    }
-
-    if (input.hasError())
-      output->error = input.error();
-    else
-      output->error = emptyString();
-
-    return output;
-  }
-};
-
-template <>
 struct TypeConverter<PaymentOptionsPtr, blink::PaymentOptions> {
   static PaymentOptionsPtr Convert(const blink::PaymentOptions& input) {
-    PaymentOptionsPtr output = PaymentOptions::New();
+    PaymentOptionsPtr output = payments::mojom::blink::PaymentOptions::New();
     output->request_payer_name = input.requestPayerName();
     output->request_payer_email = input.requestPayerEmail();
     output->request_payer_phone = input.requestPayerPhone();
@@ -240,128 +179,71 @@ void validateShippingOptionOrPaymentItem(const T& item,
   }
 }
 
-void validateDisplayItems(const HeapVector<PaymentItem>& items,
-                          ExceptionState& exceptionState) {
-  for (const auto& item : items) {
+void validateAndConvertDisplayItems(const HeapVector<PaymentItem>& input,
+                                    Vector<PaymentItemPtr>& output,
+                                    ExceptionState& exceptionState) {
+  output.resize(input.size());
+  for (size_t i = 0; i < input.size(); ++i) {
+    const PaymentItem& item = input[i];
     validateShippingOptionOrPaymentItem(item, exceptionState);
     if (exceptionState.hadException())
       return;
+    output[i] = payments::mojom::blink::PaymentItem::From(item);
   }
 }
 
-// Returns false if |options| should be ignored, even if an exception was not
-// thrown. TODO(rouslan): Clear shipping options instead of ignoring them when
-// http://crbug.com/601193 is fixed.
-bool validateShippingOptions(const HeapVector<PaymentShippingOption>& options,
-                             ExceptionState& exceptionState) {
+// Validates and converts |input| shipping options into |output|. Throws an
+// exception if the data is not valid, except for duplicate identifiers, which
+// returns an empty |output| instead of throwing an exception. There's no need
+// to clear |output| when an exception is thrown, because the caller takes care
+// of deleting |output|.
+void validateAndConvertShippingOptions(
+    const HeapVector<PaymentShippingOption>& input,
+    Vector<PaymentShippingOptionPtr>& output,
+    ExceptionState& exceptionState) {
+  output.resize(input.size());
   HashSet<String> uniqueIds;
-  for (const auto& option : options) {
+  for (size_t i = 0; i < input.size(); ++i) {
+    const PaymentShippingOption& option = input[i];
     if (!option.hasId() || option.id().isEmpty()) {
       exceptionState.throwTypeError("ShippingOption id required");
-      return false;
+      return;
     }
 
-    if (uniqueIds.contains(option.id()))
-      return false;
+    if (uniqueIds.contains(option.id())) {
+      // Clear |output| instead of throwing an exception.
+      output.clear();
+      return;
+    }
 
     uniqueIds.add(option.id());
 
     validateShippingOptionOrPaymentItem(option, exceptionState);
     if (exceptionState.hadException())
-      return false;
-  }
-
-  return true;
-}
-
-void validatePaymentDetailsModifiers(
-    const HeapVector<PaymentDetailsModifier>& modifiers,
-    ExceptionState& exceptionState) {
-  if (modifiers.isEmpty()) {
-    exceptionState.throwTypeError(
-        "Must specify at least one payment details modifier");
-    return;
-  }
-
-  for (const auto& modifier : modifiers) {
-    if (modifier.supportedMethods().isEmpty()) {
-      exceptionState.throwTypeError(
-          "Must specify at least one payment method identifier");
       return;
-    }
-
-    if (modifier.hasTotal()) {
-      validateShippingOptionOrPaymentItem(modifier.total(), exceptionState);
-      if (exceptionState.hadException())
-        return;
 
-      if (modifier.total().amount().value()[0] == '-') {
-        exceptionState.throwTypeError(
-            "Total amount value should be non-negative");
-        return;
-      }
-    }
-
-    if (modifier.hasAdditionalDisplayItems()) {
-      validateDisplayItems(modifier.additionalDisplayItems(), exceptionState);
-      if (exceptionState.hadException())
-        return;
-    }
+    output[i] = payments::mojom::blink::PaymentShippingOption::From(input[i]);
   }
 }
 
-// Returns false if the shipping options should be ignored without throwing an
-// exception.
-bool validatePaymentDetails(const PaymentDetails& details,
-                            ExceptionState& exceptionState) {
-  bool keepShippingOptions = true;
-  if (!details.hasTotal()) {
-    exceptionState.throwTypeError("Must specify total");
-    return keepShippingOptions;
-  }
-
-  validateShippingOptionOrPaymentItem(details.total(), exceptionState);
+void validateAndConvertTotal(const PaymentItem& input,
+                             PaymentItemPtr& output,
+                             ExceptionState& exceptionState) {
+  validateShippingOptionOrPaymentItem(input, exceptionState);
   if (exceptionState.hadException())
-    return keepShippingOptions;
+    return;
 
-  if (details.total().amount().value()[0] == '-') {
+  if (input.amount().value()[0] == '-') {
     exceptionState.throwTypeError("Total amount value should be non-negative");
-    return keepShippingOptions;
-  }
-
-  if (details.hasDisplayItems()) {
-    validateDisplayItems(details.displayItems(), exceptionState);
-    if (exceptionState.hadException())
-      return keepShippingOptions;
-  }
-
-  if (details.hasShippingOptions()) {
-    keepShippingOptions =
-        validateShippingOptions(details.shippingOptions(), exceptionState);
-
-    if (exceptionState.hadException())
-      return keepShippingOptions;
-  }
-
-  if (details.hasModifiers()) {
-    validatePaymentDetailsModifiers(details.modifiers(), exceptionState);
-    if (exceptionState.hadException())
-      return keepShippingOptions;
-  }
-
-  String errorMessage;
-  if (!PaymentsValidators::isValidErrorMsgFormat(details.error(),
-                                                 &errorMessage)) {
-    exceptionState.throwTypeError(errorMessage);
+    return;
   }
 
-  return keepShippingOptions;
+  output = payments::mojom::blink::PaymentItem::From(input);
 }
 
-void maybeSetAndroidPayMethodData(
-    const ScriptValue& input,
-    payments::mojom::blink::PaymentMethodDataPtr& output,
-    ExceptionState& exceptionState) {
+void maybeSetAndroidPayMethodData(const ScriptValue& input,
+                                  PaymentMethodDataPtr& output,
+                                  ExceptionState& exceptionState) {
   AndroidPayMethodData androidPay;
   TrackExceptionState trackExceptionState;
   V8AndroidPayMethodData::toImpl(input.isolate(), input.v8Value(), androidPay,
@@ -428,70 +310,168 @@ void maybeSetAndroidPayMethodData(
   }
 }
 
-void validateAndConvertPaymentMethodData(
-    const HeapVector<PaymentMethodData>& input,
-    Vector<payments::mojom::blink::PaymentMethodDataPtr>& output,
+void stringifyAndParseMethodSpecificData(const ScriptValue& input,
+                                         PaymentMethodDataPtr& output,
+                                         ExceptionState& exceptionState) {
+  DCHECK(!input.isEmpty());
+  if (!input.v8Value()->IsObject() || input.v8Value()->IsArray()) {
+    exceptionState.throwTypeError("Data should be a JSON-serializable object");
+    return;
+  }
+
+  v8::Local<v8::String> value;
+  if (!v8::JSON::Stringify(input.context(), input.v8Value().As<v8::Object>())
+           .ToLocal(&value)) {
+    exceptionState.throwTypeError(
+        "Unable to parse payment method specific data");
+    return;
+  }
+
+  output->stringified_data =
+      v8StringToWebCoreString<String>(value, DoNotExternalize);
+  maybeSetAndroidPayMethodData(input, output, exceptionState);
+}
+
+void validateAndConvertPaymentDetailsModifiers(
+    const HeapVector<PaymentDetailsModifier>& input,
+    Vector<PaymentDetailsModifierPtr>& output,
     ExceptionState& exceptionState) {
   if (input.isEmpty()) {
     exceptionState.throwTypeError(
-        "Must specify at least one payment method identifier");
+        "Must specify at least one payment details modifier");
     return;
   }
 
   output.resize(input.size());
   for (size_t i = 0; i < input.size(); ++i) {
-    const auto& paymentMethodData = input[i];
-    if (paymentMethodData.supportedMethods().isEmpty()) {
+    const PaymentDetailsModifier& modifier = input[i];
+    output[i] = payments::mojom::blink::PaymentDetailsModifier::New();
+    if (modifier.hasTotal()) {
+      validateAndConvertTotal(modifier.total(), output[i]->total,
+                              exceptionState);
+      if (exceptionState.hadException())
+        return;
+    }
+
+    if (modifier.hasAdditionalDisplayItems()) {
+      validateAndConvertDisplayItems(modifier.additionalDisplayItems(),
+                                     output[i]->additional_display_items,
+                                     exceptionState);
+      if (exceptionState.hadException())
+        return;
+    }
+
+    if (modifier.supportedMethods().isEmpty()) {
       exceptionState.throwTypeError(
           "Must specify at least one payment method identifier");
       return;
     }
 
-    String stringifiedData = "";
-    if (paymentMethodData.hasData() && !paymentMethodData.data().isEmpty()) {
-      if (!paymentMethodData.data().v8Value()->IsObject() ||
-          paymentMethodData.data().v8Value()->IsArray()) {
-        exceptionState.throwTypeError(
-            "Data should be a JSON-serializable object");
-        return;
-      }
+    output[i]->method_data = payments::mojom::blink::PaymentMethodData::New();
+    output[i]->method_data->supported_methods = modifier.supportedMethods();
 
-      v8::Local<v8::String> value;
-      if (!v8::JSON::Stringify(
-               paymentMethodData.data().context(),
-               paymentMethodData.data().v8Value().As<v8::Object>())
-               .ToLocal(&value)) {
-        exceptionState.throwTypeError(
-            "Unable to parse payment method specific data");
+    if (modifier.hasData() && !modifier.data().isEmpty()) {
+      stringifyAndParseMethodSpecificData(
+          modifier.data(), output[i]->method_data, exceptionState);
+      if (exceptionState.hadException())
         return;
-      }
-      stringifiedData =
-          v8StringToWebCoreString<String>(value, DoNotExternalize);
+    } else {
+      output[i]->method_data->stringified_data = "";
     }
+  }
+}
 
-    output[i] = payments::mojom::blink::PaymentMethodData::New();
-    output[i]->supported_methods = paymentMethodData.supportedMethods();
-    output[i]->stringified_data = stringifiedData;
-    maybeSetAndroidPayMethodData(paymentMethodData.data(), output[i],
-                                 exceptionState);
+String getSelectedShippingOption(
+    const Vector<PaymentShippingOptionPtr>& shippingOptions) {
+  String result;
+  for (size_t i = 0; i < shippingOptions.size(); ++i) {
+    if (shippingOptions[i]->selected)
+      result = shippingOptions[i]->id;
+  }
+  return result;
+}
+
+void validateAndConvertPaymentDetails(const PaymentDetails& input,
+                                      bool requestShipping,
+                                      PaymentDetailsPtr& output,
+                                      String& shippingOptionOutput,
+                                      ExceptionState& exceptionState) {
+  if (!input.hasTotal()) {
+    exceptionState.throwTypeError("Must specify total");
+    return;
+  }
+
+  validateAndConvertTotal(input.total(), output->total, exceptionState);
+  if (exceptionState.hadException())
+    return;
+
+  if (input.hasDisplayItems()) {
+    validateAndConvertDisplayItems(input.displayItems(), output->display_items,
+                                   exceptionState);
     if (exceptionState.hadException())
       return;
   }
-}
 
-String getSelectedShippingOption(const PaymentDetails& details) {
-  String result;
-  if (!details.hasShippingOptions())
-    return result;
+  if (input.hasShippingOptions() && requestShipping) {
+    validateAndConvertShippingOptions(input.shippingOptions(),
+                                      output->shipping_options, exceptionState);
+    if (exceptionState.hadException())
+      return;
+  }
+
+  shippingOptionOutput = getSelectedShippingOption(output->shipping_options);
 
-  for (int i = details.shippingOptions().size() - 1; i >= 0; --i) {
-    if (details.shippingOptions()[i].hasSelected() &&
-        details.shippingOptions()[i].selected()) {
-      return details.shippingOptions()[i].id();
+  if (input.hasModifiers()) {
+    validateAndConvertPaymentDetailsModifiers(
+        input.modifiers(), output->modifiers, exceptionState);
+    if (exceptionState.hadException())
+      return;
+  }
+
+  if (input.hasError() && !input.error().isNull()) {
+    String errorMessage;
+    if (!PaymentsValidators::isValidErrorMsgFormat(input.error(),
+                                                   &errorMessage)) {
+      exceptionState.throwTypeError(errorMessage);
+      return;
     }
+    output->error = input.error();
+  } else {
+    output->error = "";
   }
+}
 
-  return result;
+void validateAndConvertPaymentMethodData(
+    const HeapVector<PaymentMethodData>& input,
+    Vector<payments::mojom::blink::PaymentMethodDataPtr>& output,
+    ExceptionState& exceptionState) {
+  if (input.isEmpty()) {
+    exceptionState.throwTypeError(
+        "Must specify at least one payment method identifier");
+    return;
+  }
+
+  output.resize(input.size());
+  for (size_t i = 0; i < input.size(); ++i) {
+    const auto& paymentMethodData = input[i];
+    if (paymentMethodData.supportedMethods().isEmpty()) {
+      exceptionState.throwTypeError(
+          "Must specify at least one payment method identifier");
+      return;
+    }
+
+    output[i] = payments::mojom::blink::PaymentMethodData::New();
+    output[i]->supported_methods = paymentMethodData.supportedMethods();
+
+    if (paymentMethodData.hasData() && !paymentMethodData.data().isEmpty()) {
+      stringifyAndParseMethodSpecificData(paymentMethodData.data(), output[i],
+                                          exceptionState);
+      if (exceptionState.hadException())
+        return;
+    } else {
+      output[i]->stringified_data = "";
+    }
+  }
 }
 
 String getValidShippingType(const String& shippingType) {
@@ -505,14 +485,6 @@ String getValidShippingType(const String& shippingType) {
   return validValues[0];
 }
 
-PaymentDetailsPtr maybeKeepShippingOptions(PaymentDetailsPtr details,
-                                           bool keep) {
-  if (!keep)
-    details->shipping_options.resize(0);
-
-  return details;
-}
-
 bool allowedToUsePaymentRequest(const Frame* frame) {
   // To determine whether a Document object |document| is allowed to use the
   // feature indicated by attribute name |allowpaymentrequest|, run these steps:
@@ -672,7 +644,11 @@ void PaymentRequest::onUpdatePaymentDetails(
     return;
   }
 
-  bool keepShippingOptions = validatePaymentDetails(details, exceptionState);
+  PaymentDetailsPtr validatedDetails =
+      payments::mojom::blink::PaymentDetails::New();
+  validateAndConvertPaymentDetails(details, m_options.requestShipping(),
+                                   validatedDetails, m_shippingOption,
+                                   exceptionState);
   if (exceptionState.hadException()) {
     m_showResolver->reject(
         DOMException::create(SyntaxError, exceptionState.message()));
@@ -680,16 +656,7 @@ void PaymentRequest::onUpdatePaymentDetails(
     return;
   }
 
-  if (m_options.requestShipping()) {
-    if (keepShippingOptions)
-      m_shippingOption = getSelectedShippingOption(details);
-    else
-      m_shippingOption = String();
-  }
-
-  m_paymentProvider->UpdateWith(maybeKeepShippingOptions(
-      payments::mojom::blink::PaymentDetails::From(details),
-      keepShippingOptions));
+  m_paymentProvider->UpdateWith(std::move(validatedDetails));
 }
 
 void PaymentRequest::onUpdatePaymentDetailsFailure(const String& error) {
@@ -746,20 +713,21 @@ PaymentRequest::PaymentRequest(ScriptState* scriptState,
     return;
   }
 
-  bool keepShippingOptions = validatePaymentDetails(details, exceptionState);
+  PaymentDetailsPtr validatedDetails =
+      payments::mojom::blink::PaymentDetails::New();
+  validateAndConvertPaymentDetails(details, m_options.requestShipping(),
+                                   validatedDetails, m_shippingOption,
+                                   exceptionState);
   if (exceptionState.hadException())
     return;
 
-  if (details.hasError() && !details.error().isEmpty()) {
+  if (!validatedDetails->error.isEmpty()) {
     exceptionState.throwTypeError("Error value should be empty");
     return;
   }
 
-  if (m_options.requestShipping()) {
-    if (keepShippingOptions)
-      m_shippingOption = getSelectedShippingOption(details);
+  if (m_options.requestShipping())
     m_shippingType = getValidShippingType(m_options.shippingType());
-  }
 
   scriptState->domWindow()->frame()->interfaceProvider()->getInterface(
       mojo::GetProxy(&m_paymentProvider));
@@ -768,10 +736,7 @@ PaymentRequest::PaymentRequest(ScriptState* scriptState,
                 PaymentErrorReason::UNKNOWN)));
   m_paymentProvider->Init(
       m_clientBinding.CreateInterfacePtrAndBind(),
-      std::move(validatedMethodData),
-      maybeKeepShippingOptions(
-          payments::mojom::blink::PaymentDetails::From(details),
-          keepShippingOptions && m_options.requestShipping()),
+      std::move(validatedMethodData), std::move(validatedDetails),
       payments::mojom::blink::PaymentOptions::From(m_options));
 }
 
@@ -813,8 +778,7 @@ void PaymentRequest::OnShippingOptionChange(const String& shippingOptionId) {
   ALLOW_UNUSED_LOCAL(success);
 }
 
-void PaymentRequest::OnPaymentResponse(
-    payments::mojom::blink::PaymentResponsePtr response) {
+void PaymentRequest::OnPaymentResponse(PaymentResponsePtr response) {
   DCHECK(m_showResolver);
   DCHECK(!m_completeResolver);
   DCHECK(!m_completeTimer.isActive());