Edit contacts UI for PaymentRequest

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

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
index dd79937cb06abae042910fb64925219e3a914084..1182f4e938859c87e1ecc07437435fe2a5e79284 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
@@ -7,14 +7,20 @@ package org.chromium.chrome.browser.payments;
 import android.app.Activity;
 import android.graphics.Bitmap;
 import android.os.Handler;
+import android.telephony.PhoneNumberUtils;
 import android.text.TextUtils;
+import android.util.Patterns;
 
 import org.chromium.base.Callback;
 import org.chromium.base.Log;
 import org.chromium.base.VisibleForTesting;
+import org.chromium.chrome.R;
 import org.chromium.chrome.browser.autofill.PersonalDataManager;
 import org.chromium.chrome.browser.autofill.PersonalDataManager.AutofillProfile;
 import org.chromium.chrome.browser.favicon.FaviconHelper;
+import org.chromium.chrome.browser.payments.ui.EditorFieldModel;
+import org.chromium.chrome.browser.payments.ui.EditorFieldModel.EditorFieldValidator;
+import org.chromium.chrome.browser.payments.ui.EditorModel;
 import org.chromium.chrome.browser.payments.ui.LineItem;
 import org.chromium.chrome.browser.payments.ui.PaymentInformation;
 import org.chromium.chrome.browser.payments.ui.PaymentOption;
@@ -23,6 +29,7 @@ import org.chromium.chrome.browser.payments.ui.SectionInformation;
 import org.chromium.chrome.browser.payments.ui.ShoppingCart;
 import org.chromium.chrome.browser.preferences.PreferencesLauncher;
 import org.chromium.chrome.browser.preferences.autofill.AutofillCreditCardEditor;
+import org.chromium.chrome.browser.preferences.autofill.AutofillPreferences;
 import org.chromium.chrome.browser.preferences.autofill.AutofillProfileEditor;
 import org.chromium.chrome.browser.profiles.Profile;
 import org.chromium.chrome.browser.util.UrlUtilities;
@@ -129,6 +136,12 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
     private Pattern mRegionCodePattern;
     private boolean mMerchantNeedsShippingAddress;
     private boolean mPaymentAppRunning;
