[Payment Request] Adds address normalization on iOS.

ios/chrome/browser/ui/payments/payment_request_coordinator.mm

 // 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.
 
 #import "ios/chrome/browser/ui/payments/payment_request_coordinator.h"
 
 #include <unordered_set>
 #include <vector>
 
 #include "base/memory/ptr_util.h"
 #include "base/strings/sys_string_conversions.h"
 #include "base/strings/utf_string_conversions.h"
 #include "components/autofill/core/browser/autofill_client.h"
 #include "components/autofill/core/browser/autofill_data_util.h"
 #include "components/autofill/core/browser/autofill_manager.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "components/autofill/core/browser/credit_card.h"
 #include "components/autofill/core/browser/field_types.h"
 #include "components/autofill/core/browser/payments/full_card_request.h"
 #include "components/autofill/core/browser/personal_data_manager.h"
 #include "components/autofill/core/browser/ui/card_unmask_prompt_controller_impl.h"
+#include "components/payments/core/address_normalizer_impl.h"
 #include "components/payments/core/payment_address.h"
 #include "components/payments/core/payment_request_data_util.h"
 #include "components/signin/core/browser/signin_manager.h"
 #include "components/strings/grit/components_strings.h"
 #include "ios/chrome/browser/application_context.h"
+#include "ios/chrome/browser/autofill/validation_rules_storage_factory.h"
 #include "ios/chrome/browser/browser_state/chrome_browser_state.h"
 #include "ios/chrome/browser/payments/payment_request.h"
 #include "ios/chrome/browser/payments/payment_request_util.h"
 #include "ios/chrome/browser/signin/signin_manager_factory.h"
 #include "ios/chrome/browser/ui/autofill/card_unmask_prompt_view_bridge.h"
+#include "third_party/libaddressinput/chromium/chrome_metadata_source.h"
+#include "third_party/libaddressinput/chromium/chrome_storage_impl.h"
 #include "ui/base/l10n/l10n_util.h"
 
 #if !defined(__has_feature) || !__has_feature(objc_arc)
 #error "This file requires ARC support."
 #endif
 
 namespace {
 using ::payments::data_util::GetBasicCardResponseFromAutofillCreditCard;
 using ::payments::data_util::GetPaymentAddressFromAutofillProfile;
-}  // namespace
+
+constexpr int kAddressNormalizationTimeoutSeconds = 5;
+
+// Receives notifications from the AddressNormalizer, and forwards the details
+// to a callback. This is needed as there is currently no way to know, when
+// the |payments::AddressNormalizer::Delegate| methods are called, which address
+// has been normalized (shipping or contact). The callback block can keep track
+// of which address this delegate was created for.
+class AddressNormalizerDelegate : public payments::AddressNormalizer::Delegate {

[Moe, 2017/04/27 12:19:53]

Please move this to its own files in i/c/b/payments. We should avoid putting
non-UI stuff under ui moving forward.
Could you construct this with a reference to the coordinator similar to
FullCardRequester? it can call a function on the coordinator once the delegate
method is called. That way, calling the normalizer for a profile is just a
single line and you'll have named functions in the coordinators that receive the
normalized profiles which you can copy or keep a reference to. 

[macourteau, 2017/04/27 18:07:52]

This is just a helper class, I think it makes sense to keep it near where it's
used, and in the anonymous namespace.
Doing that would render this helper class useless: if I'm calling a named
function on an object, might as well get rid of this class and just implement
OnAddressNormalized/OnCouldNotNormalize on the coordinator directly. The problem
this helper solves is that we can't know, when these delegate methods are
called, which address it was called for (shipping? contact? other?). Using a
block (or could be a base::Callback) lets us keep state to know exactly which
address we're getting called back for.

I suggested that sebsg@ refactor the AddressNormalizer to receive two
base::Callback's instead of a delegate pointer, so that might change and then we
can get rid of this altogether. We'll be able to base::Bind a pointer to the
AutofillProfile in the callback directly, and update that with the normalized
version.

[Moe, 2017/04/28 09:04:44]

Ah I see. Forgot about the need to keep track of the normalized profile.

[macourteau, 2017/05/01 15:56:22]

Acknowledged.

+ public:
+  // Block to be called with the profile information, once normalized.
+  typedef void (^Callback)(const autofill::AutofillProfile&);
+
+  AddressNormalizerDelegate() {}
+  ~AddressNormalizerDelegate() override {}
+
+  void SetCallback(Callback callback) { callback_ = callback; }
+
+  // payments::AddressNormalizer::Delegate:
+  void OnAddressNormalized(
+      const autofill::AutofillProfile& normalizedProfile) override {
+    DCHECK(callback_ != nil);

[Moe, 2017/04/27 12:19:53]

nit? DCHECK(callback_)

[macourteau, 2017/04/27 18:07:52]

Done.

+    callback_(normalizedProfile);
+  }
+
+  // payments::AddressNormalizer::Delegate:
+  void OnCouldNotNormalize(const autofill::AutofillProfile& profile) override {
+    // Since the phone number is formatted in either case, this profile should
+    // be used.
+    DCHECK(callback_ != nil);
+    callback_(profile);
+  }
+
+ private:
+  Callback callback_ = nil;

[Moe, 2017/04/27 12:19:53]

s/nil/nullptr for C++ objects

[macourteau, 2017/04/27 18:07:52]

Done.

+
+  DISALLOW_COPY_AND_ASSIGN(AddressNormalizerDelegate);
+};
 
 // The unmask prompt UI for Payment Request.
 class PRCardUnmaskPromptViewBridge

