[Payments] More verbose 'payment method not supported' message.

Before this patch, PaymentRequest.show() could reject with "Payment
method not supported" message, which did not clarify which payment
methods were not supported. This could be debugging confusing in a large
production environment, where multiple PaymentRequest instances with
different payment method names are created.

This patch stores the set of payment method names for each instance of
PaymentRequest and prints them out in the error message, if none of them
are supported.

After this patch, PaymentRequest.show() can reject with error message
"Payment method 'xyz' is not supported". (Plurals are handled
correctly.)

Bug: 785992
Change-Id: I976b11b84f788ea98b4e413ca983edd8f792052d
Reviewed-on: https://chromium-review.googlesource.com/775045Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Rouslan Solomakhin <rouslan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#517243}

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFieldTrialTest.java

@@ -60,7 +60,7 @@ public class PaymentRequestFieldTrialTest implements MainActivityStartCallback {
             throws InterruptedException, ExecutionException, TimeoutException {
         mPaymentRequestTestRule.openPageAndClickBuyAndWait(mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"The payment method is not supported"});
+                new String[] {"The payment method", "not supported"});
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestJourneyLoggerTest.java

@@ -1065,7 +1065,7 @@ public class PaymentRequestJourneyLoggerTest implements MainActivityStartCallbac
         mPaymentRequestTestRule.openPageAndClickNodeAndWait(
                 "androidPayBuy", mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"The payment method is not supported"});
+                new String[] {"The payment method", "not supported"});
 
         // Make sure that no journey metrics were logged.
         Assert.assertEquals(0,

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestMetricsTest.java

@@ -272,7 +272,7 @@ public class PaymentRequestMetricsTest implements MainActivityStartCallback {
         mPaymentRequestTestRule.openPageAndClickNodeAndWait(
                 "androidPayBuy", mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"The payment method is not supported"});
+                new String[] {"The payment method", "not supported"});
 
         // Make sure that it is not logged as an abort.
         mPaymentRequestTestRule.assertOnlySpecificAbortMetricLogged(-1 /* none */);
@@ -302,7 +302,7 @@ public class PaymentRequestMetricsTest implements MainActivityStartCallback {
         mPaymentRequestTestRule.openPageAndClickNodeAndWait(
                 "noSupported", mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"The payment method is not supported"});
+                new String[] {"The payment method", "not supported"});
 
         // Make sure that it is not logged as an abort.
         mPaymentRequestTestRule.assertOnlySpecificAbortMetricLogged(-1 /* none */);

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestModifierTest.java

@@ -42,6 +42,6 @@ public class PaymentRequestModifierTest implements MainActivityStartCallback {
     public void testNoCrash() throws InterruptedException, ExecutionException, TimeoutException {
         mPaymentRequestTestRule.openPageAndClickBuyAndWait(mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"The payment method is not supported"});
+                new String[] {"The payment method", "not supported"});
     }
 }

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestPaymentAppTest.java

