Adds wallet::Address::CreateAddress() for when IDs are optional (for billing addresses).

R=isherman@chromium.org,ahutter@chromium.org
BUG=163505
TEST=unit, Save*Address()/GetFullWallet() works.


Review URL: https://chromiumcodereview.appspot.com/12388059

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

5 files changed, 113 insertions, 74 deletions

diff --git a/chrome/browser/autofill/wallet/full_wallet.cc b/chrome/browser/autofill/wallet/full_wallet.cc
index 84676d7..9bca726 100644
--- a/chrome/browser/autofill/wallet/full_wallet.cc
+++ b/chrome/browser/autofill/wallet/full_wallet.cc
@@ -99,7 +99,7 @@ scoped_ptr<FullWallet>
   }
 
   scoped_ptr<Address> billing_address =
-      Address::CreateAddressWithID(*billing_address_dict);
+      Address::CreateAddress(*billing_address_dict);
   if (!billing_address.get()) {
     DLOG(ERROR) << "Response from Google wallet has malformed billing address";
     return scoped_ptr<FullWallet>();
diff --git a/chrome/browser/autofill/wallet/full_wallet_unittest.cc b/chrome/browser/autofill/wallet/full_wallet_unittest.cc
index b2678b9f6..4a3133e 100644
--- a/chrome/browser/autofill/wallet/full_wallet_unittest.cc
+++ b/chrome/browser/autofill/wallet/full_wallet_unittest.cc
@@ -21,7 +21,6 @@ const char kFullWalletValidResponse[] =
     "  \"rest\":\"rest\","
     "  \"billing_address\":"
     "  {"
-    "    \"id\":\"id\","
     "    \"phone_number\":\"phone_number\","
     "    \"postal_address\":"
     "    {"
@@ -304,7 +303,6 @@ const char kFullWalletMalformedBillingAddress[] =
     "  \"rest\":\"rest\","
     "  \"billing_address\":"
     "  {"
-    "    \"id\":\"id\","
     "    \"phone_number\":\"phone_number\","
     "    \"postal_address\":"
     "    {"
@@ -427,6 +425,7 @@ TEST_F(FullWalletTest, CreateFullWalletWithInvalidRequiredActions) {
 
 TEST_F(FullWalletTest, CreateFullWallet) {
   SetUpDictionary(kFullWalletValidResponse);
+  // NOTE: FullWallet billing address doesn't require an ID.
   scoped_ptr<Address> billing_address(new Address(
       "country_name_code",
       ASCIIToUTF16("recipient_name"),
@@ -436,7 +435,7 @@ TEST_F(FullWalletTest, CreateFullWallet) {
       ASCIIToUTF16("administrative_area_name"),
       ASCIIToUTF16("postal_code_number"),
       ASCIIToUTF16("phone_number"),
-      "id"));
+      ""));
   scoped_ptr<Address> shipping_address(new Address(
       "ship_country_name_code",
       ASCIIToUTF16("ship_recipient_name"),
diff --git a/chrome/browser/autofill/wallet/wallet_address.cc b/chrome/browser/autofill/wallet/wallet_address.cc
index 7222281..1b497cc 100644
--- a/chrome/browser/autofill/wallet/wallet_address.cc
+++ b/chrome/browser/autofill/wallet/wallet_address.cc
@@ -12,6 +12,72 @@
 namespace autofill {
 namespace wallet {
 
+namespace {
+
+Address* CreateAddressInternal(const base::DictionaryValue& dictionary,
+                               const std::string& object_id) {
+  std::string country_name_code;
+  if (!dictionary.GetString("postal_address.country_name_code",
+                            &country_name_code)) {
+    DLOG(ERROR) << "Response from Google Wallet missing country name";
+    return NULL;
+  }
+
+  string16 recipient_name;
+  if (!dictionary.GetString("postal_address.recipient_name",
+                            &recipient_name)) {
+    DLOG(ERROR) << "Response from Google Wallet recipient name";
+    return NULL;
+  }
+
+  string16 postal_code_number;
+  if (!dictionary.GetString("postal_address.postal_code_number",
+                            &postal_code_number)) {
+    DLOG(ERROR) << "Response from Google Wallet missing postal code number";
+    return NULL;
+  }
+
+  string16 phone_number;
+  if (!dictionary.GetString("phone_number", &phone_number))
+    DVLOG(1) << "Response from Google Wallet missing phone number";
+
+  string16 address_line_1;
+  string16 address_line_2;
+  const ListValue* address_line_list;
+  if (dictionary.GetList("postal_address.address_line", &address_line_list)) {
+    if (!address_line_list->GetString(0, &address_line_1))
+      DVLOG(1) << "Response from Google Wallet missing address line 1";
+    if (!address_line_list->GetString(1, &address_line_2))
+      DVLOG(1) << "Response from Google Wallet missing address line 2";
+  } else {
+    DVLOG(1) << "Response from Google Wallet missing address lines";
+  }
+
+  string16 locality_name;
+  if (!dictionary.GetString("postal_address.locality_name",
+                            &locality_name)) {
+    DVLOG(1) << "Response from Google Wallet missing locality name";
+  }
+
+  string16 administrative_area_name;
+  if (!dictionary.GetString("postal_address.administrative_area_name",
+                            &administrative_area_name)) {
+    DVLOG(1) << "Response from Google Wallet missing administrative area name";
+  }
+
+  return new Address(country_name_code,
+                     recipient_name ,
+                     address_line_1,
+                     address_line_2,
+                     locality_name,
+                     administrative_area_name,
+                     postal_code_number,
+                     phone_number,
+                     object_id);
+}
+
+}  // namespace
+
 Address::Address() {}
 
 Address::Address(const std::string& country_name_code,
@@ -106,76 +172,25 @@ string16 Address::GetInfo(AutofillFieldType type) const {
   }
 }
 
-scoped_ptr<Address>
-    Address::CreateAddressWithID(const base::DictionaryValue& dictionary) {
+scoped_ptr<Address> Address::CreateAddressWithID(
+    const base::DictionaryValue& dictionary) {
   std::string object_id;
   if (!dictionary.GetString("id", &object_id)) {
     DLOG(ERROR) << "Response from Google Wallet missing object id";
     return scoped_ptr<Address>();
   }
+  return scoped_ptr<Address>(CreateAddressInternal(dictionary, object_id));
+}
 
-  std::string country_name_code;
-  if (!dictionary.GetString("postal_address.country_name_code",
-                            &country_name_code)) {
-    DLOG(ERROR) << "Response from Google Wallet missing country name";
-    return scoped_ptr<Address>();
-  }
-
-  string16 recipient_name;
-  if (!dictionary.GetString("postal_address.recipient_name",
-                            &recipient_name)) {
-    DLOG(ERROR) << "Response from Google Wallet recipient name";
-    return scoped_ptr<Address>();
-  }
-
-  string16 postal_code_number;
-  if (!dictionary.GetString("postal_address.postal_code_number",
-                            &postal_code_number)) {
-    DLOG(ERROR) << "Response from Google Wallet missing postal code number";
-    return scoped_ptr<Address>();
-  }
-
-  string16 phone_number;
-  if (!dictionary.GetString("phone_number", &phone_number))
-    DVLOG(1) << "Response from Google Wallet missing phone number";
-
-  string16 address_line_1;
-  string16 address_line_2;
-  const ListValue* address_line_list;
-  if (dictionary.GetList("postal_address.address_line", &address_line_list)) {
-    if (!address_line_list->GetString(0, &address_line_1))
-      DVLOG(1) << "Response from Google Wallet missing address line 1";
-    if (!address_line_list->GetString(1, &address_line_2))
-      DVLOG(1) << "Response from Google Wallet missing address line 2";
-  } else {
-    DVLOG(1) << "Response from Google Wallet missing address lines";
-  }
-
-  string16 locality_name;
-  if (!dictionary.GetString("postal_address.locality_name",
-                            &locality_name)) {
-    DVLOG(1) << "Response from Google Wallet missing locality name";
-  }
-
-  string16 administrative_area_name;
-  if (!dictionary.GetString("postal_address.administrative_area_name",
-                            &administrative_area_name)) {
-    DVLOG(1) << "Response from Google Wallet missing administrative area name";
-  }
-
-  return scoped_ptr<Address>(new Address(country_name_code,
-                                         recipient_name ,
-                                         address_line_1,
-                                         address_line_2,
-                                         locality_name,
-                                         administrative_area_name,
-                                         postal_code_number,
-                                         phone_number,
-                                         object_id));
+scoped_ptr<Address> Address::CreateAddress(
+    const base::DictionaryValue& dictionary) {
+  std::string object_id;
+  dictionary.GetString("id", &object_id);
+  return scoped_ptr<Address>(CreateAddressInternal(dictionary, object_id));
 }
 
-scoped_ptr<Address>
-    Address::CreateDisplayAddress(const base::DictionaryValue& dictionary) {
+scoped_ptr<Address> Address::CreateDisplayAddress(
+    const base::DictionaryValue& dictionary) {
   std::string country_code;
   if (!dictionary.GetString("country_code", &country_code)) {
     DLOG(ERROR) << "Reponse from Google Wallet missing country code";
diff --git a/chrome/browser/autofill/wallet/wallet_address.h b/chrome/browser/autofill/wallet/wallet_address.h
index 9804709..fcf6d92 100644
--- a/chrome/browser/autofill/wallet/wallet_address.h
+++ b/chrome/browser/autofill/wallet/wallet_address.h
@@ -100,14 +100,21 @@ class Address {
   string16 GetInfo(AutofillFieldType type) const;
 
   // Returns an empty scoped_ptr if input is invalid or a valid address that is
-  // selectable for Google Wallet use.
-  static scoped_ptr<Address>
-      CreateAddressWithID(const base::DictionaryValue& dictionary);
+  // selectable for Google Wallet use. Does not require "id" in |dictionary|.
+  // IDs are not required for billing addresses.
+  static scoped_ptr<Address> CreateAddress(
+      const base::DictionaryValue& dictionary);
+
+  // Returns an empty scoped_ptr if input is invalid or a valid address that is
+  // selectable for Google Wallet use. Requires "id" in |dictionary|. IDs are
+  // required for shipping addresses.
+  static scoped_ptr<Address> CreateAddressWithID(
+      const base::DictionaryValue& dictionary);
 
   // Returns an empty scoped_ptr if input in invalid or a valid address that
   // can only be used for displaying to the user.
-  static scoped_ptr<Address>
-      CreateDisplayAddress(const base::DictionaryValue& dictionary);
+  static scoped_ptr<Address> CreateDisplayAddress(
+      const base::DictionaryValue& dictionary);
 
   bool operator==(const Address& other) const;
   bool operator!=(const Address& other) const;
diff --git a/chrome/browser/autofill/wallet/wallet_address_unittest.cc b/chrome/browser/autofill/wallet/wallet_address_unittest.cc
index 747e034..57ce0ab 100644
--- a/chrome/browser/autofill/wallet/wallet_address_unittest.cc
+++ b/chrome/browser/autofill/wallet/wallet_address_unittest.cc
@@ -166,23 +166,40 @@ class WalletAddressTest : public testing::Test {
   scoped_ptr<const DictionaryValue> dict_;
 };
 
+TEST_F(WalletAddressTest, CreateAddressMissingObjectId) {
+  SetUpDictionary(kAddressMissingObjectId);
+  Address address("country_name_code",
+                  ASCIIToUTF16("recipient_name"),
+                  ASCIIToUTF16("address_line_1"),
+                  ASCIIToUTF16("address_line_2"),
+                  ASCIIToUTF16("locality_name"),
+                  ASCIIToUTF16("administrative_area_name"),
+                  ASCIIToUTF16("postal_code_number"),
+                  ASCIIToUTF16("phone_number"),
+                  "");
+  ASSERT_EQ(address, *Address::CreateAddress(*dict_));
+}
+
 TEST_F(WalletAddressTest, CreateAddressWithIDMissingObjectId) {
   SetUpDictionary(kAddressMissingObjectId);
   ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
-TEST_F(WalletAddressTest, CreateAddressWithIDMissingCountryNameCode) {
+TEST_F(WalletAddressTest, CreateAddressMissingCountryNameCode) {
   SetUpDictionary(kAddressMissingCountryNameCode);
+  ASSERT_EQ(NULL, Address::CreateAddress(*dict_).get());
   ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
-TEST_F(WalletAddressTest, CreateAddressWithIDMissingRecipientName) {
+TEST_F(WalletAddressTest, CreateAddressMissingRecipientName) {
   SetUpDictionary(kAddressMissingRecipientName);
+  ASSERT_EQ(NULL, Address::CreateAddress(*dict_).get());
   ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
-TEST_F(WalletAddressTest, CreateAddressWithIDMissingPostalCodeNumber) {
+TEST_F(WalletAddressTest, CreateAddressMissingPostalCodeNumber) {
   SetUpDictionary(kAddressMissingPostalCodeNumber);
+  ASSERT_EQ(NULL, Address::CreateAddress(*dict_).get());
   ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
@@ -197,6 +214,7 @@ TEST_F(WalletAddressTest, CreateAddressWithID) {
                   ASCIIToUTF16("postal_code_number"),
                   ASCIIToUTF16("phone_number"),
                   "id");
+  ASSERT_EQ(address, *Address::CreateAddress(*dict_));
   ASSERT_EQ(address, *Address::CreateAddressWithID(*dict_));
 }