Don't save duplicates of wallet addresses to local Autofill.

Also, change name of {S,G}etAutofillServerProfiles to {S,G}etServerProfiles to better match {S,G}etServerCreditCards

BUG=459741

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

Cr-Commit-Position: refs/heads/master@{#318955}

components/autofill/core/browser/autofill_profile.cc

@@ -567,28 +567,26 @@ bool AutofillProfile::IsSubsetOf(const AutofillProfile& profile,
   ServerFieldTypeSet types;
   GetNonEmptyTypes(app_locale, &types);
 
-  for (ServerFieldTypeSet::const_iterator it = types.begin(); it != types.end();
-       ++it) {
-    if (*it == NAME_FULL || *it == ADDRESS_HOME_STREET_ADDRESS) {
+  for (ServerFieldType type : types) {
+    if (type == NAME_FULL || type == ADDRESS_HOME_STREET_ADDRESS) {
       // Ignore the compound "full name" field type.  We are only interested in
       // comparing the constituent parts.  For example, if |this| has a middle
       // name saved, but |profile| lacks one, |profile| could still be a subset
       // of |this|.  Likewise, ignore the compound "street address" type, as we
       // are only interested in matching line-by-line.
       continue;
-    } else if (AutofillType(*it).group() == PHONE_HOME) {
+    } else if (AutofillType(type).group() == PHONE_HOME) {
       // Phone numbers should be canonicalized prior to being compared.
-      if (*it != PHONE_HOME_WHOLE_NUMBER) {
+      if (type != PHONE_HOME_WHOLE_NUMBER) {
         continue;
       } else if (!i18n::PhoneNumbersMatch(
-            GetRawInfo(*it),
-            profile.GetRawInfo(*it),
-            base::UTF16ToASCII(GetRawInfo(ADDRESS_HOME_COUNTRY)),
-            app_locale)) {
+                     GetRawInfo(type), profile.GetRawInfo(type),
+                     base::UTF16ToASCII(GetRawInfo(ADDRESS_HOME_COUNTRY)),
+                     app_locale)) {
         return false;
       }
-    } else if (base::StringToLowerASCII(GetRawInfo(*it)) !=
-                   base::StringToLowerASCII(profile.GetRawInfo(*it))) {
+    } else if (base::StringToLowerASCII(GetRawInfo(type)) !=
+               base::StringToLowerASCII(profile.GetRawInfo(type))) {
       return false;
     }
   }

components/autofill/core/browser/personal_data_manager.cc

@@ -1123,7 +1123,7 @@ void PersonalDataManager::LoadProfiles() {
   CancelPendingQuery(&pending_server_profiles_query_);
 
   pending_profiles_query_ = database_->GetAutofillProfiles(this);
-  pending_server_profiles_query_ = database_->GetAutofillServerProfiles(this);
+  pending_server_profiles_query_ = database_->GetServerProfiles(this);
 }
 
 // Win, Linux, Android and iOS implementations do nothing. Mac implementation
@@ -1164,12 +1164,16 @@ std::string PersonalDataManager::SaveImportedProfile(
     return std::string();
 
   // Don't save a web profile if the data in the profile is a subset of an
-  // auxiliary profile.
-  for (std::vector<AutofillProfile*>::const_iterator iter =
-           auxiliary_profiles_.begin();
-       iter != auxiliary_profiles_.end(); ++iter) {
-    if (imported_profile.IsSubsetOf(**iter, app_locale_))
-      return (*iter)->guid();
+  // auxiliary profile...
+  for (AutofillProfile* profile : auxiliary_profiles_) {
+    if (imported_profile.IsSubsetOf(*profile, app_locale_))
+      return profile->guid();
+  }
+
+  // ...or server profile.
+  for (AutofillProfile* profile : server_profiles_) {
+    if (imported_profile.IsSubsetOf(*profile, app_locale_))
+      return profile->guid();
   }
 
   std::vector<AutofillProfile> profiles;

components/autofill/core/browser/personal_data_manager_unittest.cc

@@ -185,6 +185,45 @@ TEST_F(PersonalDataManagerTest, AddProfile) {
   ExpectSameElements(profiles, personal_data_->GetProfiles());
 }
 
+TEST_F(PersonalDataManagerTest, DontDuplicateServerProfile) {
+  EnableWalletCardImport();
+
+  std::vector<AutofillProfile> server_profiles;
+  server_profiles.push_back(
+      AutofillProfile(AutofillProfile::SERVER_PROFILE, "a123"));
+  test::SetProfileInfo(&server_profiles.back(), "John", "", "Doe", "",
+                       "ACME Corp", "500 Oak View", "Apt 8", "Houston", "TX",
+                       "77401", "US", "");
+  // Wallet only provides a full name, so the above first and last names
+  // will be ignored when the profile is written to the DB.
+  server_profiles.back().SetRawInfo(NAME_FULL, ASCIIToUTF16("John Doe"));
+  autofill_table_->SetServerProfiles(server_profiles);
+  personal_data_->Refresh();
+  EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
+      .WillOnce(QuitMainMessageLoop());
+  base::MessageLoop::current()->Run();
+  EXPECT_EQ(1U, personal_data_->GetProfiles().size());
+
+  // Add profile with identical values.  Duplicates should not get saved.
+  AutofillProfile scraped_profile(base::GenerateGUID(),
+                                  "https://www.example.com");
+  test::SetProfileInfo(&scraped_profile, "John", "", "Doe", "", "ACME Corp",
+                       "500 Oak View", "Apt 8", "Houston", "TX", "77401", "US",
+                       "");
+  EXPECT_TRUE(scraped_profile.IsSubsetOf(server_profiles.back(), "en-US"));
+  std::string saved_guid = personal_data_->SaveImportedProfile(scraped_profile);
+  EXPECT_NE(scraped_profile.guid(), saved_guid);
+
+  personal_data_->Refresh();
+  EXPECT_CALL(personal_data_observer_, OnPersonalDataChanged())
+      .WillOnce(QuitMainMessageLoop());
+  base::MessageLoop::current()->Run();
+
+  // Verify the non-addition.
+  EXPECT_EQ(1U, personal_data_->GetProfiles().size());
+  EXPECT_EQ(0U, personal_data_->web_profiles().size());
+}
+
 TEST_F(PersonalDataManagerTest, AddUpdateRemoveProfiles) {
   AutofillProfile profile0(base::GenerateGUID(), "https://www.example.com");
   test::SetProfileInfo(&profile0,

components/autofill/core/browser/webdata/autofill_table.cc

@@ -895,8 +895,7 @@ bool AutofillTable::GetAutofillProfiles(
   return s.Succeeded();
 }
 
-bool AutofillTable::GetAutofillServerProfiles(
-    std::vector<AutofillProfile*>* profiles) {
+bool AutofillTable::GetServerProfiles(std::vector<AutofillProfile*>* profiles) {
   profiles->clear();
 
   sql::Statement s(db_->GetUniqueStatement(
@@ -920,7 +919,7 @@ bool AutofillTable::GetAutofillServerProfiles(
     scoped_ptr<AutofillProfile> profile(new AutofillProfile(
         AutofillProfile::SERVER_PROFILE, s.ColumnString(index++)));
 
-    profile->SetRawInfo(NAME_FULL, s.ColumnString16(index++));
+    base::string16 recipient_name = s.ColumnString16(index++);
     profile->SetRawInfo(COMPANY_NAME, s.ColumnString16(index++));
     profile->SetRawInfo(ADDRESS_HOME_STREET_ADDRESS, s.ColumnString16(index++));
     profile->SetRawInfo(ADDRESS_HOME_STATE, s.ColumnString16(index++));
@@ -933,13 +932,18 @@ bool AutofillTable::GetAutofillServerProfiles(
     profile->SetRawInfo(ADDRESS_HOME_COUNTRY, s.ColumnString16(index++));
     profile->set_language_code(s.ColumnString(index++));
 
+    // SetInfo instead of SetRawInfo on the name so the constituent pieces will
+    // be parsed.
+    profile->SetInfo(AutofillType(NAME_FULL), recipient_name,
+                     profile->language_code());
+
     profiles->push_back(profile.release());
   }
 
   return s.Succeeded();
 }
 
-void AutofillTable::SetAutofillServerProfiles(
+void AutofillTable::SetServerProfiles(
     const std::vector<AutofillProfile>& profiles) {
   // Delete all old ones first.
   sql::Statement delete_old(db_->GetUniqueStatement(

components/autofill/core/browser/webdata/autofill_table.h

@@ -281,12 +281,11 @@ class AutofillTable : public WebDatabaseTable {
   // Retrieves local/server profiles in the database. Caller owns the returned
   // profiles.
   virtual bool GetAutofillProfiles(std::vector<AutofillProfile*>* profiles);
-  virtual bool GetAutofillServerProfiles(
-      std::vector<AutofillProfile*>* profiles);
+  virtual bool GetServerProfiles(std::vector<AutofillProfile*>* profiles);
 
   // Sets the server profiles. All old profiles are deleted and replaced with
   // the given ones.
-  void SetAutofillServerProfiles(const std::vector<AutofillProfile>& profiles);
+  void SetServerProfiles(const std::vector<AutofillProfile>& profiles);
 
   // Records a single credit card in the credit_cards table.
   bool AddCreditCard(const CreditCard& credit_card);

components/autofill/core/browser/webdata/autofill_table_unittest.cc

@@ -1768,10 +1768,10 @@ TEST_F(AutofillTableTest, SetServerProfile) {
   AutofillProfile one(AutofillProfile::SERVER_PROFILE, "a123");
   std::vector<AutofillProfile> inputs;
   inputs.push_back(one);
-  table_->SetAutofillServerProfiles(inputs);
+  table_->SetServerProfiles(inputs);
 
   std::vector<AutofillProfile*> outputs;
-  table_->GetAutofillServerProfiles(&outputs);
+  table_->GetServerProfiles(&outputs);
   ASSERT_EQ(1u, outputs.size());
   EXPECT_EQ(one.server_id(), outputs[0]->server_id());
 
@@ -1781,10 +1781,10 @@ TEST_F(AutofillTableTest, SetServerProfile) {
   // Set a different profile.
   AutofillProfile two(AutofillProfile::SERVER_PROFILE, "b456");
   inputs[0] = two;
-  table_->SetAutofillServerProfiles(inputs);
+  table_->SetServerProfiles(inputs);
 
   // The original one should have been replaced.
-  table_->GetAutofillServerProfiles(&outputs);
+  table_->GetServerProfiles(&outputs);
   ASSERT_EQ(1u, outputs.size());
   EXPECT_EQ(two.server_id(), outputs[0]->server_id());
 

components/autofill/core/browser/webdata/autofill_wallet_syncable_service.cc

@@ -260,10 +260,8 @@ syncer::SyncMergeResult AutofillWalletSyncableService::SetSyncData(
       &AutofillTable::SetServerCreditCards,
       &prev_card_count);
   bool changed_addresses = SetDataIfChanged(
-      table, wallet_addresses,
-      &AutofillTable::GetAutofillServerProfiles,
-      &AutofillTable::SetAutofillServerProfiles,
-      &prev_address_count);
+      table, wallet_addresses, &AutofillTable::GetServerProfiles,
+      &AutofillTable::SetServerProfiles, &prev_address_count);
 
   syncer::SyncMergeResult merge_result(syncer::AUTOFILL_WALLET_DATA);
   merge_result.set_num_items_before_association(

components/autofill/core/browser/webdata/autofill_webdata.h

@@ -75,7 +75,7 @@ class AutofillWebData {
   // consumer owns the profiles.
   virtual WebDataServiceBase::Handle GetAutofillProfiles(
       WebDataServiceConsumer* consumer) = 0;
-  virtual WebDataServiceBase::Handle GetAutofillServerProfiles(
+  virtual WebDataServiceBase::Handle GetServerProfiles(
       WebDataServiceConsumer* consumer) = 0;
 
   // Schedules a task to update autofill entries in the web database.

components/autofill/core/browser/webdata/autofill_webdata_backend_impl.cc

@@ -245,11 +245,11 @@ scoped_ptr<WDTypedResult> AutofillWebDataBackendImpl::GetAutofillProfiles(
               base::Unretained(this))));
 }
 
-scoped_ptr<WDTypedResult> AutofillWebDataBackendImpl::GetAutofillServerProfiles(
+scoped_ptr<WDTypedResult> AutofillWebDataBackendImpl::GetServerProfiles(
     WebDatabase* db) {
   DCHECK(db_thread_->BelongsToCurrentThread());
   std::vector<AutofillProfile*> profiles;
-  AutofillTable::FromWebDatabase(db)->GetAutofillServerProfiles(&profiles);
+  AutofillTable::FromWebDatabase(db)->GetServerProfiles(&profiles);
   return scoped_ptr<WDTypedResult>(
       new WDDestroyableResult<std::vector<AutofillProfile*> >(
           AUTOFILL_PROFILES_RESULT,

components/autofill/core/browser/webdata/autofill_webdata_backend_impl.h

@@ -112,7 +112,7 @@ class AutofillWebDataBackendImpl
 
   // Returns the local/server Autofill profiles from the web database.
   scoped_ptr<WDTypedResult> GetAutofillProfiles(WebDatabase* db);
-  scoped_ptr<WDTypedResult> GetAutofillServerProfiles(WebDatabase* db);
+  scoped_ptr<WDTypedResult> GetServerProfiles(WebDatabase* db);
 
   // Updates Autofill entries in the web database.
   WebDatabase::State UpdateAutofillEntries(

components/autofill/core/browser/webdata/autofill_webdata_service.cc

@@ -129,11 +129,11 @@ WebDataServiceBase::Handle AutofillWebDataService::GetAutofillProfiles(
       consumer);
 }
 
-WebDataServiceBase::Handle AutofillWebDataService::GetAutofillServerProfiles(
+WebDataServiceBase::Handle AutofillWebDataService::GetServerProfiles(
     WebDataServiceConsumer* consumer) {
-  return wdbs_->ScheduleDBTaskWithResult(FROM_HERE,
-      Bind(&AutofillWebDataBackendImpl::GetAutofillServerProfiles,
-           autofill_backend_),
+  return wdbs_->ScheduleDBTaskWithResult(
+      FROM_HERE,
+      Bind(&AutofillWebDataBackendImpl::GetServerProfiles, autofill_backend_),
       consumer);
 }
 

components/autofill/core/browser/webdata/autofill_webdata_service.h

@@ -72,7 +72,7 @@ class AutofillWebDataService : public AutofillWebData,
       WebDataServiceConsumer* consumer) override;
 
   // Server profiles.
-  WebDataServiceBase::Handle GetAutofillServerProfiles(
+  WebDataServiceBase::Handle GetServerProfiles(
       WebDataServiceConsumer* consumer) override;
 
   void UpdateAutofillEntries(