PaymentApp: Allow multiple payment method names for one instrument.

chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java

 // 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.
 
 package org.chromium.chrome.browser.payments;
 
 import android.app.Activity;
 import android.graphics.Bitmap;
 import android.os.Handler;
 import android.support.v4.util.ArrayMap;
@@ skipping 37 matching lines @@
 import org.chromium.payments.mojom.PaymentRequest;
 import org.chromium.payments.mojom.PaymentRequestClient;
 import org.chromium.payments.mojom.PaymentResponse;
 import org.chromium.payments.mojom.PaymentShippingOption;
 import org.chromium.payments.mojom.PaymentShippingType;
 
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 
 /**
  * Android implementation of the PaymentRequest service defined in
  * components/payments/payment_request.mojom.
  */
@@ skipping 832 matching lines @@
                 } else if (toEdit == null) {
                     mPaymentMethodsSection.addAndSelectItem(completeCard);
                 }
 
                 mUI.updateSection(PaymentRequestUI.TYPE_PAYMENT_METHODS, mPaymentMethodsSection);
             }
         });
     }
 
     @Override
-    public void loadingInstrumentDetails() {
+    public void onInstrumentDetailsLoading() {
         if (mClient == null || mUI == null || mPaymentResponseHelper == null) return;
 
         mUI.showProcessingMessage();
         mPaymentResponseHelper.onInstrumentsDetailsLoading();
     }
 
     @Override
     public boolean onPayClicked(PaymentOption selectedShippingAddress,
             PaymentOption selectedShippingOption, PaymentOption selectedPaymentMethod) {
         assert selectedPaymentMethod instanceof PaymentInstrument;
         PaymentInstrument instrument = (PaymentInstrument) selectedPaymentMethod;
         mPaymentAppRunning = true;
 
         PaymentOption selectedContact =
                 mContactSection != null ? mContactSection.getSelectedItem() : null;
         mPaymentResponseHelper = new PaymentResponseHelper(
                 selectedShippingAddress, selectedShippingOption, selectedContact, this);
 
-        instrument.getInstrumentDetails(mMerchantName, mOrigin, mRawTotal, mRawLineItems,
-                mMethodData.get(instrument.getInstrumentMethodName()), this);
+        // Create a map that is the subset of mMethodData that contains the
+        // payment methods supported by the selected payment instrument. If this
+        // intersection contains more than one payment method, the payment app is
+        // at liberty to choose (or have the user choose) one of the methods.
+        Map<String, PaymentMethodData> methodData = new HashMap<>();
+        for (String instrumentMethodName : instrument.getInstrumentMethodNames()) {
+            if (mMethodData.containsKey(instrumentMethodName)) {
+                methodData.put(instrumentMethodName, mMethodData.get(instrumentMethodName));
+            }
+        }
+
+        instrument.invokePayment(mMerchantName, mOrigin, mRawTotal, mRawLineItems,
+                methodData, this);

[please use gerrit instead, 2016/11/29 14:17:18]

Wrap methodData in Collections.unmodifiableMap().

[tommyt, 2016/11/30 10:15:50]

Done.

         recordSuccessFunnelHistograms("PayClicked");
         return !(instrument instanceof AutofillPaymentInstrument);
     }
 
     @Override
     public void onDismiss() {
         disconnectFromClientWithDebugMessage("Dialog dismissed");
         closeUI(true);
         recordAbortReasonHistogram(PaymentRequestMetrics.ABORT_REASON_ABORTED_BY_USER);
     }
@@ skipping 110 matching lines @@
      */
     @Override
     public void onInstrumentsReady(PaymentApp app, List<PaymentInstrument> instruments) {
         if (mClient == null) return;
         mPendingApps.remove(app);
 
         // Place the instruments into either "autofill" or "non-autofill" list to be displayed when
         // all apps have responded.
         if (instruments != null) {
             for (int i = 0; i < instruments.size(); i++) {
+                boolean added = false;
                 PaymentInstrument instrument = instruments.get(i);
-                if (mMethodData.containsKey(instrument.getInstrumentMethodName())) {
-                    addPendingInstrument(instrument);
-                } else {
+                // If at least one of the payment method names supported by the
+                // instrument is in mMethodData, add this instrument.

[please use gerrit instead, 2016/11/29 14:17:18]

Your approach is 9 lines long. Here's a shorter way to write the same thing:

Set<String> instrumentMethodNames = instrument.getInstrumentMethodNames();
instrumentMethodNames.retainAll(mMethodData.keySet());
if (!instrumentMethodNames.isEmpty()) {
    addPendingInstrument(instrument);
} else {
    instrument.dismissInstrument();
}

This takes up 7 lines, saving you 2 lines of code.

The additional comments seem to be not necessary either. 

[tommyt, 2016/11/30 10:15:50]

Done.

+                for (String instrumentMethodName : instrument.getInstrumentMethodNames()) {
+                    if (mMethodData.containsKey(instrumentMethodName)) {
+                        addPendingInstrument(instrument);
+                        added = true;
+                        break;
+                    }
+                }
+                // ... otherwise get rid of it.
+                if (!added) {
                     instrument.dismissInstrument();
                 }
             }
         }
 
         // Some payment apps still have not responded. Continue waiting for them.
         if (!mPendingApps.isEmpty()) return;
 
         if (disconnectIfNoPaymentMethodsSupported()) return;
 
@@ skipping 219 matching lines @@
                 "PaymentRequest.CheckoutFunnel.Aborted", abortReason,
                 PaymentRequestMetrics.ABORT_REASON_MAX);
 
         if (abortReason == PaymentRequestMetrics.ABORT_REASON_ABORTED_BY_USER) {
             mJourneyLogger.recordJourneyStatsHistograms("UserAborted");
         } else {
             mJourneyLogger.recordJourneyStatsHistograms("OtherAborted");
         }
     }
 }