retry r226004: rAc: phone number cleanup

original review: https://codereview.chromium.org/24538008/
difference to original review: android test updated.

1) fill Wallet phone number data in a format that's more similar to Autofill (i.e. whole number without any formatting)
2) display Wallet phone numbers in rAc in US-national format
3) display Autofill phone numbers in rAc in user-supplied format, unless there is no formatting. In that case add formatting.

BUG=297204
R=isherman@chromium.org

Review URL: https://codereview.chromium.org/25092011

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@226415 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 74 insertions, 40 deletions

diff --git a/components/autofill/content/browser/wallet/wallet_address.cc b/components/autofill/content/browser/wallet/wallet_address.cc
index 8ab5998..67794e3 100644
--- a/components/autofill/content/browser/wallet/wallet_address.cc
+++ b/components/autofill/content/browser/wallet/wallet_address.cc
@@ -11,6 +11,7 @@
 #include "components/autofill/core/browser/autofill_country.h"
 #include "components/autofill/core/browser/autofill_profile.h"
 #include "components/autofill/core/browser/autofill_type.h"
+#include "components/autofill/core/browser/phone_number.h"
 #include "components/autofill/core/browser/state_names.h"
 
 namespace autofill {
@@ -109,6 +110,9 @@ Address::Address(const AutofillProfile& profile)
                                       NULL,
                                       &administrative_area_name_);
   StringToUpperASCII(&administrative_area_name_);