+    private boolean mRequestPayerEmail;
+    private boolean mRequestPayerPhone;
+    private List<CharSequence> mAllEmailAddresses;
+    private List<CharSequence> mAllPhoneNumbers;
+    private EditorFieldValidator mPhoneValidator;
+    private EditorFieldValidator mEmailValidator;
 
     /**
      * Builds the PaymentRequest service implementation.
@@ -215,21 +228,34 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
         mMerchantNeedsShippingAddress =
                 requestShipping && mUiShippingOptions.getSelectedItem() == null;
 
-        boolean requestPayerEmail = options != null && options.requestPayerEmail;
-        boolean requestPayerPhone = options != null && options.requestPayerPhone;
+        mRequestPayerEmail = options != null && options.requestPayerEmail;
+        mRequestPayerPhone = options != null && options.requestPayerPhone;
 
-        if (requestShipping || requestPayerEmail || requestPayerPhone) {
+        if (requestShipping || mRequestPayerEmail || mRequestPayerPhone) {
             List<AutofillProfile> profiles =
                     PersonalDataManager.getInstance().getProfilesToSuggest();
             List<AutofillContact> contacts = new ArrayList<>();
             List<AutofillAddress> addresses = new ArrayList<>();
+            mAllEmailAddresses = new ArrayList<>();
+            mAllPhoneNumbers = new ArrayList<>();
+            int topCompleteContactIndex = SectionInformation.NO_SELECTION;

[gone, 2016/06/21 06:04:38]

nit: firstCompleteContactIndex?

[please use gerrit instead, 2016/06/22 02:12:29]

Done.

             for (int i = 0; i < profiles.size(); i++) {
                 AutofillProfile profile = profiles.get(i);
-                if (canUseContactDetails(profile, requestPayerEmail, requestPayerPhone)) {
-                    contacts.add(new AutofillContact(
-                            requestPayerEmail ? profile.getEmailAddress() : null,
-                            requestPayerPhone ? profile.getPhoneNumber() : null));
+
+                String email = mRequestPayerEmail && !TextUtils.isEmpty(profile.getEmailAddress())
+                        ? profile.getEmailAddress()

[gone, 2016/06/21 06:04:38]

nit: I don't think we're supposed to break on the colons

[please use gerrit instead, 2016/06/22 02:12:29]

Done.

+                        : null;
+                String phone = mRequestPayerPhone && !TextUtils.isEmpty(profile.getPhoneNumber())
+                        ? profile.getPhoneNumber()
+                        : null;
+                if (email != null || phone != null) {
+                    boolean isComplete = isCompleteContactInformation(email, phone);
+                    contacts.add(new AutofillContact(profile, email, phone, isComplete));
+                    if (isComplete && topCompleteContactIndex < 0) topCompleteContactIndex = i;
+                    if (getEmailValidator().isValid(email)) mAllEmailAddresses.add(email);
+                    if (getPhoneValidator().isValid(phone)) mAllPhoneNumbers.add(phone);
                 }
+
                 if (canUseAddress(profile, requestShipping)) {
                     addresses.add(new AutofillAddress(profile));
                 }
@@ -244,9 +270,9 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
                         PaymentRequestUI.TYPE_SHIPPING_ADDRESSES, selectedIndex, addresses);
             }
 

[gone, 2016/06/21 06:04:38]

// The contact section automatically selects the first complete entry.

[please use gerrit instead, 2016/06/22 02:12:29]

Done.

-            if (requestPayerEmail || requestPayerPhone) {
-                mContactSection = new SectionInformation(PaymentRequestUI.TYPE_CONTACT_DETAILS,
-                        contacts.isEmpty() ? SectionInformation.NO_SELECTION : 0, contacts);
+            if (mRequestPayerEmail || mRequestPayerPhone) {
+                mContactSection = new SectionInformation(
+                        PaymentRequestUI.TYPE_CONTACT_DETAILS, topCompleteContactIndex, contacts);
             }
         }
 
@@ -271,7 +297,7 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
         }
 
         mUI = new PaymentRequestUI(mContext, this, requestShipping,
-                requestPayerEmail || requestPayerPhone, mMerchantName, mOrigin);
+                mRequestPayerEmail || mRequestPayerPhone, mMerchantName, mOrigin);
         if (mFavicon != null) mUI.setTitleBitmap(mFavicon);
         mFavicon = null;
     }
@@ -286,7 +312,7 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
             if (!TextUtils.isEmpty(methodData[i].stringifiedData)) {
                 try {
                     data = new JSONObject(JsonSanitizer.sanitize(methodData[i].stringifiedData));
-                } catch (JSONException | IOException  | IllegalStateException e) {
+                } catch (JSONException | IOException | IllegalStateException e) {
                     // Payment method specific data should be a JSON object.
                     // According to the payment request spec[1], for each method data,
                     // if the data field is supplied but is not a JSON-serializable object,
@@ -311,10 +337,9 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
         return result;
     }
 
-    private static boolean canUseContactDetails(
-            AutofillProfile profile, boolean requestEmail, boolean requestPhone) {
-        return (requestEmail && !TextUtils.isEmpty(profile.getEmailAddress()))
-                || (requestPhone && !TextUtils.isEmpty(profile.getPhoneNumber()));
+    private boolean isCompleteContactInformation(String email, String phone) {

[gone, 2016/06/21 06:04:38]

isContactInformationComplete?

[please use gerrit instead, 2016/06/22 02:12:29]

Done.

+        return (!mRequestPayerEmail || getEmailValidator().isValid(email))
+                && (!mRequestPayerPhone || getPhoneValidator().isValid(phone));
     }
 
     private boolean canUseAddress(AutofillProfile profile, boolean requestShipping) {
@@ -585,7 +610,12 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
             mClient.onShippingOptionChange(option.getIdentifier());
         } else if (optionType == PaymentRequestUI.TYPE_CONTACT_DETAILS) {
             assert option instanceof AutofillContact;
-            mContactSection.setSelectedItem(option);
+            AutofillContact contact = (AutofillContact) option;
+            if (contact.isComplete()) {
+                mContactSection.setSelectedItem(option);
+            } else {
+                editContact(contact);
+            }
         } else if (optionType == PaymentRequestUI.TYPE_PAYMENT_METHODS) {
             assert option instanceof PaymentInstrument;
             mPaymentMethodsSection.setSelectedItem(option);
@@ -593,14 +623,108 @@ public class PaymentRequestImpl implements PaymentRequest, PaymentRequestUI.Clie
         return false;
     }
 
+    private void editContact(final AutofillContact contact) {
+        final EditorFieldModel phoneField = mRequestPayerPhone
+                ? new EditorFieldModel(EditorFieldModel.INPUT_TYPE_HINT_PHONE,
+                          mContext.getString(R.string.autofill_profile_editor_phone_number),
+                          mAllPhoneNumbers, getPhoneValidator(),
+                          mContext.getString(R.string.payments_phone_required_validation_message),
+                          mContext.getString(R.string.payments_phone_invalid_validation_message),
+                          contact == null ? null : contact.getPayerPhone())
+                : null;
+
+        final EditorFieldModel emailField = mRequestPayerEmail
+                ? new EditorFieldModel(EditorFieldModel.INPUT_TYPE_HINT_EMAIL,
+                          mContext.getString(R.string.autofill_profile_editor_email_address),
+                          mAllEmailAddresses, getEmailValidator(),
+                          mContext.getString(R.string.payments_email_required_validation_message),
+                          mContext.getString(R.string.payments_email_invalid_validation_message),
+                          contact == null ? null : contact.getPayerEmail())
+                : null;
+
+        EditorModel editor =
+                new EditorModel(mContext.getString(R.string.payments_add_contact_details_label));
+        if (phoneField != null) editor.addField(phoneField);
+        if (emailField != null) editor.addField(emailField);
+
+        editor.setCancelCallback(new Runnable() {
+            @Override
+            public void run() {
+                mContactSection.setSelectedItemIndex(SectionInformation.NO_SELECTION);
+                mUI.updateSection(PaymentRequestUI.TYPE_CONTACT_DETAILS, mContactSection);
+            }
+        });
+
+        editor.setDoneCallback(new Runnable() {
+            @Override
+            public void run() {
+                Locale defaultLocale = Locale.getDefault();
+                AutofillProfile profile = contact != null
+                        ? contact.getProfile()
+                        : new AutofillProfile("" /* guid */, AutofillPreferences.SETTINGS_ORIGIN,

