Autofill dialog: don't save duplicate shipping addresses to Wallet.

When autofill_dialog in wallet mode, and user select "same-as-billing", avoid to save a duplicated shipping address when user already has an exact matched one in his/her wallet. This applies to both rAC and Autocheckout.

BUG=225442

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

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

4 files changed, 95 insertions, 23 deletions

diff --git a/components/autofill/browser/wallet/wallet_address.cc b/components/autofill/browser/wallet/wallet_address.cc
index 6035144..92aeffb 100644
--- a/components/autofill/browser/wallet/wallet_address.cc
+++ b/components/autofill/browser/wallet/wallet_address.cc
@@ -291,7 +291,7 @@ string16 Address::GetInfo(AutofillFieldType type,
   }
 }
 
-bool Address::operator==(const Address& other) const {
+bool Address::EqualsIgnoreID(const Address& other) const {
   return country_name_code_ == other.country_name_code_ &&
          recipient_name_ == other.recipient_name_ &&
          address_line_1_ == other.address_line_1_ &&
@@ -300,10 +300,13 @@ bool Address::operator==(const Address& other) const {
          administrative_area_name_ == other.administrative_area_name_ &&
          postal_code_number_ == other.postal_code_number_ &&
          phone_number_ == other.phone_number_ &&
-         object_id_ == other.object_id_ &&
          is_complete_address_ == other.is_complete_address_;
 }
 
+bool Address::operator==(const Address& other) const {
+  return object_id_ == other.object_id_ && EqualsIgnoreID(other);
+}
+
 bool Address::operator!=(const Address& other) const {
   return !(*this == other);
 }
diff --git a/components/autofill/browser/wallet/wallet_address.h b/components/autofill/browser/wallet/wallet_address.h
index 953af5a..a13f2f2 100644
--- a/components/autofill/browser/wallet/wallet_address.h
+++ b/components/autofill/browser/wallet/wallet_address.h
@@ -143,6 +143,10 @@ class Address {
     is_complete_address_ = is_complete_address;
   }
 
+  // Tests if this address exact matches |other|. |object_id| is ignored.
+  bool EqualsIgnoreID(const Address& other) const;
+
+  // Tests if this address exact matches |other| including |object_id|.
   bool operator==(const Address& other) const;
   bool operator!=(const Address& other) const;
 
diff --git a/components/autofill/browser/wallet/wallet_address_unittest.cc b/components/autofill/browser/wallet/wallet_address_unittest.cc
index 476d3ab..27a501f 100644
--- a/components/autofill/browser/wallet/wallet_address_unittest.cc
+++ b/components/autofill/browser/wallet/wallet_address_unittest.cc
@@ -168,6 +168,71 @@ class WalletAddressTest : public testing::Test {
   scoped_ptr<const DictionaryValue> dict_;
 };
 
+TEST_F(WalletAddressTest, AddressEqualsIgnoreID) {
+  Address address1("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"),
+                   "id1");
+  // Same as address1, only id is different.
+  Address address2("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"),
+                   "id2");
+  // Has same id as address1, but name is different.
+  Address address3("country_name_code",
+                   ASCIIToUTF16("a_different_name"),
+                   ASCIIToUTF16("address_line_1"),
+                   ASCIIToUTF16("address_line_2"),
+                   ASCIIToUTF16("locality_name"),
+                   ASCIIToUTF16("administrative_area_name"),
+                   ASCIIToUTF16("postal_code_number"),
+                   ASCIIToUTF16("phone_number"),
+                   "id1");
+  // Same as address1, but no id.
+  Address address4("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"),
+                   std::string());
+
+  // Compare the address has id field to itself.
+  EXPECT_EQ(address1, address1);
+  EXPECT_TRUE(address1.EqualsIgnoreID(address1));
+
+  // Compare the address has no id field to itself
+  EXPECT_EQ(address4, address4);
+  EXPECT_TRUE(address4.EqualsIgnoreID(address4));
+
+  // Compare two addresses with different id.
+  EXPECT_NE(address1, address2);
+  EXPECT_TRUE(address1.EqualsIgnoreID(address2));
+  EXPECT_TRUE(address2.EqualsIgnoreID(address1));
+
+  // Compare two different addresses.
+  EXPECT_NE(address1, address3);
+  EXPECT_FALSE(address1.EqualsIgnoreID(address3));
+  EXPECT_FALSE(address3.EqualsIgnoreID(address1));
+
+  // Compare two same addresses, one has id, the other doesn't.
+  EXPECT_NE(address1, address4);
+  EXPECT_TRUE(address1.EqualsIgnoreID(address4));
+  EXPECT_TRUE(address4.EqualsIgnoreID(address1));
+}
+
 TEST_F(WalletAddressTest, CreateAddressMissingObjectId) {
   SetUpDictionary(kAddressMissingObjectId);
   Address address("country_name_code",
@@ -179,30 +244,30 @@ TEST_F(WalletAddressTest, CreateAddressMissingObjectId) {
                   ASCIIToUTF16("postal_code_number"),
                   ASCIIToUTF16("phone_number"),
                   std::string());
-  ASSERT_EQ(address, *Address::CreateAddress(*dict_));
+  EXPECT_EQ(address, *Address::CreateAddress(*dict_));
 }
 
 TEST_F(WalletAddressTest, CreateAddressWithIDMissingObjectId) {
   SetUpDictionary(kAddressMissingObjectId);
-  ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
 TEST_F(WalletAddressTest, CreateAddressMissingCountryNameCode) {
   SetUpDictionary(kAddressMissingCountryNameCode);
-  ASSERT_EQ(NULL, Address::CreateAddress(*dict_).get());
-  ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateAddress(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
 TEST_F(WalletAddressTest, CreateAddressMissingRecipientName) {
   SetUpDictionary(kAddressMissingRecipientName);
-  ASSERT_EQ(NULL, Address::CreateAddress(*dict_).get());
-  ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateAddress(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
 TEST_F(WalletAddressTest, CreateAddressMissingPostalCodeNumber) {
   SetUpDictionary(kAddressMissingPostalCodeNumber);
-  ASSERT_EQ(NULL, Address::CreateAddress(*dict_).get());
-  ASSERT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateAddress(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateAddressWithID(*dict_).get());
 }
 
 TEST_F(WalletAddressTest, CreateAddressWithID) {
@@ -217,23 +282,23 @@ TEST_F(WalletAddressTest, CreateAddressWithID) {
                   ASCIIToUTF16("phone_number"),
                   "id");
   address.set_is_complete_address(false);
-  ASSERT_EQ(address, *Address::CreateAddress(*dict_));
-  ASSERT_EQ(address, *Address::CreateAddressWithID(*dict_));
+  EXPECT_EQ(address, *Address::CreateAddress(*dict_));
+  EXPECT_EQ(address, *Address::CreateAddressWithID(*dict_));
 }
 
 TEST_F(WalletAddressTest, CreateDisplayAddressMissingCountryNameCode) {
   SetUpDictionary(kClientAddressMissingCountryCode);
-  ASSERT_EQ(NULL, Address::CreateDisplayAddress(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateDisplayAddress(*dict_).get());
 }
 
 TEST_F(WalletAddressTest, CreateDisplayAddressMissingName) {
   SetUpDictionary(kClientAddressMissingName);
-  ASSERT_EQ(NULL, Address::CreateDisplayAddress(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateDisplayAddress(*dict_).get());
 }
 
 TEST_F(WalletAddressTest, CreateDisplayAddressMissingPostalCode) {
   SetUpDictionary(kClientAddressMissingPostalCode);
-  ASSERT_EQ(NULL, Address::CreateDisplayAddress(*dict_).get());
+  EXPECT_EQ(NULL, Address::CreateDisplayAddress(*dict_).get());
 }
 
 TEST_F(WalletAddressTest, CreateDisplayAddress) {
@@ -247,7 +312,7 @@ TEST_F(WalletAddressTest, CreateDisplayAddress) {
                   ASCIIToUTF16("postal_code"),
                   ASCIIToUTF16("phone_number"),
                   std::string());
-  ASSERT_EQ(address, *Address::CreateDisplayAddress(*dict_));
+  EXPECT_EQ(address, *Address::CreateDisplayAddress(*dict_));
 }
 
 TEST_F(WalletAddressTest, ToDictionaryWithoutID) {
diff --git a/components/autofill/browser/wallet/wallet_items.cc b/components/autofill/browser/wallet/wallet_items.cc
index ff81bcc..062496f 100644
--- a/components/autofill/browser/wallet/wallet_items.cc
+++ b/components/autofill/browser/wallet/wallet_items.cc
@@ -222,13 +222,6 @@ bool WalletItems::MaskedInstrument::operator!=(
   return !(*this == other);
 }
 
-bool WalletItems::HasRequiredAction(RequiredAction action) const {
-  DCHECK(ActionAppliesToWalletItems(action));
-  return std::find(required_actions_.begin(),
-                   required_actions_.end(),
-                   action) != required_actions_.end();
-}
-
 const WalletItems::MaskedInstrument* WalletItems::GetInstrumentById(
     const std::string& object_id) const {
   if (object_id.empty())
@@ -242,6 +235,13 @@ const WalletItems::MaskedInstrument* WalletItems::GetInstrumentById(
   return NULL;
 }
 
+bool WalletItems::HasRequiredAction(RequiredAction action) const {
+  DCHECK(ActionAppliesToWalletItems(action));
+  return std::find(required_actions_.begin(),
+                   required_actions_.end(),
+                   action) != required_actions_.end();
+}
+
 base::string16 WalletItems::MaskedInstrument::DisplayName() const {
 #if defined(OS_ANDROID)
   // TODO(aruslan): improve this stub implementation.