diff options
Diffstat (limited to 'components')
7 files changed, 261 insertions, 25 deletions
diff --git a/components/autofill/browser/autofill_profile.cc b/components/autofill/browser/autofill_profile.cc index 0748622..82e0e68 100644 --- a/components/autofill/browser/autofill_profile.cc +++ b/components/autofill/browser/autofill_profile.cc @@ -505,6 +505,10 @@ bool AutofillProfile::IsSubsetOf(const AutofillProfile& profile, void AutofillProfile::OverwriteWithOrAddTo(const AutofillProfile& profile, const std::string& app_locale) { + // Verified profiles should never be overwritten with unverified data. + DCHECK(!IsVerified() || profile.IsVerified()); + set_origin(profile.origin()); + FieldTypeSet field_types; profile.GetNonEmptyTypes(app_locale, &field_types); diff --git a/components/autofill/browser/autofill_profile_unittest.cc b/components/autofill/browser/autofill_profile_unittest.cc index c42e4c4..8058a11 100644 --- a/components/autofill/browser/autofill_profile_unittest.cc +++ b/components/autofill/browser/autofill_profile_unittest.cc @@ -545,6 +545,42 @@ TEST(AutofillProfileTest, IsSubsetOf) { EXPECT_FALSE(a->IsSubsetOf(*b, "en-US")); } +TEST(AutofillProfileTest, OverwriteWithOrAddTo) { + AutofillProfile a(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&a, "Marion", "Mitchell", "Morrison", + "marion@me.xyz", "Fox", "123 Zoo St.", "unit 5", + "Hollywood", "CA", "91601", "US", + "12345678910"); + std::vector<base::string16> names; + a.GetRawMultiInfo(NAME_FULL, &names); + names.push_back(ASCIIToUTF16("Marion Morrison")); + a.SetRawMultiInfo(NAME_FULL, names); + + // Create an identical profile except that the new profile: + // (1) Has a different origin, + // (2) Has a different address line 2, + // (3) Lacks a company name, and + // (4) Has a different full name variant. + AutofillProfile b = a; + b.set_guid(base::GenerateGUID()); + b.set_origin("Chrome settings"); + b.SetRawInfo(ADDRESS_HOME_LINE2, ASCIIToUTF16("area 51")); + b.SetRawInfo(COMPANY_NAME, base::string16()); + b.GetRawMultiInfo(NAME_FULL, &names); + names.push_back(ASCIIToUTF16("Marion M. Morrison")); + b.SetRawMultiInfo(NAME_FULL, names); + + a.OverwriteWithOrAddTo(b, "en-US"); + EXPECT_EQ("Chrome settings", a.origin()); + EXPECT_EQ(ASCIIToUTF16("area 51"), a.GetRawInfo(ADDRESS_HOME_LINE2)); + EXPECT_EQ(ASCIIToUTF16("Fox"), a.GetRawInfo(COMPANY_NAME)); + a.GetRawMultiInfo(NAME_FULL, &names); + ASSERT_EQ(3U, names.size()); + EXPECT_EQ(ASCIIToUTF16("Marion Mitchell Morrison"), names[0]); + EXPECT_EQ(ASCIIToUTF16("Marion Morrison"), names[1]); + EXPECT_EQ(ASCIIToUTF16("Marion M. Morrison"), names[2]); +} + TEST(AutofillProfileTest, AssignmentOperator) { AutofillProfile a(base::GenerateGUID(), "https://www.example.com/"); test::SetProfileInfo(&a, "Marion", "Mitchell", "Morrison", diff --git a/components/autofill/browser/credit_card.cc b/components/autofill/browser/credit_card.cc index 5308daa1..8a96fa9 100644 --- a/components/autofill/browser/credit_card.cc +++ b/components/autofill/browser/credit_card.cc @@ -498,10 +498,15 @@ bool CreditCard::UpdateFromImportedCard(const CreditCard& imported_card, return false; } - DCHECK(!imported_card.IsVerified()); - if (this->IsVerified()) + // Heuristically aggregated data should never overwrite verified data. + // Instead, discard any heuristically aggregated credit cards that disagree + // with explicitly entered data, so that the UI is not cluttered with + // duplicate cards. + if (this->IsVerified() && !imported_card.IsVerified()) return true; + set_origin(imported_card.origin()); + // Note that the card number is intentionally not updated, so as to preserve // any formatting (i.e. separator characters). Since the card number is not // updated, there is no reason to update the card type, either. diff --git a/components/autofill/browser/credit_card_unittest.cc b/components/autofill/browser/credit_card_unittest.cc index 6d78f33..b67ff3c 100644 --- a/components/autofill/browser/credit_card_unittest.cc +++ b/components/autofill/browser/credit_card_unittest.cc @@ -155,6 +155,79 @@ TEST(CreditCardTest, Compare) { EXPECT_LT(0, b.Compare(a)); } +TEST(CreditCardTest, UpdateFromImportedCard) { + CreditCard original_card(base::GenerateGUID(), "https://www.example.com"); + test::SetCreditCardInfo( + &original_card, "John Dillinger", "123456789012", "09", "2017"); + + CreditCard a = original_card; + + // The new card has a different name, expiration date, and origin. + CreditCard b = a; + b.set_guid(base::GenerateGUID()); + b.set_origin("https://www.example.org"); + b.SetRawInfo(CREDIT_CARD_NAME, ASCIIToUTF16("J. Dillinger")); + b.SetRawInfo(CREDIT_CARD_EXP_MONTH, ASCIIToUTF16("08")); + b.SetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR, ASCIIToUTF16("2019")); + + EXPECT_TRUE(a.UpdateFromImportedCard(b, "en-US")); + EXPECT_EQ("https://www.example.org", a.origin()); + EXPECT_EQ(ASCIIToUTF16("J. Dillinger"), a.GetRawInfo(CREDIT_CARD_NAME)); + EXPECT_EQ(ASCIIToUTF16("08"), a.GetRawInfo(CREDIT_CARD_EXP_MONTH)); + EXPECT_EQ(ASCIIToUTF16("2019"), a.GetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR)); + + // Try again, but with no name set for |b|. + a = original_card; + b.SetRawInfo(CREDIT_CARD_NAME, base::string16()); + + EXPECT_TRUE(a.UpdateFromImportedCard(b, "en-US")); + EXPECT_EQ("https://www.example.org", a.origin()); + EXPECT_EQ(ASCIIToUTF16("John Dillinger"), a.GetRawInfo(CREDIT_CARD_NAME)); + EXPECT_EQ(ASCIIToUTF16("08"), a.GetRawInfo(CREDIT_CARD_EXP_MONTH)); + EXPECT_EQ(ASCIIToUTF16("2019"), a.GetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR)); + + // Try again, but with only the original card having a verified origin. + // |a| should be unchanged. + a = original_card; + a.set_origin("Chrome settings"); + b.SetRawInfo(CREDIT_CARD_NAME, ASCIIToUTF16("J. Dillinger")); + + EXPECT_TRUE(a.UpdateFromImportedCard(b, "en-US")); + EXPECT_EQ("Chrome settings", a.origin()); + EXPECT_EQ(ASCIIToUTF16("John Dillinger"), a.GetRawInfo(CREDIT_CARD_NAME)); + EXPECT_EQ(ASCIIToUTF16("09"), a.GetRawInfo(CREDIT_CARD_EXP_MONTH)); + EXPECT_EQ(ASCIIToUTF16("2017"), a.GetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR)); + + // Try again, but with only the new card having a verified origin. + a = original_card; + b.set_origin("Chrome settings"); + + EXPECT_TRUE(a.UpdateFromImportedCard(b, "en-US")); + EXPECT_EQ("Chrome settings", a.origin()); + EXPECT_EQ(ASCIIToUTF16("J. Dillinger"), a.GetRawInfo(CREDIT_CARD_NAME)); + EXPECT_EQ(ASCIIToUTF16("08"), a.GetRawInfo(CREDIT_CARD_EXP_MONTH)); + EXPECT_EQ(ASCIIToUTF16("2019"), a.GetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR)); + + // Try again, with both cards having a verified origin. + a = original_card; + a.set_origin("Chrome Autofill dialog"); + b.set_origin("Chrome settings"); + + EXPECT_TRUE(a.UpdateFromImportedCard(b, "en-US")); + EXPECT_EQ("Chrome settings", a.origin()); + EXPECT_EQ(ASCIIToUTF16("J. Dillinger"), a.GetRawInfo(CREDIT_CARD_NAME)); + EXPECT_EQ(ASCIIToUTF16("08"), a.GetRawInfo(CREDIT_CARD_EXP_MONTH)); + EXPECT_EQ(ASCIIToUTF16("2019"), a.GetRawInfo(CREDIT_CARD_EXP_4_DIGIT_YEAR)); + + // Try again, but with |b| having a different card number. + // |a| should be unchanged. + a = original_card; + b.SetRawInfo(CREDIT_CARD_NUMBER, ASCIIToUTF16("4111111111111111")); + + EXPECT_FALSE(a.UpdateFromImportedCard(b, "en-US")); + EXPECT_EQ(original_card, a); +} + TEST(CreditCardTest, IsComplete) { CreditCard card(base::GenerateGUID(), "https://www.example.com/"); EXPECT_FALSE(card.IsComplete()); diff --git a/components/autofill/browser/personal_data_manager.cc b/components/autofill/browser/personal_data_manager.cc index 6a7fb1f..ecd5538 100644 --- a/components/autofill/browser/personal_data_manager.cc +++ b/components/autofill/browser/personal_data_manager.cc @@ -725,41 +725,41 @@ bool PersonalDataManager::IsValidLearnableProfile( // static bool PersonalDataManager::MergeProfile( - const AutofillProfile& profile, + const AutofillProfile& new_profile, const std::vector<AutofillProfile*>& existing_profiles, const std::string& app_locale, std::vector<AutofillProfile>* merged_profiles) { merged_profiles->clear(); - // Set to true if |profile| is merged into |existing_profiles|. - bool merged = false; + // Set to true if |existing_profiles| already contains an equivalent profile. + bool matching_profile_found = false; // If we have already saved this address, merge in any missing values. // Only merge with the first match. for (std::vector<AutofillProfile*>::const_iterator iter = existing_profiles.begin(); iter != existing_profiles.end(); ++iter) { - if (!merged) { - if (!profile.PrimaryValue().empty() && - StringToLowerASCII((*iter)->PrimaryValue()) == - StringToLowerASCII(profile.PrimaryValue())) { - merged = true; - - // Automatically aggregated profiles should never overwrite explicitly - // user-entered ones. If one would, just drop it. - DCHECK(!profile.IsVerified()); - if (!(*iter)->IsVerified()) - (*iter)->OverwriteWithOrAddTo(profile, app_locale); - } + AutofillProfile* existing_profile = *iter; + if (!matching_profile_found && + !new_profile.PrimaryValue().empty() && + StringToLowerASCII(existing_profile->PrimaryValue()) == + StringToLowerASCII(new_profile.PrimaryValue())) { + // Unverified profiles should always be updated with the newer data, + // whereas verified profiles should only ever be overwritten by verified + // data. If an automatically aggregated profile would overwrite a + // verified profile, just drop it. + matching_profile_found = true; + if (!existing_profile->IsVerified() || new_profile.IsVerified()) + existing_profile->OverwriteWithOrAddTo(new_profile, app_locale); } - merged_profiles->push_back(**iter); + merged_profiles->push_back(*existing_profile); } // If the new profile was not merged with an existing one, add it to the list. - if (!merged) - merged_profiles->push_back(profile); + if (!matching_profile_found) + merged_profiles->push_back(new_profile); - return merged; + return matching_profile_found; } void PersonalDataManager::SetProfiles(std::vector<AutofillProfile>* profiles) { diff --git a/components/autofill/browser/personal_data_manager.h b/components/autofill/browser/personal_data_manager.h index 8692dfb..6a8b8c9 100644 --- a/components/autofill/browser/personal_data_manager.h +++ b/components/autofill/browser/personal_data_manager.h @@ -166,11 +166,11 @@ class PersonalDataManager : public WebDataServiceConsumer, static bool IsValidLearnableProfile(const AutofillProfile& profile, const std::string& app_locale); - // Merges |profile| into one of the |existing_profiles| if possible; otherwise - // appends |profile| to the end of that list. Fills |merged_profiles| with the - // result. + // Merges |new_profile| into one of the |existing_profiles| if possible; + // otherwise appends |new_profile| to the end of that list. Fills + // |merged_profiles| with the result. static bool MergeProfile( - const AutofillProfile& profile, + const AutofillProfile& new_profile, const std::vector<AutofillProfile*>& existing_profiles, const std::string& app_locale, std::vector<AutofillProfile>* merged_profiles); diff --git a/components/autofill/browser/personal_data_manager_unittest.cc b/components/autofill/browser/personal_data_manager_unittest.cc index 6c20883..9452d3c 100644 --- a/components/autofill/browser/personal_data_manager_unittest.cc +++ b/components/autofill/browser/personal_data_manager_unittest.cc @@ -2020,6 +2020,124 @@ TEST_F(PersonalDataManagerTest, EXPECT_EQ(0, credit_card.Compare(*results[0])); } +// Ensure that verified profiles can be saved via SaveImportedProfile, +// overwriting existing unverified profiles. +TEST_F(PersonalDataManagerTest, SaveImportedProfileWithVerifiedData) { + // Start with an unverified profile. + AutofillProfile profile(base::GenerateGUID(), "https://www.example.com"); + test::SetProfileInfo(&profile, + "Marion", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA", + "91601", "US", "12345678910"); + EXPECT_FALSE(profile.IsVerified()); + + // Add the profile to the database. + personal_data_->AddProfile(profile); + + // Verify that the web database has been updated and the notification sent. + EXPECT_CALL(personal_data_observer_, + OnPersonalDataChanged()).WillOnce(QuitUIMessageLoop()); + base::MessageLoop::current()->Run(); + + AutofillProfile new_verified_profile = profile; + new_verified_profile.set_guid(base::GenerateGUID()); + new_verified_profile.set_origin("Chrome settings"); + new_verified_profile.SetRawInfo(COMPANY_NAME, ASCIIToUTF16("Fizzbang, Inc.")); + EXPECT_TRUE(new_verified_profile.IsVerified()); + + personal_data_->SaveImportedProfile(new_verified_profile); + + // Verify that the web database has been updated and the notification sent. + EXPECT_CALL(personal_data_observer_, + OnPersonalDataChanged()).WillOnce(QuitUIMessageLoop()); + base::MessageLoop::current()->Run(); + + // Expect that the existing profile is not modified, and instead the new + // profile is added. + const std::vector<AutofillProfile*>& results = personal_data_->GetProfiles(); + ASSERT_EQ(1U, results.size()); + EXPECT_EQ(0, new_verified_profile.Compare(*results[0])); +} + +// Ensure that verified profiles can be saved via SaveImportedProfile, +// overwriting existing verified profiles as well. +TEST_F(PersonalDataManagerTest, SaveImportedProfileWithExistingVerifiedData) { + // Start with a verified profile. + AutofillProfile profile(base::GenerateGUID(), "Chrome settings"); + test::SetProfileInfo(&profile, + "Marion", "Mitchell", "Morrison", + "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA", + "91601", "US", "12345678910"); + EXPECT_TRUE(profile.IsVerified()); + + // Add the profile to the database. + personal_data_->AddProfile(profile); + + // Verify that the web database has been updated and the notification sent. + EXPECT_CALL(personal_data_observer_, + OnPersonalDataChanged()).WillOnce(QuitUIMessageLoop()); + base::MessageLoop::current()->Run(); + + AutofillProfile new_verified_profile = profile; + new_verified_profile.set_guid(base::GenerateGUID()); + new_verified_profile.SetRawInfo(COMPANY_NAME, ASCIIToUTF16("Fizzbang, Inc.")); + new_verified_profile.SetRawInfo(NAME_MIDDLE, base::string16()); + EXPECT_TRUE(new_verified_profile.IsVerified()); + + personal_data_->SaveImportedProfile(new_verified_profile); + + // Verify that the web database has been updated and the notification sent. + EXPECT_CALL(personal_data_observer_, + OnPersonalDataChanged()).WillOnce(QuitUIMessageLoop()); + base::MessageLoop::current()->Run(); + + // The new profile should be merged into the existing one. + AutofillProfile expected_profile = new_verified_profile; + expected_profile.set_guid(profile.guid()); + std::vector<base::string16> names; + expected_profile.GetRawMultiInfo(NAME_FULL, &names); + names.insert(names.begin(), ASCIIToUTF16("Marion Mitchell Morrison")); + expected_profile.SetRawMultiInfo(NAME_FULL, names); + + const std::vector<AutofillProfile*>& results = personal_data_->GetProfiles(); + ASSERT_EQ(1U, results.size()); + EXPECT_EQ(expected_profile, *results[0]); +} + +// Ensure that verified credit cards can be saved via SaveImportedCreditCard. +TEST_F(PersonalDataManagerTest, SaveImportedCreditCardWithVerifiedData) { + // Start with a verified credit card. + CreditCard credit_card(base::GenerateGUID(), "Chrome settings"); + test::SetCreditCardInfo(&credit_card, + "Biggie Smalls", "4111 1111 1111 1111" /* Visa */, "01", "2011"); + EXPECT_TRUE(credit_card.IsVerified()); + + // Add the credit card to the database. + personal_data_->AddCreditCard(credit_card); + + // Verify that the web database has been updated and the notification sent. + EXPECT_CALL(personal_data_observer_, + OnPersonalDataChanged()).WillOnce(QuitUIMessageLoop()); + base::MessageLoop::current()->Run(); + + CreditCard new_verified_card = credit_card; + new_verified_card.set_guid(base::GenerateGUID()); + new_verified_card.SetRawInfo(CREDIT_CARD_NAME, ASCIIToUTF16("B. Small")); + EXPECT_TRUE(new_verified_card.IsVerified()); + + personal_data_->SaveImportedCreditCard(new_verified_card); + + // Verify that the web database has been updated and the notification sent. + EXPECT_CALL(personal_data_observer_, + OnPersonalDataChanged()).WillOnce(QuitUIMessageLoop()); + base::MessageLoop::current()->Run(); + + // Expect that the saved credit card is updated. + const std::vector<CreditCard*>& results = personal_data_->GetCreditCards(); + ASSERT_EQ(1U, results.size()); + EXPECT_EQ(ASCIIToUTF16("B. Small"), results[0]->GetRawInfo(CREDIT_CARD_NAME)); +} + TEST_F(PersonalDataManagerTest, GetNonEmptyTypes) { // Check that there are no available types with no profiles stored. FieldTypeSet non_empty_types; |