[Moe, 2017/04/27 12:19:53]

I'll move PRCardUnmaskPromptViewBridge and FullCardRequester to i/c/b/payments
myself.

[macourteau, 2017/04/27 18:07:52]

Acknowledged.

     : public autofill::CardUnmaskPromptViewBridge {
  public:
-  explicit PRCardUnmaskPromptViewBridge(
-      autofill::CardUnmaskPromptController* controller,
-      UIViewController* base_view_controller)
+  PRCardUnmaskPromptViewBridge(autofill::CardUnmaskPromptController* controller,
+                               UIViewController* base_view_controller)
       : autofill::CardUnmaskPromptViewBridge(controller),
         base_view_controller_(base_view_controller) {}
 
   // autofill::CardUnmaskPromptView:
   void Show() override {
     view_controller_.reset(
         [[CardUnmaskPromptViewController alloc] initWithBridge:this]);
     [base_view_controller_ presentViewController:view_controller_
                                         animated:YES
                                       completion:nil];
   };
 
  private:
   __weak UIViewController* base_view_controller_;
   DISALLOW_COPY_AND_ASSIGN(PRCardUnmaskPromptViewBridge);
 };
 
 // Receives the full credit card details. Also displays the unmask prompt UI.
 class FullCardRequester
     : public autofill::payments::FullCardRequest::ResultDelegate,
       public autofill::payments::FullCardRequest::UIDelegate,
       public base::SupportsWeakPtr<FullCardRequester> {
  public:
-  explicit FullCardRequester(PaymentRequestCoordinator* owner,
-                             UIViewController* base_view_controller,
-                             ios::ChromeBrowserState* browser_state)
+  FullCardRequester(PaymentRequestCoordinator* owner,
+                    UIViewController* base_view_controller,
+                    ios::ChromeBrowserState* browser_state)
       : owner_(owner),
         base_view_controller_(base_view_controller),
         unmask_controller_(browser_state->GetPrefs(),
                            browser_state->IsOffTheRecord()) {}
 
   void GetFullCard(autofill::CreditCard* card,
                    autofill::AutofillManager* autofill_manager) {
     DCHECK(card);
     DCHECK(autofill_manager);
     autofill_manager->GetOrCreateFullCardRequest()->GetFullCard(
@@ skipping 34 matching lines @@
   }
 
  private:
   __weak PaymentRequestCoordinator* owner_;
   __weak UIViewController* base_view_controller_;
   autofill::CardUnmaskPromptControllerImpl unmask_controller_;
 
   DISALLOW_COPY_AND_ASSIGN(FullCardRequester);
 };
 
+}  // namespace
+
 @implementation PaymentRequestCoordinator {
   UINavigationController* _navigationController;
   PaymentRequestViewController* _viewController;
   PaymentRequestErrorCoordinator* _errorCoordinator;
   PaymentItemsDisplayCoordinator* _itemsDisplayCoordinator;
   ShippingAddressSelectionCoordinator* _shippingAddressSelectionCoordinator;
   ShippingOptionSelectionCoordinator* _shippingOptionSelectionCoordinator;
   PaymentMethodSelectionCoordinator* _methodSelectionCoordinator;
 
   // Receiver of the full credit card details. Also displays the unmask prompt
   // UI.
   std::unique_ptr<FullCardRequester> _fullCardRequester;
 
   // The selected shipping address, pending approval from the page.
   autofill::AutofillProfile* _pendingShippingAddress;
+
+  // The address normalizer, to normalize contact & shipping addresses. Also,
+  // delegates to handle callbacks for normalization of each address.
+  std::unique_ptr<payments::AddressNormalizer> _addressNormalizer;
+  AddressNormalizerDelegate _contactAddressNormalizerDelegate;
+  AddressNormalizerDelegate _shippingAddressNormalizerDelegate;
+
+  // Member variables to keep track of which operations are still pending.
+  BOOL _contactAddressNormalizationPending;
+  BOOL _shippingAddressNormalizationPending;
+  BOOL _fullCardRequestPending;
+
+  // Storage for the full card.
+  struct {
+    autofill::CreditCard creditCard;
+    base::string16 cvc;
+  } _fullCard;

[Moe, 2017/04/27 12:19:52]

please typedef this.

[macourteau, 2017/04/27 18:07:52]

Done.

 }
 
 @synthesize paymentRequest = _paymentRequest;
 @synthesize autofillManager = _autofillManager;
 @synthesize browserState = _browserState;
 @synthesize pageFavicon = _pageFavicon;
 @synthesize pageTitle = _pageTitle;
 @synthesize pageHost = _pageHost;
 @synthesize delegate = _delegate;
 
