Java implementation of PaymentRequest mojo service

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

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillPaymentApp.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillPaymentApp.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillPaymentApp.java
new file mode 100644
index 0000000000000000000000000000000000000000..394a70d0cfdb72df51f74c51a35528d6388f5a52
--- /dev/null
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillPaymentApp.java
@@ -0,0 +1,60 @@
+// 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 org.chromium.chrome.browser.autofill.PersonalDataManager;
+import org.chromium.chrome.browser.autofill.PersonalDataManager.CreditCard;
+import org.chromium.chrome.browser.payments.ui.LineItem;
+import org.chromium.content_public.browser.WebContents;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.List;
+
+/**
+ * Provides access to locally stored user credit cards.
+ */
+public class AutofillPaymentApp implements PaymentApp {
+    private final WebContents mWebContents;
+
+    /**
+     * Builds a payment app backed by autofill cards.
+     *
+     * @param webContents The web contents where PaymentRequest was invoked.
+     */
+    public AutofillPaymentApp(WebContents webContents) {
+        mWebContents = webContents;
+    }
+
+    @Override
+    public void getInstruments(ArrayList<LineItem> unusedItems, InstrumentsCallback callback) {
+        List<PaymentInstrument> instruments = new LinkedList<PaymentInstrument>();
+        for (CreditCard card : PersonalDataManager.getInstance().getCreditCards()) {

[Ted C, 2016/04/22 03:59:36]

if getCreditCards doesn't return an array (instead a list), use the for (int i =
0; i < ...) instead of the foreach version.  This will cause an iterator to be
allocated and that should be avoided if possible.

[please use gerrit instead, 2016/04/25 19:22:30]

Done.

+            instruments.add(new AutofillPaymentInstrument(mWebContents, card));
+        }
+        callback.onInstrumentsReady(this, instruments);

[Ted C, 2016/04/22 03:59:36]

Maybe we should be posting this (new Handler().post<...>) since this is supposed
to be async.  should make the calling sites more predictable.

[please use gerrit instead, 2016/04/25 19:22:29]

Made AutofillPaymentApp implement the Runnable interface so that we can post via
"new Handler().post(this);". This should be more efficient, right?

PRO: Eliminate the need for the anonymous Runnable instance. You did not want to
allocate an iterator on line 36, so I assume that allocating new objects is
expensive.

CON: Need to store "callback" and "instruments" as member variables.

getInstruments() is called only once, so there's no worry about overwriting the
member variables.

[Ted C, 2016/04/25 22:49:02]

it is more efficient, but I would argue that readability is worse of here.

the iterator thing is again more of a convention.  It really only matters when
you are doing something on the draw path that could push android over the java
limit and force a GC.  I tell people to do it that way as a means of consistency
more than anything else.  If all places follow it then it's more likely that new
people will do that. etc, etc..  I would just define a runnable here.

[please use gerrit instead, 2016/04/25 23:19:24]

Done.

+    }
+
+    @Override
+    public HashSet<String> getSupportedMethodNames() {
+        // https://w3c.github.io/browser-payment-api/specs/basic-card-payment.html#method-id
+        //
+        // The spec does not include "generic" card types. That's the type of card for which
+        // Chrome cannot determine the type.
+        //
+        // The spec also includes more detailed card types, e.g., "visa/credit" and "visa/debit".
+        // Autofill does not distinguish between these types of cards, so they are not in the list
+        // of supported method names.
+        return new HashSet<String>(Arrays.asList(new String[] {

[Ted C, 2016/04/22 03:59:36]

this is an allocation of a string array, arraylist, and finally a set for the
same strings. 

better to make this more verbose and add them one at a time.

[please use gerrit instead, 2016/04/25 19:22:30]

Done.

+                "visa", "mastercard", "amex", "discover", "diners", "jcb", "unionpay", "generic"}));
+    }
+
+    @Override
+    public String getIdentifier() {
+        return "org.chromium.chrome.browser.payments.AutofillPaymentApp";

[Ted C, 2016/04/22 03:59:36]

do you want this to stay consistent over time (across file moves) or would you
be happy if this just reflected the current class name.

Just wondering if we should just use getClass().getName()?

ah, I see it is stored in prefs.  I wonder if we should use something other than
class name here (maybe make it a constant).  Chrome_Autofill_Payment_App or
something.

[please use gerrit instead, 2016/04/25 19:22:30]

Using "Chrome_Autofill_Payment_App".

+    }
+}