@@ -49,7 +49,7 @@ public class PaymentRequestPaymentAppTest {
             throws InterruptedException, ExecutionException, TimeoutException {
         mPaymentRequestTestRule.openPageAndClickBuyAndWait(mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"show() rejected", "The payment method is not supported"});
+                new String[] {"show() rejected", "The payment method", "not supported"});
     }
 
     /**
@@ -64,7 +64,7 @@ public class PaymentRequestPaymentAppTest {
         mPaymentRequestTestRule.installPaymentApp(NO_INSTRUMENTS, IMMEDIATE_RESPONSE);
         mPaymentRequestTestRule.openPageAndClickBuyAndWait(mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"show() rejected", "The payment method is not supported"});
+                new String[] {"show() rejected", "The payment method", "not supported"});
     }
 
     /**
@@ -79,7 +79,7 @@ public class PaymentRequestPaymentAppTest {
         mPaymentRequestTestRule.installPaymentApp(NO_INSTRUMENTS, DELAYED_RESPONSE);
         mPaymentRequestTestRule.openPageAndClickBuyAndWait(mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"show() rejected", "The payment method is not supported"});
+                new String[] {"show() rejected", "The payment method", "not supported"});
     }
 
     /**
@@ -122,7 +122,7 @@ public class PaymentRequestPaymentAppTest {
         mPaymentRequestTestRule.openPageAndClickBuyAndWait(mPaymentRequestTestRule.getShowFailed());
         ThreadUtils.runOnUiThreadBlocking(() -> app.respond());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"show() rejected", "The payment method is not supported"});
+                new String[] {"show() rejected", "The payment method", "not supported"});
     }
 
     /**

chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestServiceWorkerPaymentAppTest.java

@@ -70,7 +70,7 @@ public class PaymentRequestServiceWorkerPaymentAppTest {
 
         mPaymentRequestTestRule.openPageAndClickBuyAndWait(mPaymentRequestTestRule.getShowFailed());
         mPaymentRequestTestRule.expectResultContains(
-                new String[] {"show() rejected", "The payment method is not supported"});
+                new String[] {"show() rejected", "The payment method", "not supported"});
     }
 
     @Test

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

@@ -48,6 +48,7 @@
 #include "platform/runtime_enabled_features.h"
 #include "platform/weborigin/KURL.h"
 #include "platform/wtf/HashSet.h"
+#include "platform/wtf/text/StringBuilder.h"
 #include "public/platform/Platform.h"
 #include "public/platform/TaskType.h"
 #include "public/platform/WebTraceLocation.h"
@@ -662,6 +663,7 @@ void ValidateAndConvertPaymentDetailsUpdate(const PaymentDetailsUpdate& input,
 void ValidateAndConvertPaymentMethodData(
     const HeapVector<PaymentMethodData>& input,
     Vector<payments::mojom::blink::PaymentMethodDataPtr>& output,
+    HashSet<String>& method_names,
     ExecutionContext& execution_context,
     ExceptionState& exception_state) {
   if (input.IsEmpty()) {
@@ -715,6 +717,7 @@ void ValidateAndConvertPaymentMethodData(
             "Invalid payment method identifier format");
         return;
       }
+      method_names.insert(identifier);
     }
 
     CountPaymentRequestNetworkNameInSupportedMethods(supported_methods,
@@ -1016,7 +1019,8 @@ PaymentRequest::PaymentRequest(ExecutionContext* execution_context,
 
   Vector<payments::mojom::blink::PaymentMethodDataPtr> validated_method_data;
   ValidateAndConvertPaymentMethodData(method_data, validated_method_data,
-                                      *GetExecutionContext(), exception_state);
+                                      method_names_, *GetExecutionContext(),
+                                      exception_state);
   if (exception_state.HadException())
     return;
 
@@ -1156,18 +1160,39 @@ void PaymentRequest::OnError(PaymentErrorReason error) {
   String message;
 
   switch (error) {
-    case PaymentErrorReason::USER_CANCEL:
+    case PaymentErrorReason::USER_CANCEL: {
       ec = kAbortError;
       message = "Request cancelled";
       break;
-    case PaymentErrorReason::NOT_SUPPORTED:
+    }
+
+    case PaymentErrorReason::NOT_SUPPORTED: {
       ec = kNotSupportedError;
-      message = "The payment method is not supported";
+      DCHECK_LE(1U, method_names_.size());
+      auto it = method_names_.begin();
+      if (method_names_.size() == 1U) {
+        message = "The payment method \"" + *it + "\" is not supported";
+      } else {
+        StringBuilder sb;
+        sb.Append("The payment methods \"");
+        sb.Append(*it);
+        sb.Append("\"");
+        while (++it != method_names_.end()) {
+          sb.Append(", \"");
+          sb.Append(*it);
+          sb.Append("\"");
+        }
+        sb.Append(" are not supported");
+        message = sb.ToString();
+      }
       break;
-    case PaymentErrorReason::UNKNOWN:
+    }
+
+    case PaymentErrorReason::UNKNOWN: {
       ec = kUnknownError;
       message = "Request failed";
       break;
+    }
   }
 
   DCHECK(!message.IsEmpty());

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

@@ -130,6 +130,7 @@ class MODULES_EXPORT PaymentRequest final
   String id_;
   String shipping_option_;
   String shipping_type_;
+  HashSet<String> method_names_;
   Member<ScriptPromiseResolver> show_resolver_;
   Member<ScriptPromiseResolver> complete_resolver_;
   Member<ScriptPromiseResolver> abort_resolver_;

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

@@ -343,7 +343,7 @@ TEST(PaymentRequestTest, RejectShowPromiseOnErrorPaymentMethodNotSupported) {
       payments::mojom::blink::PaymentErrorReason::NOT_SUPPORTED);
 
   v8::MicrotasksScope::PerformCheckpoint(scope.GetScriptState()->GetIsolate());
-  EXPECT_EQ("NotSupportedError: The payment method is not supported",
+  EXPECT_EQ("NotSupportedError: The payment method \"foo\" is not supported",
             error_message);
 }