+- (void)initAddressNormalizer {
+  autofill::PersonalDataManager* personalDataManager =
+      _paymentRequest->GetPersonalDataManager();
+
+  std::unique_ptr<i18n::addressinput::Source> addressNormalizerSource =
+      base::WrapUnique(new autofill::ChromeMetadataSource(

[Moe, 2017/04/27 12:19:53]

MakeUnique is preferred.

[macourteau, 2017/04/27 18:07:52]

Done.

+          I18N_ADDRESS_VALIDATION_DATA_URL,
+          personalDataManager->GetURLRequestContextGetter()));
+
+  std::unique_ptr<i18n::addressinput::Storage> addressNormalizerStorage =
+      autofill::ValidationRulesStorageFactory::CreateStorage();
+
+  _addressNormalizer.reset(new payments::AddressNormalizerImpl(
+      std::move(addressNormalizerSource), std::move(addressNormalizerStorage)));
+
+  // Kickoff the process of loading the rules (which is asynchronous) for each
+  // profile's country, to get faster address normalization later.
+  for (const autofill::AutofillProfile* profile :
+       personalDataManager->GetProfilesToSuggest()) {
+    std::string countryCode =
+        base::UTF16ToUTF8(profile->GetRawInfo(autofill::ADDRESS_HOME_COUNTRY));
+    if (autofill::data_util::IsValidCountryCode(countryCode)) {
+      _addressNormalizer->LoadRulesForRegion(countryCode);
+    }

[sebsg, 2017/04/26 20:52:51]

I don't know if you have access to that here but it could also be useful to load
the rules of the app locale.

+  }
+}
+
 - (void)start {
   _viewController = [[PaymentRequestViewController alloc]
       initWithPaymentRequest:_paymentRequest];
   [_viewController setPageFavicon:_pageFavicon];
   [_viewController setPageTitle:_pageTitle];
   [_viewController setPageHost:_pageHost];
   [_viewController setDelegate:self];
   DCHECK(_browserState);
   const SigninManager* signinManager =
       ios::SigninManagerFactory::GetForBrowserStateIfExists(_browserState);
   if (signinManager && signinManager->IsAuthenticated()) {
     NSString* accountName = base::SysUTF8ToNSString(
         signinManager->GetAuthenticatedAccountInfo().email);
     [_viewController setAuthenticatedAccountName:accountName];
   }
   [_viewController loadModel];
 
   _navigationController = [[UINavigationController alloc]
       initWithRootViewController:_viewController];
   [_navigationController setNavigationBarHidden:YES];
 
   [[self baseViewController] presentViewController:_navigationController
                                           animated:YES
                                         completion:nil];
+
+  [self initAddressNormalizer];
 }
 
 - (void)stop {
   [[_navigationController presentingViewController]
       dismissViewControllerAnimated:YES
                          completion:nil];
   [_itemsDisplayCoordinator stop];
   _itemsDisplayCoordinator = nil;
   [_shippingAddressSelectionCoordinator stop];
   _shippingAddressSelectionCoordinator = nil;
   [_shippingOptionSelectionCoordinator stop];
   _shippingOptionSelectionCoordinator = nil;
   [_methodSelectionCoordinator stop];
   _methodSelectionCoordinator = nil;
   [_errorCoordinator stop];
   _errorCoordinator = nil;
   _viewController = nil;
   _navigationController = nil;
 }
 
