binding: Lets Dictionary::getPropertyNames, etc. rethrow an exception.

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

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "modules/payments/PaymentRequest.h"
 
 #include "bindings/core/v8/ExceptionState.h"
 #include "bindings/core/v8/ScriptPromiseResolver.h"
 #include "bindings/core/v8/ScriptState.h"
 #include "bindings/core/v8/V8StringResource.h"
@@ skipping 339 matching lines @@
 
   String errorMessage;
   if (!PaymentsValidators::isValidErrorMsgFormat(details.error(),
                                                  &errorMessage)) {
     exceptionState.throwTypeError(errorMessage);
   }
 
   return keepShippingOptions;
 }
 
-void maybeSetAndroidPayMethodata(
+void maybeSetAndroidPayMethodData(
     const ScriptValue& input,
-    payments::mojom::blink::PaymentMethodDataPtr& output) {
+    payments::mojom::blink::PaymentMethodDataPtr& output,
+    ExceptionState& exceptionState) {
   AndroidPayMethodData androidPay;
-  TrackExceptionState exceptionState;
+  TrackExceptionState trackExceptionState;

[haraken, 2016/11/24 07:22:31]

Why can't you use exceptionState here?

[Yuki, 2016/11/24 10:19:33]

Reading the call site of this function, as "maybe" in the function name implies,
this operation is totally "optional" and okay to fail.  The call site doesn't
want this function to throw.  So, the original code intentionally has used
TrackExceptionState to swallow a possible exception.

I just keep the same behavior as before.

The reason why I let getPropertyNames() on line 410 throw an exception is
because it's unusual.  getPropertyNames() rarely fails and it's not unlike a
simple type mismatch.  So I think we don't want to ignore this kind of
exceptions.

[haraken, 2016/11/24 12:39:13]

That sounds too arbitrary to me though. To keep the existing behavior, I'm okay
with this CL, but in long term I think we should try to remove
TrackExceptionState from production code. In limited cases where we really want
to ignore a thrown exception, we can explicitly call
exceptionState.clearException.

[Yuki, 2016/11/25 08:31:04]

Acknowledged.

   V8AndroidPayMethodData::toImpl(input.isolate(), input.v8Value(), androidPay,
-                                 exceptionState);
-  if (exceptionState.hadException())
+                                 trackExceptionState);
+  if (trackExceptionState.hadException())
     return;
 
   if (androidPay.hasEnvironment() && androidPay.environment() == "TEST")
     output->environment = payments::mojom::blink::AndroidPayEnvironment::TEST;
 
   output->merchant_name = androidPay.merchantName();
   output->merchant_id = androidPay.merchantId();
 
   if (androidPay.hasAllowedCardNetworks()) {
     output->allowed_card_networks.resize(
@@ skipping 21 matching lines @@
         if (tokenization.tokenizationType() ==
             kAndroidPayTokenization[j].name) {
           output->tokenization_type = kAndroidPayTokenization[j].code;
           break;
         }
       }
     }
 
     if (tokenization.hasParameters()) {
       Vector<String> keys;
-      tokenization.parameters().getPropertyNames(keys);
+      if (!tokenization.parameters().getPropertyNames(keys, exceptionState))
+        return;
       output->parameters.resize(keys.size());
       size_t numberOfParameters = 0;
       String value;
       for (size_t i = 0; i < keys.size(); ++i) {
         if (!DictionaryHelper::get(tokenization.parameters(), keys[i], value))
           continue;
         output->parameters[numberOfParameters] =
             payments::mojom::blink::AndroidPayTokenizationParameter::New();
         output->parameters[numberOfParameters]->key = keys[i];
         output->parameters[numberOfParameters]->value = value;
@@ skipping 41 matching lines @@
             "Unable to parse payment method specific data");
         return;
       }
       stringifiedData =
           v8StringToWebCoreString<String>(value, DoNotExternalize);
     }
 
     output[i] = payments::mojom::blink::PaymentMethodData::New();
     output[i]->supported_methods = paymentMethodData.supportedMethods();
     output[i]->stringified_data = stringifiedData;
-    maybeSetAndroidPayMethodata(paymentMethodData.data(), output[i]);
+    maybeSetAndroidPayMethodData(paymentMethodData.data(), output[i],
+                                 exceptionState);
+    if (exceptionState.hadException())
+      return;
   }
 }
 
 String getSelectedShippingOption(const PaymentDetails& details) {
   String result;
   if (!details.hasShippingOptions())
     return result;
 
   for (int i = details.shippingOptions().size() - 1; i >= 0; --i) {
     if (details.shippingOptions()[i].hasSelected() &&
@@ skipping 481 matching lines @@
   m_completeResolver.clear();
   m_showResolver.clear();
   m_abortResolver.clear();
   m_canMakeActivePaymentResolver.clear();
   if (m_clientBinding.is_bound())
     m_clientBinding.Close();
   m_paymentProvider.reset();
 }
 
 }  // namespace blink