PaymentRequest: Ignore shipping options if there are duplicated IDs.

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 fb01a7ba7708bb871637c854cd8f1de218a5b5df..b35af800b0d52c5ea69c2cfb77c2c18c6b55c576 100644
--- a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
+++ b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
@@ -223,18 +223,17 @@ void validateDisplayItems(const HeapVector<PaymentItem>& items,
   }
 }
 
-void validateShippingOptions(const HeapVector<PaymentShippingOption>& options,
-                             ExceptionState& exceptionState) {
+void validateAndFixupShippingOptions(PaymentDetails& details,

[please use gerrit instead, 2016/10/12 01:28:02]

nit: Pass in only "HeapVector<PaymentShippingOption>& options", so it's clear
that this function acts only on the shipping options. You can reset it via
"options = HeapVector<PaymentShippingOption>();"

[zino, 2016/10/12 18:05:24]

Done.

+                                     ExceptionState& exceptionState) {
   HashSet<String> uniqueIds;
-  for (const auto& option : options) {
+  for (const auto& option : details.shippingOptions()) {
     if (!option.hasId() || option.id().isEmpty()) {
       exceptionState.throwTypeError("ShippingOption id required");
       return;
     }
 
     if (uniqueIds.contains(option.id())) {
-      exceptionState.throwTypeError(
-          "Duplicate shipping option identifiers are not allowed");
+      details.setShippingOptions(HeapVector<PaymentShippingOption>());
       return;
     }
     uniqueIds.add(option.id());
@@ -291,8 +290,8 @@ void validatePaymentDetailsModifiers(
   }
 }
 
-void validatePaymentDetails(const PaymentDetails& details,
-                            ExceptionState& exceptionState) {
+void validateAndFixupPaymentDetails(PaymentDetails& details,
+                                    ExceptionState& exceptionState) {
   if (!details.hasTotal()) {
     exceptionState.throwTypeError("Must specify total");
     return;
@@ -314,7 +313,7 @@ void validatePaymentDetails(const PaymentDetails& details,
   }
 
   if (details.hasShippingOptions()) {
-    validateShippingOptions(details.shippingOptions(), exceptionState);
+    validateAndFixupShippingOptions(details, exceptionState);
     if (exceptionState.hadException())
       return;
   }
@@ -524,7 +523,7 @@ void PaymentRequest::onUpdatePaymentDetails(
     return;
   }
 
-  validatePaymentDetails(details, exceptionState);
+  validateAndFixupPaymentDetails(details, exceptionState);
   if (exceptionState.hadException()) {
     m_showResolver->reject(
         DOMException::create(SyntaxError, exceptionState.message()));
@@ -589,7 +588,8 @@ PaymentRequest::PaymentRequest(ScriptState* scriptState,
     return;
   }
 
-  validatePaymentDetails(details, exceptionState);
+  PaymentDetails fixedDetails(details);
+  validateAndFixupPaymentDetails(fixedDetails, exceptionState);
   if (exceptionState.hadException())
     return;
 
@@ -599,7 +599,7 @@ PaymentRequest::PaymentRequest(ScriptState* scriptState,
   }
 
   if (m_options.requestShipping()) {
-    m_shippingOption = getSelectedShippingOption(details);
+    m_shippingOption = getSelectedShippingOption(fixedDetails);
     m_shippingType = getValidShippingType(m_options.shippingType());
   }
 
@@ -612,7 +612,7 @@ PaymentRequest::PaymentRequest(ScriptState* scriptState,
       m_clientBinding.CreateInterfacePtrAndBind(),
       mojo::WTFArray<mojom::blink::PaymentMethodDataPtr>::From(
           validatedMethodData),
-      mojom::blink::PaymentDetails::From(details),
+      mojom::blink::PaymentDetails::From(fixedDetails),
       mojom::blink::PaymentOptions::From(m_options));
 }