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

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 f67b63bede62e3795377d09323f5ff31669c1382..66bacd358d5eb09a5bb4f6c02b07bcf6277f5b4d 100644
--- a/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
+++ b/third_party/WebKit/Source/modules/payments/PaymentRequest.cpp
@@ -357,14 +357,15 @@ bool validatePaymentDetails(const PaymentDetails& details,
   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")
@@ -406,7 +407,8 @@ void maybeSetAndroidPayMethodata(
 
     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;
@@ -468,7 +470,10 @@ void validateAndConvertPaymentMethodData(
     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;
   }
 }