[Payments] Format shipping and billing phone number in normalizer.

components/payments/core/address_normalizer.cc

 // Copyright 2017 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.
 
 #include "components/payments/core/address_normalizer.h"
 
 #include <stddef.h>
 #include <utility>
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/cancelable_callback.h"
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/threading/sequenced_task_runner_handle.h"
 #include "base/time/time.h"
 #include "components/autofill/core/browser/address_i18n.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "third_party/libaddressinput/chromium/chrome_address_validator.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/address_data.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/source.h"
 #include "third_party/libaddressinput/src/cpp/include/libaddressinput/storage.h"
+#include "third_party/libphonenumber/phonenumber_api.h"
 
 namespace payments {
 namespace {
 
 using ::autofill::AutofillProfile;
 using ::i18n::addressinput::Source;
 using ::i18n::addressinput::Storage;
+using ::i18n::phonenumbers::PhoneNumberUtil;
 
 class AddressNormalizationRequest : public AddressNormalizer::Request {
  public:
   // The |delegate| and |address_validator| need to outlive this Request.
   AddressNormalizationRequest(const AutofillProfile& profile,
                               const std::string& region_code,
                               int timeout_seconds,
                               AddressNormalizer::Delegate* delegate,
                               autofill::AddressValidator* address_validator)
       : profile_(profile),
@@ skipping 13 matching lines @@
   ~AddressNormalizationRequest() override {}
 
   void OnRulesLoaded(bool success) override {
     on_timeout_.Cancel();
 
     // Check if the timeout happened before the rules were loaded.
     if (has_responded_)
       return;
     has_responded_ = true;
 
+    // In either case, format the phone number.
+    FormatPhoneNumber();

[Mathieu, 2017/04/11 18:53:31]

is there a possibility we early return above and never normalize the phone?

[sebsg, 2017/04/11 21:34:54]

It should not, the first time this method is called the phone will be formatted.

+
     if (!success) {
       delegate_->OnCouldNotNormalize(profile_);
       return;
     }
 
     // The rules should be loaded.
     DCHECK(address_validator_->AreRulesLoadedForRegion(region_code_));
 
     // Create the AddressData from the profile.
     ::i18n::addressinput::AddressData address_data =
         *autofill::i18n::CreateAddressDataFromAutofillProfile(profile_,
                                                               region_code_);
 
     // Normalize the address.
     if (address_validator_ &&
         address_validator_->NormalizeAddress(&address_data)) {
       profile_.SetRawInfo(autofill::ADDRESS_HOME_STATE,
                           base::UTF8ToUTF16(address_data.administrative_area));
       profile_.SetRawInfo(autofill::ADDRESS_HOME_CITY,
                           base::UTF8ToUTF16(address_data.locality));
       profile_.SetRawInfo(autofill::ADDRESS_HOME_DEPENDENT_LOCALITY,
                           base::UTF8ToUTF16(address_data.dependent_locality));
     }
 
     delegate_->OnAddressNormalized(profile_);
   }
 
  private:
+  void FormatPhoneNumber() {

[Mathieu, 2017/04/11 18:53:31]

Add a function comment, mentioning the format and so forth

[sebsg, 2017/04/11 21:34:55]

Done.

+    const std::string original_number = base::UTF16ToUTF8(
+        profile_.GetRawInfo(autofill::PHONE_HOME_WHOLE_NUMBER));

[Mathieu, 2017/04/11 18:53:31]

GetRawInfo vs GetInfo? I seem to remember it matters for phone numbers

[sebsg, 2017/04/11 21:34:55]

I changed it to GetInfo to be safe, but it seems like you cannot set phone parts
with SetRawInfo or SetInfo, you can only Get*Raw*Info the parts. Thanks for
spotting this

+    i18n::phonenumbers::PhoneNumber parsed_number;
+    PhoneNumberUtil* phone_number_util = PhoneNumberUtil::GetInstance();
+    if (phone_number_util->Parse(original_number, region_code_,
+                                 &parsed_number) ==
+        PhoneNumberUtil::NO_PARSING_ERROR) {
+      std::string formatted_number;
+      phone_number_util->Format(parsed_number,
+                                PhoneNumberUtil::PhoneNumberFormat::E164,
+                                &formatted_number);
+      profile_.SetRawInfo(autofill::PHONE_HOME_WHOLE_NUMBER,
+                          base::UTF8ToUTF16(formatted_number));
+    }
+  }
+
   AutofillProfile profile_;
   std::string region_code_;
   AddressNormalizer::Delegate* delegate_;
   autofill::AddressValidator* address_validator_;
 
   bool has_responded_;
   base::CancelableCallback<void()> on_timeout_;
 
   DISALLOW_COPY_AND_ASSIGN(AddressNormalizationRequest);
 };
@@ skipping 57 matching lines @@
   auto it = pending_normalization_.find(region_code);
   if (it != pending_normalization_.end()) {
     for (size_t i = 0; i < it->second.size(); ++i) {
       it->second[i]->OnRulesLoaded(success);
     }
     pending_normalization_.erase(it);
   }
 }
 
 }  // namespace payments