[Autofill] Sort profiles and credit cards by frecency in PaymentRequest.

chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java

Index: chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java b/chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java
index 305b302e69d025a07217d3d83f4a01d3aa5c5fa0..ec410bcfff3e29c6221f3364f6a03422a760e2c3 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/autofill/PersonalDataManager.java
@@ -473,23 +473,26 @@ public class PersonalDataManager {
         mDataObservers.remove(observer);
     }
 
-    public List<AutofillProfile> getProfiles() {
-        return getProfilesWithAddressOnlyLabels(false);
-    }
-
-    public List<AutofillProfile> getAddressOnlyProfiles() {
-        return getProfilesWithAddressOnlyLabels(true);
+    // TODO(crbug.com/616102): Reduce the number of Java to Native calls when getting profiles.
+    public List<AutofillProfile> getProfilesForSettings() {

[gone, 2016/05/31 22:39:42]

Add a javadoc here so that people who aren't familiar with Payments know why it
is different from the similarly named one below.  Given their similarities, is
there any reason you switched away from using the boolean?  Seems like you
could've used something like an enum to make this clearer without making
multiple similarly-named flavors of the same method.

[please use gerrit instead, 2016/06/01 17:42:59]

It was originally my request. getProfiles(), which is now called
getProfilesForSettings() is called in multiple places, so I wanted to make sure
those places don't use the wrong version accidentally. Both function name and
enum parameter provide more verbosity than a boolean that was here before. I'm
OK with using an enum, if you would prefer that.

[gone, 2016/06/01 17:48:10]

Eh, it's fine.  I'm just worried that you could get into a situation where you
have to add another flavor of these methods and the code becomes harder to read.
 I'll defer to your judgement.

[sebsg, 2016/06/01 20:58:24]

Good point to the javadoc. I'll stay purposely vague because we are actively
modifying the processing in the *ToSuggest path. We currently sort the data by
Frecency (frequency and recency) and dedupe them.

For the naming, I thought it would be clearer that way because they follow a
linear path down to a native method with the (almost) same name. For example,
getProfilesToSuggest in PersonalDataManager.java ends up calling
GetProfileToSuggest in personal_data_manager.cc.  Almost because
get(Profiles|CreditCards)ToSuggest end up calling get(Profiles|CreditCards) in
the native code.

I can change it to an enum if you prefer. I prefer this way but I can see why
you would prefer the other way.

+        ThreadUtils.assertOnUiThread();
+        return getProfilesWithLabels(nativeGetProfileLabelsForSettings(mPersonalDataManagerAndroid),
+                nativeGetProfileGUIDsForSettings(mPersonalDataManagerAndroid));
     }
 
-    private List<AutofillProfile> getProfilesWithAddressOnlyLabels(boolean addressOnly) {
+    // TODO(crbug.com/616102): Reduce the number of Java to Native calls when getting profiles.

[gone, 2016/05/31 22:39:42]

This really is kind of an absurd number of JNI calls, but it seems that the
number of calls hasn't increased, overall.  Is there a plan for this?

[sebsg, 2016/06/01 20:58:24]

You are correct, this seems like an unncessary number of calls but it's the same
number as it was before this CL. I'd like to work on that as a next CL. I
created un bug for this (crbug.com/616102).

+    public List<AutofillProfile> getProfilesToSuggest() {
         ThreadUtils.assertOnUiThread();
+        return getProfilesWithLabels(nativeGetProfileLabelsToSuggest(mPersonalDataManagerAndroid),
+                nativeGetProfileGUIDsToSuggest(mPersonalDataManagerAndroid));
+    }
 
-        String[] profileLabels = nativeGetProfileLabels(mPersonalDataManagerAndroid, addressOnly);
-
-        int profileCount = nativeGetProfileCount(mPersonalDataManagerAndroid);
-        List<AutofillProfile> profiles = new ArrayList<AutofillProfile>(profileCount);
-        for (int i = 0; i < profileCount; i++) {
-            AutofillProfile profile = nativeGetProfileByIndex(mPersonalDataManagerAndroid, i);
+    private List<AutofillProfile> getProfilesWithLabels(
+            String[] profileLabels, String[] profileGUIDs) {
+        List<AutofillProfile> profiles = new ArrayList<AutofillProfile>(profileGUIDs.length);
+        for (int i = 0; i < profileGUIDs.length; i++) {
+            AutofillProfile profile =
+                    nativeGetProfileByGUID(mPersonalDataManagerAndroid, profileGUIDs[i]);
             profile.setLabel(profileLabels[i]);
             profiles.add(profile);
         }
@@ -512,12 +515,22 @@ public class PersonalDataManager {
         return nativeSetProfile(mPersonalDataManagerAndroid, profile);
     }
 
-    public List<CreditCard> getCreditCards() {
+    public List<CreditCard> getCreditCardsForSettings() {
         ThreadUtils.assertOnUiThread();
-        int count = nativeGetCreditCardCount(mPersonalDataManagerAndroid);
-        List<CreditCard> cards = new ArrayList<CreditCard>(count);
-        for (int i = 0; i < count; i++) {
-            cards.add(nativeGetCreditCardByIndex(mPersonalDataManagerAndroid, i));
+

[please use gerrit instead, 2016/05/31 18:51:56]

nit: no new line.

[sebsg, 2016/06/01 20:58:24]

Done.

+        return getCreditCards(nativeGetCreditCardGUIDsForSettings(mPersonalDataManagerAndroid));
+    }
+
+    public List<CreditCard> getCreditCardsToSuggest() {
+        ThreadUtils.assertOnUiThread();
+

[please use gerrit instead, 2016/05/31 18:51:56]

nit: no new line.

[sebsg, 2016/06/01 20:58:24]

Done.

+        return getCreditCards(nativeGetCreditCardGUIDsToSuggest(mPersonalDataManagerAndroid));
+    }
+
+    private List<CreditCard> getCreditCards(String[] creditCardGUIDs) {
+        List<CreditCard> cards = new ArrayList<CreditCard>(creditCardGUIDs.length);
+        for (int i = 0; i < creditCardGUIDs.length; i++) {
+            cards.add(nativeGetCreditCardByGUID(mPersonalDataManagerAndroid, creditCardGUIDs[i]));
         }
         return cards;
     }
@@ -555,6 +568,18 @@ public class PersonalDataManager {
                 mPersonalDataManagerAndroid, webContents, guid, delegate);
     }
 
+    @VisibleForTesting
+    public void setProfileUseStatsForTesting(String guid, int count, int date) {

[gone, 2016/05/31 22:39:42]

1) Can this be package protected?
2) Timestamps are longs, not ints.

[sebsg, 2016/06/01 20:58:24]

Done.

+        ThreadUtils.assertOnUiThread();
+        nativeSetProfileUseStatsForTesting(mPersonalDataManagerAndroid, guid, count, date);
+    }
+
+    @VisibleForTesting
+    public void setCreditCardUseStatsForTesting(String guid, int count, int date) {
+        ThreadUtils.assertOnUiThread();
+        nativeSetCreditCardUseStatsForTesting(mPersonalDataManagerAndroid, guid, count, date);
+    }
+
     /**
      * @return Whether the Autofill feature is enabled.
      */
@@ -593,18 +618,19 @@ public class PersonalDataManager {
     }
 
     private native long nativeInit();
-    private native int nativeGetProfileCount(long nativePersonalDataManagerAndroid);
-    private native String[] nativeGetProfileLabels(long nativePersonalDataManagerAndroid,
-            boolean addressOnly);
-    private native AutofillProfile nativeGetProfileByIndex(long nativePersonalDataManagerAndroid,
-            int index);
+    private native String[] nativeGetProfileGUIDsForSettings(long nativePersonalDataManagerAndroid);
+    private native String[] nativeGetProfileGUIDsToSuggest(long nativePersonalDataManagerAndroid);
+    private native String[] nativeGetProfileLabelsForSettings(
+            long nativePersonalDataManagerAndroid);
+    private native String[] nativeGetProfileLabelsToSuggest(long nativePersonalDataManagerAndroid);
     private native AutofillProfile nativeGetProfileByGUID(long nativePersonalDataManagerAndroid,
             String guid);
     private native String nativeSetProfile(long nativePersonalDataManagerAndroid,
             AutofillProfile profile);
-    private native int nativeGetCreditCardCount(long nativePersonalDataManagerAndroid);
-    private native CreditCard nativeGetCreditCardByIndex(long nativePersonalDataManagerAndroid,
-            int index);
+    private native String[] nativeGetCreditCardGUIDsForSettings(
+            long nativePersonalDataManagerAndroid);
+    private native String[] nativeGetCreditCardGUIDsToSuggest(
+            long nativePersonalDataManagerAndroid);
     private native CreditCard nativeGetCreditCardByGUID(long nativePersonalDataManagerAndroid,
             String guid);
     private native String nativeSetCreditCard(long nativePersonalDataManagerAndroid,
@@ -612,6 +638,10 @@ public class PersonalDataManager {
     private native void nativeAddServerCreditCardForTest(long nativePersonalDataManagerAndroid,
             CreditCard card);
     private native void nativeRemoveByGUID(long nativePersonalDataManagerAndroid, String guid);
+    private native void nativeSetProfileUseStatsForTesting(
+            long nativePersonalDataManagerAndroid, String guid, int count, int date);
+    private native void nativeSetCreditCardUseStatsForTesting(
+            long nativePersonalDataManagerAndroid, String guid, int count, int date);
     private native void nativeClearUnmaskedCache(
             long nativePersonalDataManagerAndroid, String guid);
     private native void nativeGetFullCardForPaymentRequest(long nativePersonalDataManagerAndroid,