[gone, 2016/06/21 06:04:38]

You should just have an empty AutofillProfile constructor.  Passing in a bunch
of empty strings and then describing what each empty string is seems kind of
silly.

[please use gerrit instead, 2016/06/22 02:12:29]

Done.

+                                  true /* isLocal */, "" /* fullName */, "" /* companyName */,
+                                  "" /* streetAddress */, "" /* region */, "" /* locality */,
+                                  "" /* dependentLocality */, "" /* postalCode */,
+                                  "" /* sortingCode */, defaultLocale.getCountry(), "" /* phone */,
+                                  "" /* email */, "" /* languageTag */);
+                String phone = null;
+                String email = null;
+                if (phoneField != null) {
+                    phone = phoneField.getValue().toString();
+                    profile.setPhoneNumber(phone);
+                }
+                if (emailField != null) {
+                    email = emailField.getValue().toString();
+                    profile.setEmailAddress(email);
+                }
+                PersonalDataManager.getInstance().setProfile(profile);
+
+                if (contact == null) {
+                    mContactSection.addAndSelectItem(
+                            new AutofillContact(profile, email, phone, true));
+                } else {
+                    contact.setPayerPhone(phone);
+                    contact.setPayerEmail(email);
+                    contact.setComplete();
+                }
+                mUI.updateSection(PaymentRequestUI.TYPE_CONTACT_DETAILS, mContactSection);
+            }
+        });
+        mUI.showEditor(editor);
+    }
+
+    private EditorFieldValidator getPhoneValidator() {
+        if (mPhoneValidator == null) {
+            mPhoneValidator = new EditorFieldValidator() {
+                @Override
+                public boolean isValid(CharSequence value) {
+                    return value != null
+                            && PhoneNumberUtils.isGlobalPhoneNumber(
+                                       PhoneNumberUtils.stripSeparators(value.toString()));

[please use gerrit instead, 2016/06/20 14:35:53]

Is there a better way to validate phone numbers? This is too permissive in my
experience.

[gone, 2016/06/21 06:04:38]

Haven't had to do it before, so I wouldn't know :/

[please use gerrit instead, 2016/06/22 02:12:29]

Acknowledged.

+                }
+            };
+        }
+        return mPhoneValidator;
+    }
+
+    private EditorFieldValidator getEmailValidator() {
+        if (mEmailValidator == null) {
+            mEmailValidator = new EditorFieldValidator() {
+                @Override
+                public boolean isValid(CharSequence value) {
+                    return value != null && Patterns.EMAIL_ADDRESS.matcher(value).matches();
+                }
+            };
+        }
+        return mEmailValidator;
+    }
 
     @Override
     public void onSectionAddOption(@PaymentRequestUI.DataType int optionType) {
-        // TODO(rouslan, dfalcantara): Make this code do something more useful.
         if (optionType == PaymentRequestUI.TYPE_SHIPPING_ADDRESSES) {
             PreferencesLauncher.launchSettingsPage(mContext, AutofillProfileEditor.class.getName());
         } else if (optionType == PaymentRequestUI.TYPE_CONTACT_DETAILS) {
-            PreferencesLauncher.launchSettingsPage(mContext, AutofillProfileEditor.class.getName());
+            editContact(null);
         } else if (optionType == PaymentRequestUI.TYPE_PAYMENT_METHODS) {
             PreferencesLauncher.launchSettingsPage(
                     mContext, AutofillCreditCardEditor.class.getName());