[Payments] Display warning messages for incomplete shipping address and update editor title

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

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
index db3f746d68ffd1a37a4f4d440e8a8ba56683f94d..b20af9561a63c628f97452f76f541718c1cc03fe 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/AutofillAddress.java
@@ -4,12 +4,18 @@
 
 package org.chromium.chrome.browser.payments;
 
+import android.content.Context;
+import android.telephony.PhoneNumberUtils;
 import android.text.TextUtils;
 
+import org.chromium.chrome.R;
 import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
 import org.chromium.chrome.browser.payments.ui.PaymentOption;
+import org.chromium.chrome.browser.preferences.autofill.AutofillProfileBridge;
+import org.chromium.chrome.browser.preferences.autofill.AutofillProfileBridge.AddressField;
 import org.chromium.payments.mojom.PaymentAddress;
 
+import java.util.List;
 import java.util.Locale;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -31,6 +37,7 @@ public class AutofillAddress extends PaymentOption {
 
     @Nullable private static Pattern sRegionCodePattern;
 
+    private Context mContext;
     private AutofillProfile mProfile;
     @Nullable private Pattern mLanguageScriptCodePattern;
 
@@ -39,11 +46,12 @@ public class AutofillAddress extends PaymentOption {
      *
      * @param profile The autofill profile containing the address information.
      */
-    public AutofillAddress(AutofillProfile profile, boolean isComplete) {
+    public AutofillAddress(Context context, AutofillProfile profile) {
         super(profile.getGUID(), profile.getFullName(), profile.getLabel(),
                 profile.getPhoneNumber(), null);
+        mContext = context;
         mProfile = profile;
-        mIsComplete = isComplete;
+        checkAndUpdateAddressCompleteness();
     }
 
     /** @return The autofill profile where this address data lives. */
@@ -59,9 +67,104 @@ public class AutofillAddress extends PaymentOption {
      */
     public void completeAddress(AutofillProfile profile) {
         mProfile = profile;
-        mIsComplete = true;
         updateIdentifierAndLabels(mProfile.getGUID(), mProfile.getFullName(), mProfile.getLabel(),
                 mProfile.getPhoneNumber());
+        checkAndUpdateAddressCompleteness();
+        assert mIsComplete;
+    }
+
+    /**
+     * Checks whether this address is complete and updates edit message, edit title and complete
+     * status.
+     */
+    private void checkAndUpdateAddressCompleteness() {
+        int[] editMessageResIds = getInformationRequiredMessageResIdsForEdit(mProfile);
+
+        mEditMessage = editMessageResIds == null ? null : mContext.getString(editMessageResIds[0]);
+        mEditTitle = editMessageResIds == null ? null : mContext.getString(editMessageResIds[1]);
+        mIsComplete = mEditMessage == null;
+    }
+
+    /**
+     * Gets information required message resource Ids for edit if the profile does not contain
+     * complete address information, i.e., can not be sent to the merchant as-is without editing
+     * first. Otherwise returns null.
+     *
+     * If the country code is not set or invalid, but all fields for the default locale's country
+     * code are present, then the profile is deemed "complete." AutoflllAddress.toPaymentAddress()
+     * will use the default locale to fill in a blank country code before sending the address to the
+     * renderer.
+     *
+     * @param profile The autofill profile containing the address information.
+     * @return int[]  The information required message resource Ids for edit. int[0] contains
+     *                required information description message resource Id. int[1] contains message
+     *                resource Id for editor title.
+     *                Returns null if the contained address information in the profile is complete.
+     */
+    @Nullable
+    public static int[] getInformationRequiredMessageResIdsForEdit(AutofillProfile profile) {

[Ted C, 2016/11/08 22:14:06]

don't want to derail this too much, but I find this to be an odd api (especially
for one to be exposed).

are the message ids only used in this class?  If so, I think the public api
should be something like

boolean isAutofillProfileComplete(AutofillProfile profile) {
    return getInformationRequiredMessageResIdsForEdit(profile) == null;
}

Granted, the "Is Complete" might not be right for this context, but it seems
like what outside users would actually want.

Another option would be to not have this method deal with message ids but
instead would return a state for the profile.

public static final int PROFILE_COMPLETE = 0;
public static final int PROFILE_MISSING_PHONE_NUMBER = 1;
...
public static final int PROFILE_MISSING_MULTIPLE_FIELDS = X;

then this method would simply return that state.  For the external case, it
would just check that it returns PROFILE_COMPLETE.  Then for message calculation
you would look at the other states and populate it that way.

[gogerald1, 2016/11/09 01:42:46]

Done. Yes, a little bit, was more worried about time efficiency.

+        int invalidFieldsCount = 0;
+        int editMessageResId = 0;
+        int editTitleResId = 0;
+
+        if (!PhoneNumberUtils.isGlobalPhoneNumber(
+                    PhoneNumberUtils.stripSeparators(profile.getPhoneNumber().toString()))) {
+            editMessageResId = R.string.payments_phone_number_required;
+            editTitleResId = R.string.payments_add_phone_number;
+            invalidFieldsCount++;
+        }
+
+        List<Integer> requiredFields = AutofillProfileBridge.getRequiredAddressFields(
+                AutofillAddress.getCountryCode(profile));
+        for (int fieldId : requiredFields) {
+            if (fieldId == AddressField.RECIPIENT || fieldId == AddressField.COUNTRY) continue;
+            if (!TextUtils.isEmpty(getProfileField(profile, fieldId))) continue;
+            editMessageResId = R.string.payments_invalid_address;
+            editTitleResId = R.string.payments_add_valid_address;
+            invalidFieldsCount++;
+            break;
+        }
+
+        if (TextUtils.isEmpty(profile.getFullName())) {
+            editMessageResId = R.string.payments_recipient_required;
+            editTitleResId = R.string.payments_add_recipient;
+            invalidFieldsCount++;
+        }
+
+        if (invalidFieldsCount > 1) {
+            editMessageResId = R.string.payments_more_information_required;
+            editTitleResId = R.string.payments_add_more_information;
+        }
+
+        return editMessageResId == 0 ? null : new int[] {editMessageResId, editTitleResId};

[Ted C, 2016/11/08 22:14:06]

not really important here, but I think return a Pair<Integer, Integer> is
slightly better than an int[].  It's not as memory efficient, but this should
have a relatively low call rate (just wouldn't recommend this for something in
the draw path per frame).

[gogerald1, 2016/11/09 01:42:46]

not applicable now, but why it would be more time efficient?

[please use gerrit instead, 2016/11/09 17:04:52]

I think Ted meant that Pair is not as memory efficient as int[], not that other
way around. Number of calls in your code is important for CPU cache
invalidation. There's no Big-O time complexity difference.

[gogerald1, 2016/11/09 17:06:03]

Ah, I think I misunderstand this "low call rate" as "more time efficient" for
one call. Agreed that code clarity is preferred here.

+    }
+
+    /** @return The given autofill profile field. */
+    public static String getProfileField(AutofillProfile profile, int field) {
+        assert profile != null;
+        switch (field) {
+            case AddressField.COUNTRY:
+                return profile.getCountryCode();
+            case AddressField.ADMIN_AREA:
+                return profile.getRegion();
+            case AddressField.LOCALITY:
+                return profile.getLocality();
+            case AddressField.DEPENDENT_LOCALITY:
+                return profile.getDependentLocality();
+            case AddressField.SORTING_CODE:
+                return profile.getSortingCode();
+            case AddressField.POSTAL_CODE:
+                return profile.getPostalCode();
+            case AddressField.STREET_ADDRESS:
+                return profile.getStreetAddress();
+            case AddressField.ORGANIZATION:
+                return profile.getCompanyName();
+            case AddressField.RECIPIENT:
+                return profile.getFullName();
+        }
+
+        assert false;
+        return null;
     }
 
     /** @return The country code to use, e.g., when constructing an editor for this address. */