+
+  if (!country_name_code_.empty())
+    phone_object_ = i18n::PhoneObject(phone_number_, country_name_code_);
 }
 
 Address::Address(const std::string& country_name_code,
@@ -128,9 +132,9 @@ Address::Address(const std::string& country_name_code,
       administrative_area_name_(administrative_area_name),
       postal_code_number_(postal_code_number),
       phone_number_(phone_number),
+      phone_object_(phone_number, country_name_code),
       object_id_(object_id),
-      is_complete_address_(true) {
-}
+      is_complete_address_(true) {}
 
 Address::~Address() {}
 
@@ -262,6 +266,15 @@ string16 Address::DisplayNameDetail() const {
 #endif
 }
 
+string16 Address::DisplayPhoneNumber() const {
+  // Return a formatted phone number. Wallet doesn't store user formatting, so
+  // impose our own. phone_number() always includes a country code, so using
+  // PhoneObject to format it would result in an internationalized format. Since
+  // Wallet only supports the US right now, stick to national formatting.
+  return i18n::PhoneObject(phone_number(), country_name_code()).
+      GetNationallyFormattedNumber();
+}
+
 string16 Address::GetInfo(const AutofillType& type,
                           const std::string& app_locale) const {
   if (type.html_type() == HTML_TYPE_COUNTRY_CODE) {
@@ -299,7 +312,9 @@ string16 Address::GetInfo(const AutofillType& type,
     }
 
     case PHONE_HOME_WHOLE_NUMBER:
-      return phone_number();
+      // Wallet doesn't store user phone number formatting, so just strip all
+      // formatting.
+      return phone_object_.GetWholeNumber();
 
     // TODO(estade): implement more.
     default:
@@ -308,6 +323,11 @@ string16 Address::GetInfo(const AutofillType& type,
   }
 }
 
+void Address::SetPhoneNumber(const base::string16& phone_number) {
+  phone_number_ = phone_number;
+  phone_object_ = i18n::PhoneObject(phone_number_, country_name_code_);
+}
+
 bool Address::EqualsIgnoreID(const Address& other) const {
   return country_name_code_ == other.country_name_code_ &&
          recipient_name_ == other.recipient_name_ &&
diff --git a/components/autofill/content/browser/wallet/wallet_address.h b/components/autofill/content/browser/wallet/wallet_address.h
index cead6cc..ae83c6f 100644
--- a/components/autofill/content/browser/wallet/wallet_address.h
+++ b/components/autofill/content/browser/wallet/wallet_address.h
@@ -10,6 +10,7 @@
 #include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/strings/string16.h"
+#include "components/autofill/core/browser/phone_number_i18n.h"
 
 namespace base {
 class DictionaryValue;
@@ -90,6 +91,10 @@ class Address {
   // to the user together with DisplayName().
   base::string16 DisplayNameDetail() const;
 
+  // Returns the phone number as a string that is suitable for display to the
+  // user.
+  base::string16 DisplayPhoneNumber() const;
+
   // Returns data appropriate for |type|.
   base::string16 GetInfo(const AutofillType& type,
                          const std::string& app_locale) const;
@@ -133,9 +138,7 @@ class Address {
   void set_postal_code_number(const base::string16& postal_code_number) {
     postal_code_number_ = postal_code_number;
   }
-  void set_phone_number(const base::string16& phone_number) {
-    phone_number_ = phone_number;
-  }
+  void SetPhoneNumber(const base::string16& phone_number);
   void set_object_id(const std::string& object_id) {
     object_id_ = object_id;
   }
@@ -185,6 +188,9 @@ class Address {
   // this class before being set; see http://code.google.com/p/libphonenumber/.
   base::string16 phone_number_;
 
+  // The parsed phone number.
+  i18n::PhoneObject phone_object_;
+
   // Externalized Online Wallet id for this address.
   std::string object_id_;
 
diff --git a/components/autofill/content/browser/wallet/wallet_address_unittest.cc b/components/autofill/content/browser/wallet/wallet_address_unittest.cc
index 9135f54..42b1ef2 100644
--- a/components/autofill/content/browser/wallet/wallet_address_unittest.cc
+++ b/components/autofill/content/browser/wallet/wallet_address_unittest.cc
@@ -28,7 +28,7 @@ const char kAddressMissingObjectId[] =
     "    \"locality_name\":\"locality_name\","
     "    \"administrative_area_name\":\"administrative_area_name\","
     "    \"postal_code_number\":\"postal_code_number\","
-    "    \"country_name_code\":\"country_name_code\""
+    "    \"country_name_code\":\"US\""
     "  }"
     "}";
 
@@ -64,7 +64,7 @@ const char kAddressMissingRecipientName[] =
     "    \"locality_name\":\"locality_name\","
     "    \"administrative_area_name\":\"administrative_area_name\","
     "    \"postal_code_number\":\"postal_code_number\","
-    "    \"country_name_code\":\"country_name_code\""
+    "    \"country_name_code\":\"US\""
     "  }"
     "}";
 
@@ -82,7 +82,7 @@ const char kAddressMissingPostalCodeNumber[] =
     "    ],"
     "    \"locality_name\":\"locality_name\","
     "    \"administrative_area_name\":\"administrative_area_name\","
-    "    \"country_name_code\":\"country_name_code\""
+    "    \"country_name_code\":\"US\""
     "  }"
     "}";
 
@@ -101,7 +101,7 @@ const char kValidAddress[] =
     "    ],"
     "    \"locality_name\":\"locality_name\","
     "    \"administrative_area_name\":\"administrative_area_name\","
-    "    \"country_name_code\":\"country_name_code\","
+    "    \"country_name_code\":\"US\","
     "    \"postal_code_number\":\"postal_code_number\""
     "  }"
     "}";
@@ -125,7 +125,7 @@ const char kClientAddressMissingPostalCode[] =
   "  \"city\":\"city\","
   "  \"state\":\"state\","
   "  \"phone_number\":\"phone_number\","
-  "  \"country_code\":\"country_code\""
+  "  \"country_code\":\"US\""
   "}";
 
 const char kClientAddressMissingName[] =
@@ -136,7 +136,7 @@ const char kClientAddressMissingName[] =
   "  \"state\":\"state\","
   "  \"postal_code\":\"postal_code\","
   "  \"phone_number\":\"phone_number\","
-  "  \"country_code\":\"country_code\""
+  "  \"country_code\":\"US\""
   "}";
 
 const char kClientValidAddress[] =
@@ -148,7 +148,7 @@ const char kClientValidAddress[] =
   "  \"state\":\"state\","
   "  \"postal_code\":\"postal_code\","
   "  \"phone_number\":\"phone_number\","
-  "  \"country_code\":\"country_code\","
+  "  \"country_code\":\"US\","
   "  \"type\":\"FULL\""
   "}";
 
@@ -171,7 +171,7 @@ class WalletAddressTest : public testing::Test {
 };
 
 TEST_F(WalletAddressTest, AddressEqualsIgnoreID) {
-  Address address1("country_name_code",
+  Address address1("US",
                    ASCIIToUTF16("recipient_name"),
                    ASCIIToUTF16("address_line_1"),
                    ASCIIToUTF16("address_line_2"),
@@ -181,7 +181,7 @@ TEST_F(WalletAddressTest, AddressEqualsIgnoreID) {
                    ASCIIToUTF16("phone_number"),
                    "id1");
   // Same as address1, only id is different.
-  Address address2("country_name_code",
+  Address address2("US",
                    ASCIIToUTF16("recipient_name"),
                    ASCIIToUTF16("address_line_1"),
                    ASCIIToUTF16("address_line_2"),
@@ -191,7 +191,7 @@ TEST_F(WalletAddressTest, AddressEqualsIgnoreID) {
                    ASCIIToUTF16("phone_number"),
                    "id2");
   // Has same id as address1, but name is different.
-  Address address3("country_name_code",
+  Address address3("US",
                    ASCIIToUTF16("a_different_name"),
                    ASCIIToUTF16("address_line_1"),
                    ASCIIToUTF16("address_line_2"),
@@ -201,7 +201,7 @@ TEST_F(WalletAddressTest, AddressEqualsIgnoreID) {
                    ASCIIToUTF16("phone_number"),
                    "id1");
   // Same as address1, but no id.
-  Address address4("country_name_code",
+  Address address4("US",
                    ASCIIToUTF16("recipient_name"),
                    ASCIIToUTF16("address_line_1"),
                    ASCIIToUTF16("address_line_2"),
@@ -237,7 +237,7 @@ TEST_F(WalletAddressTest, AddressEqualsIgnoreID) {
 
 TEST_F(WalletAddressTest, CreateAddressMissingObjectId) {
   SetUpDictionary(kAddressMissingObjectId);
-  Address address("country_name_code",
+  Address address("US",
                   ASCIIToUTF16("recipient_name"),
                   ASCIIToUTF16("address_line_1"),
                   ASCIIToUTF16("address_line_2"),
@@ -274,7 +274,7 @@ TEST_F(WalletAddressTest, CreateAddressMissingPostalCodeNumber) {
 
 TEST_F(WalletAddressTest, CreateAddressWithID) {
   SetUpDictionary(kValidAddress);
-  Address address("country_name_code",
+  Address address("US",
                   ASCIIToUTF16("recipient_name"),
                   ASCIIToUTF16("address_line_1"),
                   ASCIIToUTF16("address_line_2"),
@@ -305,7 +305,7 @@ TEST_F(WalletAddressTest, CreateDisplayAddressMissingPostalCode) {
 
 TEST_F(WalletAddressTest, CreateDisplayAddress) {
   SetUpDictionary(kClientValidAddress);
-  Address address("country_code",
+  Address address("US",
                   ASCIIToUTF16("name"),
                   ASCIIToUTF16("address1"),
                   ASCIIToUTF16("address2"),
@@ -320,7 +320,7 @@ TEST_F(WalletAddressTest, CreateDisplayAddress) {
 TEST_F(WalletAddressTest, ToDictionaryWithoutID) {
   base::DictionaryValue expected;
   expected.SetString("country_name_code",
-                     "country_name_code");
+                     "US");
   expected.SetString("recipient_name",
                      "recipient_name");
   expected.SetString("locality_name",
@@ -334,7 +334,7 @@ TEST_F(WalletAddressTest, ToDictionaryWithoutID) {
   address_lines->AppendString("address_line_2");
   expected.Set("address_line", address_lines);
 
-  Address address("country_name_code",
+  Address address("US",
                   ASCIIToUTF16("recipient_name"),
                   ASCIIToUTF16("address_line_1"),
                   ASCIIToUTF16("address_line_2"),
@@ -352,7 +352,7 @@ TEST_F(WalletAddressTest, ToDictionaryWithID) {
   expected.SetString("id", "id");
   expected.SetString("phone_number", "phone_number");
   expected.SetString("postal_address.country_name_code",
-                     "country_name_code");
+                     "US");
   expected.SetString("postal_address.recipient_name",
                      "recipient_name");
   expected.SetString("postal_address.locality_name",
@@ -366,7 +366,7 @@ TEST_F(WalletAddressTest, ToDictionaryWithID) {
   address_lines->AppendString("address_line_2");
   expected.Set("postal_address.address_line", address_lines);
 
-  Address address("country_name_code",
+  Address address("US",
                   ASCIIToUTF16("recipient_name"),
                   ASCIIToUTF16("address_line_1"),
                   ASCIIToUTF16("address_line_2"),
diff --git a/components/autofill/content/browser/wallet/wallet_client_unittest.cc b/components/autofill/content/browser/wallet/wallet_client_unittest.cc
index 32d598e..e1f5a1d 100644
--- a/components/autofill/content/browser/wallet/wallet_client_unittest.cc
+++ b/components/autofill/content/browser/wallet/wallet_client_unittest.cc
@@ -117,7 +117,7 @@ const char kGetWalletItemsValidResponse[] =
     "        \"state\":\"state\","
     "        \"postal_code\":\"postal_code\","
     "        \"phone_number\":\"phone_number\","
-    "        \"country_code\":\"country_code\""
+    "        \"country_code\":\"US\""
     "      },"
     "      \"status\":\"VALID\","
     "      \"object_id\":\"default_instrument_id\""
diff --git a/components/autofill/content/browser/wallet/wallet_items_unittest.cc b/components/autofill/content/browser/wallet/wallet_items_unittest.cc
index 75da0fd..0766fea 100644
--- a/components/autofill/content/browser/wallet/wallet_items_unittest.cc
+++ b/components/autofill/content/browser/wallet/wallet_items_unittest.cc
@@ -34,7 +34,7 @@ const char kMaskedInstrument[] =
     "    \"state\":\"state\","
     "    \"postal_code\":\"postal_code\","
     "    \"phone_number\":\"phone_number\","
-    "    \"country_code\":\"country_code\","
+    "    \"country_code\":\"US\","
     "    \"type\":\"FULL\""
     "  },"
     "  \"status\":\"VALID\","
@@ -61,7 +61,7 @@ const char kMaskedInstrumentMissingStatus[] =
     "    \"state\":\"state\","
     "    \"postal_code\":\"postal_code\","
     "    \"phone_number\":\"phone_number\","
-    "    \"country_code\":\"country_code\""
+    "    \"country_code\":\"US\""
     "  },"
     "  \"object_id\":\"object_id\","
     "  \"amex_disallowed\":true"
@@ -86,7 +86,7 @@ const char kMaskedInstrumentMissingType[] =
     "    \"state\":\"state\","
     "    \"postal_code\":\"postal_code\","
     "    \"phone_number\":\"phone_number\","
-    "    \"country_code\":\"country_code\""
+    "    \"country_code\":\"US\""
     "  },"
     "  \"status\":\"VALID\","
     "  \"object_id\":\"object_id\""
@@ -111,7 +111,7 @@ const char kMaskedInstrumentMissingLastFourDigits[] =
     "    \"state\":\"state\","
     "    \"postal_code\":\"postal_code\","
     "    \"phone_number\":\"phone_number\","
-    "    \"country_code\":\"country_code\""
+    "    \"country_code\":\"US\""
     "  },"
     "  \"status\":\"VALID\","
     "  \"object_id\":\"object_id\""
@@ -150,7 +150,7 @@ const char kMaskedInstrumentMalformedAddress[] =
     "    \"city\":\"city\","
     "    \"state\":\"state\","
     "    \"phone_number\":\"phone_number\","
-    "    \"country_code\":\"country_code\""
+    "    \"country_code\":\"US\""
     "  },"
     "  \"status\":\"VALID\","
     "  \"object_id\":\"object_id\""
@@ -176,7 +176,7 @@ const char kMaskedInstrumentMissingObjectId[] =
     "    \"state\":\"state\","
     "    \"postal_code\":\"postal_code\","
     "    \"phone_number\":\"phone_number\","
-    "    \"country_code\":\"country_code\""
+    "    \"country_code\":\"US\""
     "  },"
     "  \"status\":\"VALID\""
     "}";
@@ -250,7 +250,7 @@ const char kWalletItemsMissingGoogleTransactionId[] =
     "        \"state\":\"state\","
     "        \"postal_code\":\"postal_code\","
     "        \"phone_number\":\"phone_number\","
-    "        \"country_code\":\"country_code\""
+    "        \"country_code\":\"US\""
     "      },"
     "      \"status\":\"VALID\","
     "      \"object_id\":\"object_id\""
@@ -273,7 +273,7 @@ const char kWalletItemsMissingGoogleTransactionId[] =
     "        \"locality_name\":\"locality_name\","
     "        \"administrative_area_name\":\"administrative_area_name\","
     "        \"postal_code_number\":\"postal_code_number\","
-    "        \"country_name_code\":\"country_name_code\""
+    "        \"country_name_code\":\"US\""
     "      }"
     "    }"
     "  ],"
@@ -316,7 +316,7 @@ const char kWalletItems[] =
     "        \"state\":\"state\","
     "        \"postal_code\":\"postal_code\","
     "        \"phone_number\":\"phone_number\","
-    "        \"country_code\":\"country_code\","
+    "        \"country_code\":\"US\","
     "        \"type\":\"FULL\""
     "      },"
     "      \"status\":\"VALID\","
@@ -340,7 +340,7 @@ const char kWalletItems[] =
     "        \"locality_name\":\"locality_name\","
     "        \"administrative_area_name\":\"administrative_area_name\","
     "        \"postal_code_number\":\"postal_code_number\","
-    "        \"country_name_code\":\"country_name_code\""
+    "        \"country_name_code\":\"US\""
     "      }"
     "    }"
     "  ],"
@@ -416,7 +416,7 @@ TEST_F(WalletItemsTest, CreateMaskedInstrumentMissingObjectId) {
 
 TEST_F(WalletItemsTest, CreateMaskedInstrument) {
   SetUpDictionary(kMaskedInstrument);
-  scoped_ptr<Address> address(new Address("country_code",
+  scoped_ptr<Address> address(new Address("US",
                                           ASCIIToUTF16("name"),
                                           ASCIIToUTF16("address1"),
                                           ASCIIToUTF16("address2"),
@@ -528,7 +528,7 @@ TEST_F(WalletItemsTest, CreateWalletItems) {
                        "obfuscated_gaia_id",
                        AMEX_DISALLOWED);
 
-  scoped_ptr<Address> billing_address(new Address("country_code",
+  scoped_ptr<Address> billing_address(new Address("US",
                                                   ASCIIToUTF16("name"),
                                                   ASCIIToUTF16("address1"),
                                                   ASCIIToUTF16("address2"),
@@ -552,7 +552,7 @@ TEST_F(WalletItemsTest, CreateWalletItems) {
   expected.AddInstrument(masked_instrument.Pass());
 
   scoped_ptr<Address> shipping_address(
-      new Address("country_name_code",
+      new Address("US",
                   ASCIIToUTF16("recipient_name"),
                   ASCIIToUTF16("address_line_1"),
                   ASCIIToUTF16("address_line_2"),
diff --git a/components/autofill/core/browser/phone_number_i18n.cc b/components/autofill/core/browser/phone_number_i18n.cc
index 1cb49ca..b3c9d79 100644
--- a/components/autofill/core/browser/phone_number_i18n.cc
+++ b/components/autofill/core/browser/phone_number_i18n.cc
@@ -253,8 +253,7 @@ PhoneObject::PhoneObject(const PhoneObject& other) { *this = other; }
 
 PhoneObject::PhoneObject() {}
 
-PhoneObject::~PhoneObject() {
-}
+PhoneObject::~PhoneObject() {}
 
 base::string16 PhoneObject::GetFormattedNumber() const {
   if (i18n_number_ && formatted_number_.empty()) {
@@ -265,6 +264,14 @@ base::string16 PhoneObject::GetFormattedNumber() const {
   return formatted_number_;
 }
 
+base::string16 PhoneObject::GetNationallyFormattedNumber() const {
+  base::string16 formatted = whole_number_;
+  if (i18n_number_)
+    FormatValidatedNumber(*i18n_number_, base::string16(), &formatted, NULL);
+
+  return formatted;
+}
+
 base::string16 PhoneObject::GetWholeNumber() const {
   if (i18n_number_ && whole_number_.empty()) {
     FormatValidatedNumber(*i18n_number_, country_code_, &formatted_number_,
diff --git a/components/autofill/core/browser/phone_number_i18n.h b/components/autofill/core/browser/phone_number_i18n.h
index 3fcc824..f370517 100644
--- a/components/autofill/core/browser/phone_number_i18n.h
+++ b/components/autofill/core/browser/phone_number_i18n.h
@@ -80,6 +80,7 @@ class PhoneObject {
   base::string16 number() const { return number_; }
 
   base::string16 GetFormattedNumber() const;
+  base::string16 GetNationallyFormattedNumber() const;
   base::string16 GetWholeNumber() const;
 
   PhoneObject& operator=(const PhoneObject& other);