+- (void)normalizeProfile:(autofill::AutofillProfile*)profileToNormalize
+                delegate:(AddressNormalizerDelegate*)delegate
+                    flag:(BOOL*)normalizationRequestPendingFlag {
+  DCHECK(normalizationRequestPendingFlag);
+
+  __weak PaymentRequestCoordinator* weakSelf = self;
+
+  delegate->SetCallback(^(const autofill::AutofillProfile& normalizedProfile) {
+    // Check that PaymentRequestCoordinator still exists as |profileToNormalize|
+    // and |normalizationRequestPendingFlag| are member variables of that
+    // object.
+    PaymentRequestCoordinator* strongSelf = weakSelf;
+    if (!strongSelf)
+      return;
+
+    *profileToNormalize = normalizedProfile;
+    *normalizationRequestPendingFlag = NO;
+    [strongSelf maybeDone];
+  });
+
+  const std::string country_code = base::UTF16ToUTF8(
+      profileToNormalize->GetRawInfo(autofill::ADDRESS_HOME_COUNTRY));
+  if (autofill::data_util::IsValidCountryCode(country_code)) {

[sebsg, 2017/04/26 20:52:51]

If the country code is not valid, you could fallback to the one inferred from
the app_locale.

To get the country code from the app_locale:
autofill::AutofillCountry::CountryCodeForLocale(app_locale_);

[macourteau, 2017/04/27 18:07:52]

Done (with iOS APIs).

+    _addressNormalizer->StartAddressNormalization(
+        *profileToNormalize, country_code, kAddressNormalizationTimeoutSeconds,
+        delegate);
+  } else {
+    *normalizationRequestPendingFlag = NO;
+  }
+}
+
 - (void)sendPaymentResponse {
+  // Set these before starting any potentially asynchronous calls, to ensure we
+  // wait for completion of all operations before proceeding.
+  _contactAddressNormalizationPending = YES;
+  _shippingAddressNormalizationPending = _paymentRequest->request_shipping();
+  _fullCardRequestPending = YES;
+
   DCHECK(_paymentRequest->selected_credit_card());
   autofill::CreditCard* card = _paymentRequest->selected_credit_card();
   _fullCardRequester = base::MakeUnique<FullCardRequester>(
       self, _navigationController, _browserState);
   _fullCardRequester->GetFullCard(card, _autofillManager);
+
+  if (_paymentRequest->request_shipping()) {
+    [self normalizeProfile:_paymentRequest->selected_shipping_profile()
+                  delegate:&_shippingAddressNormalizerDelegate
+                      flag:&_shippingAddressNormalizationPending];
+  }
+
+  [self normalizeProfile:_paymentRequest->selected_contact_profile()
+                delegate:&_contactAddressNormalizerDelegate
+                    flag:&_contactAddressNormalizationPending];
 }
 
 - (void)fullCardRequestDidSucceedWithCard:(const autofill::CreditCard&)card
                                       CVC:(const base::string16&)cvc {
-  web::PaymentResponse paymentResponse;
-
-  // If the merchant specified the card network as part of the "basic-card"
-  // payment method, return "basic-card" as the method_name. Otherwise, return
-  // the name of the network directly.
-  std::string basic_card_type =
-      autofill::data_util::GetPaymentRequestData(card.type())
-          .basic_card_payment_type;
-  paymentResponse.method_name =
-      _paymentRequest->basic_card_specified_networks().find(basic_card_type) !=
-              _paymentRequest->basic_card_specified_networks().end()
-          ? base::ASCIIToUTF16("basic-card")
-          : base::ASCIIToUTF16(basic_card_type);
-
-  paymentResponse.details = GetBasicCardResponseFromAutofillCreditCard(
-      card, cvc, _paymentRequest->billing_profiles(),
-      GetApplicationContext()->GetApplicationLocale());
-
-  if (_paymentRequest->request_shipping()) {
-    autofill::AutofillProfile* shippingAddress =
-        _paymentRequest->selected_shipping_profile();
-    // TODO(crbug.com/602666): User should get here only if they have selected
-    // a shipping address.
-    DCHECK(shippingAddress);
-    paymentResponse.shipping_address = GetPaymentAddressFromAutofillProfile(
-        *shippingAddress, GetApplicationContext()->GetApplicationLocale());
-
-    web::PaymentShippingOption* shippingOption =
-        _paymentRequest->selected_shipping_option();
-    DCHECK(shippingOption);
-    paymentResponse.shipping_option = shippingOption->id;
-  }
-
-  if (_paymentRequest->request_payer_name()) {
-    autofill::AutofillProfile* contactInfo =
-        _paymentRequest->selected_contact_profile();
-    // TODO(crbug.com/602666): User should get here only if they have selected
-    // a contact info.
-    DCHECK(contactInfo);
-    paymentResponse.payer_name =
-        contactInfo->GetInfo(autofill::AutofillType(autofill::NAME_FULL),
-                             GetApplicationContext()->GetApplicationLocale());
-  }
-
-  if (_paymentRequest->request_payer_email()) {
-    autofill::AutofillProfile* contactInfo =
-        _paymentRequest->selected_contact_profile();
-    // TODO(crbug.com/602666): User should get here only if they have selected
-    // a contact info.
-    DCHECK(contactInfo);
-    paymentResponse.payer_email =
-        contactInfo->GetRawInfo(autofill::EMAIL_ADDRESS);
-  }
-
-  if (_paymentRequest->request_payer_phone()) {
-    autofill::AutofillProfile* contactInfo =
-        _paymentRequest->selected_contact_profile();
-    // TODO(crbug.com/602666): User should get here only if they have selected
-    // a contact info.
-    DCHECK(contactInfo);
-    paymentResponse.payer_phone =
-        contactInfo->GetRawInfo(autofill::PHONE_HOME_WHOLE_NUMBER);
-  }
+  _fullCard.creditCard = card;
+  _fullCard.cvc = cvc;
+  _fullCardRequestPending = NO;
 
   _viewController.view.userInteractionEnabled = NO;
   [_viewController setPending:YES];
   [_viewController loadModel];
   [[_viewController collectionView] reloadData];
 
-  [_delegate paymentRequestCoordinator:self
-         didConfirmWithPaymentResponse:paymentResponse];
+  [self maybeDone];
+}
+
+- (void)maybeDone {

[Moe, 2017/04/27 12:19:53]

nit: please change to something more descriptive like
sendPaymentResponseIfAllAsyncCallsCompleted
dont' be shy to be verbose in Objective-C :)

[macourteau, 2017/04/27 18:07:52]

Done. That *is* quite verbose 😳

+  if (_contactAddressNormalizationPending == NO &&
+      _shippingAddressNormalizationPending == NO &&
+      _fullCardRequestPending == NO) {
+    web::PaymentResponse paymentResponse;
+
+    // If the merchant specified the card network as part of the "basic-card"
+    // payment method, return "basic-card" as the method_name. Otherwise, return
+    // the name of the network directly.
+    std::string basic_card_type =
+        autofill::data_util::GetPaymentRequestData(_fullCard.creditCard.type())
+            .basic_card_payment_type;
+    paymentResponse.method_name =
+        _paymentRequest->basic_card_specified_networks().find(
+            basic_card_type) !=
+                _paymentRequest->basic_card_specified_networks().end()
+            ? base::ASCIIToUTF16("basic-card")
+            : base::ASCIIToUTF16(basic_card_type);
+
+    paymentResponse.details = GetBasicCardResponseFromAutofillCreditCard(
+        _fullCard.creditCard, _fullCard.cvc,
+        _paymentRequest->billing_profiles(),

[sebsg, 2017/04/26 20:52:51]

Also, you will eventually want to normalize that billing address too, shouldn't
be complicated with the nice framework for normalization you created.

[sebsg, 2017/04/26 20:52:51]

Just a heads-up I just landed a CL which adds a bit more behavior here,
basically finding the associated billing address and only sending that one to
GetBasicCardResponseFromAutofillCreditCard.

[macourteau, 2017/04/27 18:07:52]

Acknowledged, taken into account.

[macourteau, 2017/04/27 18:07:52]

Done.

+        GetApplicationContext()->GetApplicationLocale());
+
+    if (_paymentRequest->request_shipping()) {
+      autofill::AutofillProfile* shippingAddress =
+          _paymentRequest->selected_shipping_profile();
+      // TODO(crbug.com/602666): User should get here only if they have selected
+      // a shipping address.
+      DCHECK(shippingAddress);
+      paymentResponse.shipping_address = GetPaymentAddressFromAutofillProfile(
+          *shippingAddress, GetApplicationContext()->GetApplicationLocale());
+
+      web::PaymentShippingOption* shippingOption =
+          _paymentRequest->selected_shipping_option();
+      DCHECK(shippingOption);
+      paymentResponse.shipping_option = shippingOption->id;
+    }
+
+    if (_paymentRequest->request_payer_name()) {
+      autofill::AutofillProfile* contactInfo =
+          _paymentRequest->selected_contact_profile();
+      // TODO(crbug.com/602666): User should get here only if they have selected
+      // a contact info.
+      DCHECK(contactInfo);
+      paymentResponse.payer_name =
+          contactInfo->GetInfo(autofill::AutofillType(autofill::NAME_FULL),
+                               GetApplicationContext()->GetApplicationLocale());
+    }
+
+    if (_paymentRequest->request_payer_email()) {
+      autofill::AutofillProfile* contactInfo =
+          _paymentRequest->selected_contact_profile();
+      // TODO(crbug.com/602666): User should get here only if they have selected
+      // a contact info.
+      DCHECK(contactInfo);
+      paymentResponse.payer_email =
+          contactInfo->GetRawInfo(autofill::EMAIL_ADDRESS);
+    }
+
+    if (_paymentRequest->request_payer_phone()) {
+      autofill::AutofillProfile* contactInfo =
+          _paymentRequest->selected_contact_profile();
+      // TODO(crbug.com/602666): User should get here only if they have selected
+      // a contact info.
+      DCHECK(contactInfo);
+      paymentResponse.payer_phone =
+          contactInfo->GetRawInfo(autofill::PHONE_HOME_WHOLE_NUMBER);
+    }
+
+    [_delegate paymentRequestCoordinator:self
+           didConfirmWithPaymentResponse:paymentResponse];
+  }
 }
 
 - (void)updatePaymentDetails:(web::PaymentDetails)paymentDetails {
   BOOL totalValueChanged =
       (_paymentRequest->payment_details().total != paymentDetails.total);
   _paymentRequest->set_payment_details(paymentDetails);
 
   if (_paymentRequest->shipping_options().empty()) {
     // Display error in the shipping address/option selection view.
     if (_shippingAddressSelectionCoordinator) {
@@ skipping 189 matching lines @@
 - (void)paymentMethodSelectionCoordinatorDidReturn:
     (PaymentMethodSelectionCoordinator*)coordinator {
   // Clear the 'Updated' label on the payment summary item, if there is one.
   [_viewController updatePaymentSummaryWithTotalValueChanged:NO];
 
   [_methodSelectionCoordinator stop];
   _methodSelectionCoordinator = nil;
 }
 
 @end