Added multi-valued field sync to Autofill (client) (Issue: Autofill profiles with multi-valued...)

BUG=77444
TEST=unit-tests


Review URL: http://codereview.chromium.org/8969006

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

7 files changed, 473 insertions, 111 deletions

diff --git a/chrome/browser/autofill/autofill_profile.h b/chrome/browser/autofill/autofill_profile.h
index df6fe30..c58d084 100644
--- a/chrome/browser/autofill/autofill_profile.h
+++ b/chrome/browser/autofill/autofill_profile.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -109,7 +109,8 @@ class AutofillProfile : public FormGroup {
   // multi-valued items.
   int Compare(const AutofillProfile& profile) const;
 
-  // Comparison for Sync.  Same as |Compare| but includes multi-valued fields.
+  // Full profile comparision, use instead of |Compare|. Same as |Compare| but
+  // includes multi-valued fields.
   int CompareMulti(const AutofillProfile& profile) const;
 
   // Equality operators compare GUIDs and the contents in the comparison.
diff --git a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
index de786b5..be71eae 100644
--- a/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
+++ b/chrome/browser/sync/profile_sync_service_autofill_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -114,7 +114,7 @@ class AutofillTableMock : public AutofillTable {
                bool(const std::vector<AutofillEntry>&));  // NOLINT
   MOCK_METHOD1(GetAutofillProfiles,
                bool(std::vector<AutofillProfile*>*));  // NOLINT
-  MOCK_METHOD1(UpdateAutofillProfile,
+  MOCK_METHOD1(UpdateAutofillProfileMulti,
                bool(const AutofillProfile&));  // NOLINT
   MOCK_METHOD1(AddAutofillProfile,
                bool(const AutofillProfile&));  // NOLINT
@@ -122,6 +122,11 @@ class AutofillTableMock : public AutofillTable {
                bool(const std::string&));  // NOLINT
 };
 
+MATCHER_P(MatchProfiles, profile, "") {
+  return (profile.CompareMulti(arg) == 0);
+}
+
+
 class WebDatabaseFake : public WebDatabase {
  public:
   explicit WebDatabaseFake(AutofillTable* autofill_table)
@@ -359,6 +364,10 @@ class ProfileSyncServiceAutofillTest : public AbstractProfileSyncServiceTest {
     personal_data_manager_ = static_cast<PersonalDataManagerMock*>(
         PersonalDataManagerFactory::GetInstance()->SetTestingFactoryAndUse(
             &profile_, PersonalDataManagerMock::Build));
+    // GetHistoryService() gets called indirectly, but the result is ignored, so
+    // it is safe to return NULL.
+    EXPECT_CALL(profile_, GetHistoryService(_)).
+        WillRepeatedly(Return(static_cast<HistoryService*>(NULL)));
     EXPECT_CALL(*personal_data_manager_, LoadProfiles()).Times(1);
     EXPECT_CALL(*personal_data_manager_, LoadCreditCards()).Times(1);
     EXPECT_CALL(profile_, GetWebDataService(_)).
@@ -723,6 +732,29 @@ class FakeServerUpdater : public base::RefCountedThreadSafe<FakeServerUpdater> {
   syncable::Id parent_id_;
 };
 
+namespace {
+
+// Checks if the field of type |field_type| in |profile1| includes all values
+// of the field in |profile2|.
+bool IncludesField(const AutofillProfile& profile1,
+                   const AutofillProfile& profile2,
+                   AutofillFieldType field_type) {
+  std::vector<string16> values1;
+  profile1.GetMultiInfo(field_type, &values1);
+  std::vector<string16> values2;
+  profile2.GetMultiInfo(field_type, &values2);
+
+  std::set<string16> values_set;
+  for (size_t i = 0; i < values1.size(); ++i)
+    values_set.insert(values1[i]);
+  for (size_t i = 0; i < values2.size(); ++i)
+    if (values_set.find(values2[i]) == values_set.end())
+      return false;
+  return true;
+}
+
+};
+
 // TODO(skrul): Test abort startup.
 // TODO(skrul): Test processing of cloud changes.
 // TODO(tim): Add autofill data type controller test, and a case to cover
@@ -897,7 +929,8 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeProfile) {
   sync_profiles.push_back(sync_profile);
   AddAutofillHelper<AutofillProfile> add_autofill(this, sync_profiles);
 
-  EXPECT_CALL(autofill_table_, UpdateAutofillProfile(_)).
+  EXPECT_CALL(autofill_table_,
+              UpdateAutofillProfileMulti(MatchProfiles(sync_profile))).
       WillOnce(Return(true));
   EXPECT_CALL(*personal_data_manager_, Refresh());
   StartSyncService(add_autofill.callback(), false, syncable::AUTOFILL_PROFILE);
@@ -907,7 +940,57 @@ TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeProfile) {
   ASSERT_TRUE(GetAutofillProfilesFromSyncDBUnderProfileNode(
       &new_sync_profiles));
   ASSERT_EQ(1U, new_sync_profiles.size());
-  EXPECT_EQ(0, sync_profile.Compare(new_sync_profiles[0]));
+  EXPECT_EQ(0, sync_profile.CompareMulti(new_sync_profiles[0]));
+}
+
+TEST_F(ProfileSyncServiceAutofillTest, HasNativeHasSyncMergeProfileCombine) {
+  AutofillProfile sync_profile;
+  autofill_test::SetProfileInfoWithGuid(&sync_profile,
+      "23355099-1170-4B71-8ED4-144470CC9EBE", "Billing",
+      "Mitchell", "Morrison",
+      "johnwayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA",
+      "91601", "US", "12345678910");
+
+  AutofillProfile* native_profile = new AutofillProfile;
+  // Same address, but different names, phones and e-mails.
+  autofill_test::SetProfileInfoWithGuid(native_profile,
+      "23355099-1170-4B71-8ED4-144470CC9EBF", "Billing", "Alicia", "Saenz",
+      "joewayne@me.xyz", "Fox", "123 Zoo St.", "unit 5", "Hollywood", "CA",
+      "91601", "US", "19482937549");
+
+  AutofillProfile expected_profile(sync_profile);
+  expected_profile.OverwriteWithOrAddTo(*native_profile);
+
+  std::vector<AutofillProfile*> native_profiles;
+  native_profiles.push_back(native_profile);
+  EXPECT_CALL(autofill_table_, GetAutofillProfiles(_)).
+      WillOnce(DoAll(SetArgumentPointee<0>(native_profiles), Return(true)));
+  EXPECT_CALL(autofill_table_,
+              AddAutofillProfile(MatchProfiles(expected_profile))).
+      WillOnce(Return(true));
+  EXPECT_CALL(autofill_table_,
+              RemoveAutofillProfile("23355099-1170-4B71-8ED4-144470CC9EBF")).
+      WillOnce(Return(true));
+  std::vector<AutofillProfile> sync_profiles;
+  sync_profiles.push_back(sync_profile);
+  AddAutofillHelper<AutofillProfile> add_autofill(this, sync_profiles);
+
+  EXPECT_CALL(*personal_data_manager_, Refresh());
+  StartSyncService(add_autofill.callback(), false, syncable::AUTOFILL_PROFILE);
+  ASSERT_TRUE(add_autofill.success());
+
+  std::vector<AutofillProfile> new_sync_profiles;
+  ASSERT_TRUE(GetAutofillProfilesFromSyncDBUnderProfileNode(
+      &new_sync_profiles));
+  ASSERT_EQ(1U, new_sync_profiles.size());
+  // Check that key fields are the same.
+  EXPECT_TRUE(new_sync_profiles[0].IsSubsetOf(sync_profile));
+  // Check that multivalued fields of the synced back data include original
+  // data.
+  EXPECT_TRUE(IncludesField(new_sync_profiles[0], sync_profile, NAME_FULL));
+  EXPECT_TRUE(IncludesField(new_sync_profiles[0], sync_profile, EMAIL_ADDRESS));
+  EXPECT_TRUE(IncludesField(new_sync_profiles[0], sync_profile,
+                            PHONE_HOME_WHOLE_NUMBER));
 }
 
 TEST_F(ProfileSyncServiceAutofillTest, MergeProfileWithDifferentGuid) {
@@ -1114,10 +1197,6 @@ TEST_F(ProfileSyncServiceAutofillTest, ProcessUserChangeRemoveProfile) {
 }
 
 TEST_F(ProfileSyncServiceAutofillTest, FLAKY_ServerChangeRace) {
-  // GetHistoryService() gets called indirectly, but the result is ignored, so
-  // it is safe to return NULL.
-  EXPECT_CALL(profile_, GetHistoryService(_)).
-      WillRepeatedly(Return(static_cast<HistoryService*>(NULL)));
   // Once for MergeDataAndStartSyncing() and twice for ProcessSyncChanges(), via
   // LoadAutofillData().
   EXPECT_CALL(autofill_table_, GetAllAutofillEntries(_)).
diff --git a/chrome/browser/sync/protocol/autofill_specifics.proto b/chrome/browser/sync/protocol/autofill_specifics.proto
index d9026b1..0e9aded 100644
--- a/chrome/browser/sync/protocol/autofill_specifics.proto
+++ b/chrome/browser/sync/protocol/autofill_specifics.proto
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 //
@@ -26,10 +26,10 @@ message AutofillProfileSpecifics {
   optional string guid = 15;
 
   // Contact info.
-  optional string name_first = 2;
-  optional string name_middle = 3;
-  optional string name_last = 4;
-  optional string email_address = 5;
+  repeated string name_first = 2;
+  repeated string name_middle = 3;
+  repeated string name_last = 4;
+  repeated string email_address = 5;
   optional string company_name = 6;
 
   // Home address.
@@ -41,11 +41,10 @@ message AutofillProfileSpecifics {
   optional string address_home_country = 12;
 
   // Phone.
-  optional string phone_home_whole_number = 13;
+  repeated string phone_home_whole_number = 13;
 
   // Deprecated.
-  // TODO(jhawkins): Determine if this is safe to remove.
-  optional string phone_fax_whole_number = 14;
+  optional string phone_fax_whole_number = 14 [deprecated=true];
 }
 
 message AutofillSpecifics {
diff --git a/chrome/browser/sync/protocol/proto_value_conversions.cc b/chrome/browser/sync/protocol/proto_value_conversions.cc
index 58b4c1d..3885fc2 100644
--- a/chrome/browser/sync/protocol/proto_value_conversions.cc
+++ b/chrome/browser/sync/protocol/proto_value_conversions.cc
@@ -84,6 +84,13 @@ ListValue* MakeRepeatedValue(const F& fields, V* (*converter_fn)(T)) {
 #define SET_INT64(field) SET(field, MakeInt64Value)
 #define SET_INT64_REP(field) SET_REP(field, MakeInt64Value)
 #define SET_STR(field) SET(field, Value::CreateStringValue)
+#define SET_STR_REP(field) \
+  value->Set(#field, \
+             MakeRepeatedValue<const std::string&, \
+                               google::protobuf::RepeatedPtrField< \
+                                   std::string >, \
+                               StringValue>(proto.field(), \
+                                            Value::CreateStringValue))
 
 #define SET_EXTENSION(ns, field, fn)                                    \
   do {                                                                  \
@@ -232,10 +239,10 @@ DictionaryValue* AutofillProfileSpecificsToValue(
   SET_STR(label);
   SET_STR(guid);
 
-  SET_STR(name_first);
-  SET_STR(name_middle);
-  SET_STR(name_last);
-  SET_STR(email_address);
+  SET_STR_REP(name_first);
+  SET_STR_REP(name_middle);
+  SET_STR_REP(name_last);
+  SET_STR_REP(email_address);
   SET_STR(company_name);
 
   SET_STR(address_home_line1);
@@ -245,8 +252,7 @@ DictionaryValue* AutofillProfileSpecificsToValue(
   SET_STR(address_home_zip);
   SET_STR(address_home_country);
 
-  SET_STR(phone_home_whole_number);
-  SET_STR(phone_fax_whole_number);
+  SET_STR_REP(phone_home_whole_number);
   return value;
 }
 
@@ -400,6 +406,7 @@ DictionaryValue* EntitySpecificsToValue(
 #undef SET_INT64
 #undef SET_INT64_REP
 #undef SET_STR
+#undef SET_STR_REP
 
 #undef SET_EXTENSION
 
diff --git a/chrome/browser/webdata/autofill_profile_syncable_service.cc b/chrome/browser/webdata/autofill_profile_syncable_service.cc
index 028044c..ee2fc49 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service.cc
+++ b/chrome/browser/webdata/autofill_profile_syncable_service.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -24,17 +24,6 @@ using content::BrowserThread;
 
 namespace {
 
-// Helper to compare the local value and cloud value of a field, merge into
-// the local value if they differ, and return whether the merge happened.
-bool MergeField(FormGroup* form_group,
-                AutofillFieldType field_type,
-                const std::string& specifics_field) {
-  if (UTF16ToUTF8(form_group->GetInfo(field_type)) == specifics_field)
-    return false;
-  form_group->SetInfo(field_type, UTF8ToUTF16(specifics_field));
-  return true;
-}
-
 std::string LimitData(const std::string& data) {
   std::string sanitized_value(data);
   if (sanitized_value.length() > AutofillTable::kMaxDataLength)
@@ -72,7 +61,7 @@ SyncError AutofillProfileSyncableService::MergeDataAndStartSyncing(
     SyncChangeProcessor* sync_processor) {
   DCHECK(CalledOnValidThread());
   DCHECK(!sync_processor_.get());
-  VLOG(1) << "Associating Autofill: MergeDataAndStartSyncing";
+  DVLOG(1) << "Associating Autofill: MergeDataAndStartSyncing";
 
   if (!LoadAutofillData(&profiles_.get())) {
     return SyncError(
@@ -80,17 +69,17 @@ SyncError AutofillProfileSyncableService::MergeDataAndStartSyncing(
         model_type());
   }
 
-  if (VLOG_IS_ON(2)) {
-    VLOG(2) << "[AUTOFILL MIGRATION]"
-            << "Printing profiles from web db";
+  if (DLOG_IS_ON(INFO)) {
+    DVLOG(2) << "[AUTOFILL MIGRATION]"
+             << "Printing profiles from web db";
 
     for (ScopedVector<AutofillProfile>::const_iterator ix =
-        profiles_.begin(); ix != profiles_.end(); ++ix) {
+         profiles_.begin(); ix != profiles_.end(); ++ix) {
       AutofillProfile* p = *ix;
-      VLOG(2) << "[AUTOFILL MIGRATION]  "
-              << p->GetInfo(NAME_FIRST)
-              << p->GetInfo(NAME_LAST)
-              << p->guid();
+      DVLOG(2) << "[AUTOFILL MIGRATION]  "
+               << p->GetInfo(NAME_FIRST)
+               << p->GetInfo(NAME_LAST)
+               << p->guid();
     }
   }
 
@@ -106,10 +95,35 @@ SyncError AutofillProfileSyncableService::MergeDataAndStartSyncing(
        ++sync_iter) {
     GUIDToProfileMap::iterator it =
         CreateOrUpdateProfile(*sync_iter, &remaining_profiles, &bundle);
+    // |it| points to created/updated profile. Add it to the |profiles_map_| and
+    // then remove it from |remaining_profiles|. After this loop is completed
+    // |remaining_profiles| will have only those profiles that are not in the
+    // sync.
     profiles_map_[it->first] = it->second;
     remaining_profiles.erase(it);
   }
 
+  // Check for similar unmatched profiles - they are created independently on
+  // two systems, so merge them.
+  for (GUIDToProfileMap::iterator it = bundle.candidates_to_merge.begin();
+       it != bundle.candidates_to_merge.end(); ++it) {
+    GUIDToProfileMap::iterator profile_to_merge =
+        remaining_profiles.find(it->first);
+    if (profile_to_merge != remaining_profiles.end()) {
+      bundle.profiles_to_delete.push_back(profile_to_merge->second->guid());
+      if (MergeProfile(*(profile_to_merge->second), it->second))
+        bundle.profiles_to_sync_back.push_back(it->second);
+      DVLOG(2) << "[AUTOFILL SYNC]"
+               << "Found similar profile in sync db but with a different guid: "
+               << UTF16ToUTF8(it->second->GetInfo(NAME_FIRST))
+               << UTF16ToUTF8(it->second->GetInfo(NAME_LAST))
+               << "New guid " << it->second->guid()
+               << ". Profile to be deleted "
+               << profile_to_merge->second->guid();
+      remaining_profiles.erase(profile_to_merge);
+    }
+  }
+
   if (!SaveChangesToWebData(bundle))
     return SyncError(FROM_HERE, "Failed to update webdata.", model_type());
 
@@ -121,6 +135,12 @@ SyncError AutofillProfileSyncableService::MergeDataAndStartSyncing(
     profiles_map_[i->first] = i->second;
   }
 
+  for (size_t i = 0; i < bundle.profiles_to_sync_back.size(); ++i) {
+    new_changes.push_back(
+        SyncChange(SyncChange::ACTION_UPDATE,
+                   CreateData(*(bundle.profiles_to_sync_back[i]))));
+  }
+
   SyncError error;
   if (!new_changes.empty())
     error = sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes);
@@ -222,24 +242,25 @@ bool AutofillProfileSyncableService::SaveChangesToWebData(
     const DataBundle& bundle) {
   DCHECK(CalledOnValidThread());
 
+  bool success = true;
+  for (size_t i = 0; i< bundle.profiles_to_delete.size(); ++i) {
+    if (!GetAutofillTable()->RemoveAutofillProfile(
+        bundle.profiles_to_delete[i]))
+      success = false;
+  }
+
   for (size_t i = 0; i < bundle.profiles_to_add.size(); i++) {
     if (!GetAutofillTable()->AddAutofillProfile(
         *bundle.profiles_to_add[i]))
-      return false;
+      success = false;
   }
 
   for (size_t i = 0; i < bundle.profiles_to_update.size(); i++) {
-    if (!GetAutofillTable()->UpdateAutofillProfile(
+    if (!GetAutofillTable()->UpdateAutofillProfileMulti(
         *bundle.profiles_to_update[i]))
-      return false;
+      success = false;
   }
-
-  for (size_t i = 0; i< bundle.profiles_to_delete.size(); ++i) {
-    if (!GetAutofillTable()->RemoveAutofillProfile(
-        bundle.profiles_to_delete[i]))
-      return false;
-  }
-  return true;
+  return success;
 }
 
 // static
@@ -247,25 +268,30 @@ bool AutofillProfileSyncableService::OverwriteProfileWithServerData(
     const sync_pb::AutofillProfileSpecifics& specifics,
     AutofillProfile* profile) {
   bool diff = false;
-  diff = MergeField(profile, NAME_FIRST, specifics.name_first()) || diff;
-  diff = MergeField(profile, NAME_LAST, specifics.name_last()) || diff;
-  diff = MergeField(profile, NAME_MIDDLE, specifics.name_middle()) || diff;
-  diff = MergeField(profile, ADDRESS_HOME_LINE1,
-                    specifics.address_home_line1()) || diff;
-  diff = MergeField(profile, ADDRESS_HOME_LINE2,
-                    specifics.address_home_line2()) || diff;
-  diff = MergeField(profile, ADDRESS_HOME_CITY,
-                    specifics.address_home_city()) || diff;
-  diff = MergeField(profile, ADDRESS_HOME_STATE,
-                    specifics.address_home_state()) || diff;
-  diff = MergeField(profile, ADDRESS_HOME_COUNTRY,
-                    specifics.address_home_country()) || diff;
-  diff = MergeField(profile, ADDRESS_HOME_ZIP,
-                    specifics.address_home_zip()) || diff;
-  diff = MergeField(profile, EMAIL_ADDRESS, specifics.email_address()) || diff;
-  diff = MergeField(profile, COMPANY_NAME, specifics.company_name()) || diff;
-  diff = MergeField(profile, PHONE_HOME_WHOLE_NUMBER,
-                    specifics.phone_home_whole_number()) || diff;
+  diff = UpdateMultivaluedField(NAME_FIRST,
+                                specifics.name_first(), profile) || diff;
+  diff = UpdateMultivaluedField(NAME_MIDDLE,
+                                specifics.name_middle(), profile) || diff;
+  diff = UpdateMultivaluedField(NAME_LAST,
+                                specifics.name_last(), profile) || diff;
+  diff = UpdateField(ADDRESS_HOME_LINE1,
+                     specifics.address_home_line1(), profile) || diff;
+  diff = UpdateField(ADDRESS_HOME_LINE2,
+                     specifics.address_home_line2(), profile) || diff;
+  diff = UpdateField(ADDRESS_HOME_CITY,
+                     specifics.address_home_city(), profile) || diff;
+  diff = UpdateField(ADDRESS_HOME_STATE,
+                     specifics.address_home_state(), profile) || diff;
+  diff = UpdateField(ADDRESS_HOME_COUNTRY,
+                     specifics.address_home_country(), profile) || diff;
+  diff = UpdateField(ADDRESS_HOME_ZIP,
+                     specifics.address_home_zip(), profile) || diff;
+  diff = UpdateMultivaluedField(EMAIL_ADDRESS,
+                                specifics.email_address(), profile) || diff;
+  diff = UpdateField(COMPANY_NAME, specifics.company_name(), profile) || diff;
+  diff = UpdateMultivaluedField(PHONE_HOME_WHOLE_NUMBER,
+                                specifics.phone_home_whole_number(),
+                                profile) || diff;
   return diff;
 }
 
@@ -278,12 +304,24 @@ void AutofillProfileSyncableService::WriteAutofillProfile(
 
   DCHECK(guid::IsValidGUID(profile.guid()));
 
+  // Reset all multi-valued fields in the protobuf.
+  specifics->clear_name_first();
+  specifics->clear_name_middle();
+  specifics->clear_name_last();
+  specifics->clear_email_address();
+  specifics->clear_phone_home_whole_number();
+
   specifics->set_guid(profile.guid());
-  specifics->set_name_first(
-      LimitData(UTF16ToUTF8(profile.GetInfo(NAME_FIRST))));
-  specifics->set_name_middle(
-      LimitData(UTF16ToUTF8(profile.GetInfo(NAME_MIDDLE))));
-  specifics->set_name_last(LimitData(UTF16ToUTF8(profile.GetInfo(NAME_LAST))));
+  std::vector<string16> values;
+  profile.GetMultiInfo(NAME_FIRST, &values);
+  for (size_t i = 0; i < values.size(); ++i)
+    specifics->add_name_first(LimitData(UTF16ToUTF8(values[i])));
+  profile.GetMultiInfo(NAME_MIDDLE, &values);
+  for (size_t i = 0; i < values.size(); ++i)
+    specifics->add_name_middle(LimitData(UTF16ToUTF8(values[i])));
+  profile.GetMultiInfo(NAME_LAST, &values);
+  for (size_t i = 0; i < values.size(); ++i)
+    specifics->add_name_last(LimitData(UTF16ToUTF8(values[i])));
   specifics->set_address_home_line1(
       LimitData(UTF16ToUTF8(profile.GetInfo(ADDRESS_HOME_LINE1))));
   specifics->set_address_home_line2(
@@ -296,12 +334,14 @@ void AutofillProfileSyncableService::WriteAutofillProfile(
       LimitData(UTF16ToUTF8(profile.GetInfo(ADDRESS_HOME_COUNTRY))));
   specifics->set_address_home_zip(
       LimitData(UTF16ToUTF8(profile.GetInfo(ADDRESS_HOME_ZIP))));
-  specifics->set_email_address(
-      LimitData(UTF16ToUTF8(profile.GetInfo(EMAIL_ADDRESS))));
+  profile.GetMultiInfo(EMAIL_ADDRESS, &values);
+  for (size_t i = 0; i < values.size(); ++i)
+    specifics->add_email_address(LimitData(UTF16ToUTF8(values[i])));
   specifics->set_company_name(
       LimitData(UTF16ToUTF8(profile.GetInfo(COMPANY_NAME))));
-  specifics->set_phone_home_whole_number(
-      LimitData(UTF16ToUTF8(profile.GetInfo(PHONE_HOME_WHOLE_NUMBER))));
+  profile.GetMultiInfo(PHONE_HOME_WHOLE_NUMBER, &values);
+  for (size_t i = 0; i < values.size(); ++i)
+    specifics->add_phone_home_whole_number(LimitData(UTF16ToUTF8(values[i])));
 }
 
 void AutofillProfileSyncableService::CreateGUIDToProfileMap(
@@ -340,16 +380,21 @@ AutofillProfileSyncableService::CreateOrUpdateProfile(
     // Check if profile appears under a different guid.
     for (GUIDToProfileMap::iterator i = profile_map->begin();
          i != profile_map->end(); ++i) {
-      if (i->second->Compare(*new_profile) == 0) {
+      if (i->second->CompareMulti(*new_profile) == 0) {
         bundle->profiles_to_delete.push_back(i->second->guid());
-        VLOG(2) << "[AUTOFILL SYNC]"
-                << "Found in sync db but with a different guid: "
-                << UTF16ToUTF8(it->second->GetInfo(NAME_FIRST))
-                << UTF16ToUTF8(it->second->GetInfo(NAME_LAST))
-                << "New guid " << new_profile->guid()
-                << ". Profile to be deleted " << it->second->guid();
+        DVLOG(2) << "[AUTOFILL SYNC]"
+                 << "Found in sync db but with a different guid: "
+                 << UTF16ToUTF8(i->second->GetInfo(NAME_FIRST))
+                 << UTF16ToUTF8(i->second->GetInfo(NAME_LAST))
+                 << "New guid " << new_profile->guid()
+                 << ". Profile to be deleted " << i->second->guid();
         profile_map->erase(i);
         break;
+      } else if (i->second->PrimaryValue() == new_profile->PrimaryValue()) {
+        // Add it to candidates for merge - if there is no profile with this
+        // guid we will merge them.
+        bundle->candidates_to_merge.insert(std::make_pair(i->second->guid(),
+                                                          new_profile));
       }
     }
     profiles_.push_back(new_profile);
@@ -399,10 +444,10 @@ void AutofillProfileSyncableService::ActOnChange(
   }
   SyncError error = sync_processor_->ProcessSyncChanges(FROM_HERE, new_changes);
   if (error.IsSet()) {
-    LOG(WARNING) << "[AUTOFILL SYNC]"
-                 << " Failed processing change:"
-                 << " Error:" << error.message()
-                 << " Guid:" << change.key();
+    DLOG(WARNING) << "[AUTOFILL SYNC]"
+                  << " Failed processing change:"
+                  << " Error:" << error.message()
+                  << " Guid:" << change.key();
   }
 }
 
@@ -413,6 +458,48 @@ SyncData AutofillProfileSyncableService::CreateData(
   return SyncData::CreateLocalData(profile.guid(), profile.guid(), specifics);
 }
 
+bool AutofillProfileSyncableService::UpdateField(
+    AutofillFieldType field_type,
+    const std::string& new_value,
+    AutofillProfile* autofill_profile) {
+  if (UTF16ToUTF8(autofill_profile->GetInfo(field_type)) == new_value)
+    return false;
+  autofill_profile->SetInfo(field_type, UTF8ToUTF16(new_value));
+  return true;
+}
+
+bool AutofillProfileSyncableService::UpdateMultivaluedField(
+    AutofillFieldType field_type,
+    const ::google::protobuf::RepeatedPtrField<std::string>& new_values,
+    AutofillProfile* autofill_profile) {
+  std::vector<string16> values;
+  autofill_profile->GetMultiInfo(field_type, &values);
+  bool changed = false;
+  if (static_cast<size_t>(new_values.size()) != values.size()) {
+    values.clear();
+    values.resize(static_cast<size_t>(new_values.size()));
+    changed = true;
+  }
+  for (size_t i = 0; i < values.size(); ++i) {
+    string16 synced_value(
+        UTF8ToUTF16(new_values.Get(static_cast<int>(i))));
+    if (values[i] != synced_value) {
+      values[i] = synced_value;
+      changed = true;
+    }
+  }
+  if (changed)
+    autofill_profile->SetMultiInfo(field_type, values);
+  return changed;
+}
+
+bool AutofillProfileSyncableService::MergeProfile(
+    const AutofillProfile& merge_from,
+    AutofillProfile* merge_into) {
+  merge_into->OverwriteWithOrAddTo(merge_from);
+  return (merge_into->CompareMulti(merge_from) != 0);
+}
+
 AutofillTable* AutofillProfileSyncableService::GetAutofillTable() const {
   return web_data_service_->GetDatabase()->GetAutofillTable();
 }
diff --git a/chrome/browser/webdata/autofill_profile_syncable_service.h b/chrome/browser/webdata/autofill_profile_syncable_service.h
index bab8d78..b797f06 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service.h
+++ b/chrome/browser/webdata/autofill_profile_syncable_service.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 #ifndef CHROME_BROWSER_WEBDATA_AUTOFILL_PROFILE_SYNCABLE_SERVICE_H_
@@ -13,6 +13,7 @@
 #include "base/memory/scoped_vector.h"
 #include "base/synchronization/lock.h"
 #include "base/threading/non_thread_safe.h"
+#include "chrome/browser/autofill/autofill_type.h"
 #include "chrome/browser/sync/api/sync_change.h"
 #include "chrome/browser/sync/api/sync_data.h"
 #include "chrome/browser/sync/api/sync_error.h"
@@ -26,6 +27,7 @@
 
 class AutofillProfile;
 class AutofillTable;
+class FormGroup;
 class ProfileSyncServiceAutofillTest;
 class WebDataService;
 
@@ -88,6 +90,12 @@ class AutofillProfileSyncableService
                            ProcessSyncChanges);
   FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
                            ActOnChange);
+  FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
+                           UpdateField);
+  FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
+                           UpdateMultivaluedField);
+  FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
+                           MergeProfile);
 
   // The map of the guid to profiles owned by the |profiles_| vector.
   typedef std::map<std::string, AutofillProfile*> GUIDToProfileMap;
@@ -123,6 +131,23 @@ class AutofillProfileSyncableService
 
   AutofillTable* GetAutofillTable() const;
 
+  // Helper to compare the local value and cloud value of a field, copy into
+  // the local value if they differ, and return whether the change happened.
+  static bool UpdateField(AutofillFieldType field_type,
+                          const std::string& new_value,
+                          AutofillProfile* autofill_profile);
+  // The same as |UpdateField|, but for multi-valued fields.
+  static bool UpdateMultivaluedField(
+      AutofillFieldType field_type,
+      const ::google::protobuf::RepeatedPtrField<std::string>& new_value,
+      AutofillProfile* autofill_profile);
+
+  // Calls merge_into->OverwriteWithOrAddTo() and then checks if the
+  // |merge_into| has extra data. Returns |true| if |merge_into| posseses some
+  // multi-valued field values that are not in |merge_from|, false otherwise.
+  static bool MergeProfile(const AutofillProfile& merge_from,
+                           AutofillProfile* merge_into);
+
   // For unit-tests.
   AutofillProfileSyncableService();
   void set_sync_processor(SyncChangeProcessor* sync_processor) {
@@ -150,6 +175,12 @@ struct AutofillProfileSyncableService::DataBundle {
   std::vector<std::string> profiles_to_delete;
   std::vector<AutofillProfile*> profiles_to_update;
   std::vector<AutofillProfile*> profiles_to_add;
+
+  // When we go through sync we find profiles that are similar but unmatched.
+  // Merge such profiles.
+  GUIDToProfileMap candidates_to_merge;
+  // Profiles that have multi-valued fields that are not in sync.
+  std::vector<AutofillProfile*> profiles_to_sync_back;
 };
 
 #endif  // CHROME_BROWSER_WEBDATA_AUTOFILL_PROFILE_SYNCABLE_SERVICE_H_
diff --git a/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc b/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
index ef2f5d8..f839d09 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
+++ b/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -24,6 +24,12 @@ using ::testing::Property;
 using content::BrowserThread;
 class AutofillProfile;
 
+// Some guids for testing.
+std::string kGuid1 = "EDC609ED-7EEE-4F27-B00C-423242A9C44B";
+std::string kGuid2 = "EDC609ED-7EEE-4F27-B00C-423242A9C44C";
+std::string kGuid3 = "EDC609ED-7EEE-4F27-B00C-423242A9C44D";
+std::string kGuid4 = "EDC609ED-7EEE-4F27-B00C-423242A9C44E";
+
 class MockAutofillProfileSyncableService
     : public AutofillProfileSyncableService {
  public:
@@ -97,7 +103,8 @@ class MockSyncChangeProcessor : public SyncChangeProcessor {
 class AutofillProfileSyncableServiceTest : public testing::Test {
  public:
   AutofillProfileSyncableServiceTest()
-    : db_thread_(BrowserThread::DB, &message_loop_) {}
+    : ui_thread_(BrowserThread::UI, &message_loop_),
+      db_thread_(BrowserThread::DB, &message_loop_) {}
 
   virtual void SetUp() OVERRIDE {
     sync_processor_.reset(new MockSyncChangeProcessor);
@@ -113,6 +120,7 @@ class AutofillProfileSyncableServiceTest : public testing::Test {
   }
  protected:
   MessageLoop message_loop_;
+  content::TestBrowserThread ui_thread_;
   content::TestBrowserThread db_thread_;
   MockAutofillProfileSyncableService autofill_syncable_service_;
   scoped_ptr<MockSyncChangeProcessor> sync_processor_;
@@ -120,15 +128,19 @@ class AutofillProfileSyncableServiceTest : public testing::Test {
 
 TEST_F(AutofillProfileSyncableServiceTest, MergeDataAndStartSyncing) {
   std::vector<AutofillProfile *> profiles_from_web_db;
-  std::string guid_present1 = "EDC609ED-7EEE-4F27-B00C-423242A9C44B";
-  std::string guid_present2 = "EDC609ED-7EEE-4F27-B00C-423242A9C44C";
-  std::string guid_synced1 = "EDC609ED-7EEE-4F27-B00C-423242A9C44D";
-  std::string guid_synced2 = "EDC609ED-7EEE-4F27-B00C-423242A9C44E";
+  std::string guid_present1 = kGuid1;
+  std::string guid_present2 = kGuid2;
+  std::string guid_synced1 = kGuid3;
+  std::string guid_synced2 = kGuid4;
 
   profiles_from_web_db.push_back(new AutofillProfile(guid_present1));
   profiles_from_web_db.back()->SetInfo(NAME_FIRST, UTF8ToUTF16("John"));
+  profiles_from_web_db.back()->SetInfo(ADDRESS_HOME_LINE1,
+                                       UTF8ToUTF16("1 1st st"));
   profiles_from_web_db.push_back(new AutofillProfile(guid_present2));
   profiles_from_web_db.back()->SetInfo(NAME_FIRST, UTF8ToUTF16("Tom"));
+  profiles_from_web_db.back()->SetInfo(ADDRESS_HOME_LINE1,
+                                       UTF8ToUTF16("2 2nd st"));
 
   SyncDataList data_list;
   AutofillProfile profile1(guid_synced1);
@@ -175,8 +187,8 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeDataAndStartSyncing) {
 
 TEST_F(AutofillProfileSyncableServiceTest, GetAllSyncData) {
   std::vector<AutofillProfile *> profiles_from_web_db;
-  std::string guid_present1 = "EDC609ED-7EEE-4F27-B00C-423242A9C44B";
-  std::string guid_present2 = "EDC609ED-7EEE-4F27-B00C-423242A9C44C";
+  std::string guid_present1 = kGuid1;
+  std::string guid_present2 = kGuid2;
 
   profiles_from_web_db.push_back(new AutofillProfile(guid_present1));
   profiles_from_web_db.back()->SetInfo(NAME_FIRST, UTF8ToUTF16("John"));
@@ -213,8 +225,8 @@ TEST_F(AutofillProfileSyncableServiceTest, GetAllSyncData) {
 
 TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
   std::vector<AutofillProfile *> profiles_from_web_db;
-  std::string guid_present = "EDC609ED-7EEE-4F27-B00C-423242A9C44B";
-  std::string guid_synced = "EDC609ED-7EEE-4F27-B00C-423242A9C44C";
+  std::string guid_present = kGuid1;
+  std::string guid_synced = kGuid2;
 
   SyncChangeList change_list;
   AutofillProfile profile(guid_synced);
@@ -244,18 +256,164 @@ TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
 }
 
 TEST_F(AutofillProfileSyncableServiceTest, ActOnChange) {
-  std::string guid1 = "EDC609ED-7EEE-4F27-B00C-423242A9C44B";
-  std::string guid2 = "EDC609ED-7EEE-4F27-B00C-423242A9C44C";
-
-  AutofillProfile profile(guid1);
+  AutofillProfile profile(kGuid1);
   profile.SetInfo(NAME_FIRST, UTF8ToUTF16("Jane"));
-  AutofillProfileChange change1(AutofillProfileChange::ADD, guid1, &profile);
-  AutofillProfileChange change2(AutofillProfileChange::REMOVE, guid2, NULL);
+  AutofillProfileChange change1(AutofillProfileChange::ADD, kGuid1, &profile);
+  AutofillProfileChange change2(AutofillProfileChange::REMOVE, kGuid2, NULL);
   ON_CALL(*sync_processor_, ProcessSyncChanges(_, _))
       .WillByDefault(Return(SyncError(FROM_HERE, std::string("an error"),
                                       syncable::AUTOFILL_PROFILE)));
+  EXPECT_CALL(*sync_processor_, ProcessSyncChanges(_, _)).Times(2);
 
   autofill_syncable_service_.set_sync_processor(sync_processor_.get());
   autofill_syncable_service_.ActOnChange(change1);
   autofill_syncable_service_.ActOnChange(change2);
 }
+
+TEST_F(AutofillProfileSyncableServiceTest, UpdateField) {
+  AutofillProfile profile(kGuid1);
+  std::string company1 = "A Company";
+  std::string company2 = "Another Company";
+  profile.SetInfo(COMPANY_NAME, UTF8ToUTF16(company1));
+  EXPECT_FALSE(AutofillProfileSyncableService::UpdateField(
+      COMPANY_NAME, company1, &profile));
+  EXPECT_EQ(profile.GetInfo(COMPANY_NAME), UTF8ToUTF16(company1));
+  EXPECT_TRUE(AutofillProfileSyncableService::UpdateField(
+      COMPANY_NAME, company2, &profile));
+  EXPECT_EQ(profile.GetInfo(COMPANY_NAME), UTF8ToUTF16(company2));
+  EXPECT_FALSE(AutofillProfileSyncableService::UpdateField(
+      COMPANY_NAME, company2, &profile));
+  EXPECT_EQ(profile.GetInfo(COMPANY_NAME), UTF8ToUTF16(company2));
+}
+
+TEST_F(AutofillProfileSyncableServiceTest, UpdateMultivaluedField) {
+  AutofillProfile profile(kGuid1);
+
+  std::vector<string16> values;
+  values.push_back(UTF8ToUTF16("1@1.com"));
+  values.push_back(UTF8ToUTF16("2@1.com"));
+  profile.SetMultiInfo(EMAIL_ADDRESS, values);
+
+  ::google::protobuf::RepeatedPtrField<std::string> specifics_fields;
+  specifics_fields.AddAllocated(new std::string("2@1.com"));
+  specifics_fields.AddAllocated(new std::string("3@1.com"));
+
+  EXPECT_TRUE(AutofillProfileSyncableService::UpdateMultivaluedField(
+      EMAIL_ADDRESS, specifics_fields, &profile));
+  profile.GetMultiInfo(EMAIL_ADDRESS, &values);
+  ASSERT_TRUE(values.size() == 2);
+  EXPECT_EQ(values[0], UTF8ToUTF16("2@1.com"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("3@1.com"));
+
+  EXPECT_FALSE(AutofillProfileSyncableService::UpdateMultivaluedField(
+      EMAIL_ADDRESS, specifics_fields, &profile));
+  profile.GetMultiInfo(EMAIL_ADDRESS, &values);
+  ASSERT_EQ(values.size(), 2U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("2@1.com"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("3@1.com"));
+  EXPECT_TRUE(AutofillProfileSyncableService::UpdateMultivaluedField(
+      EMAIL_ADDRESS, ::google::protobuf::RepeatedPtrField<std::string>(),
+      &profile));
+  profile.GetMultiInfo(EMAIL_ADDRESS, &values);
+  ASSERT_EQ(values.size(), 1U);  // Always have at least an empty string.
+  EXPECT_EQ(values[0], UTF8ToUTF16(""));
+}
+
+TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
+  AutofillProfile profile1(kGuid1);
+  profile1.SetInfo(ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));
+
+  std::vector<string16> values;
+  values.push_back(UTF8ToUTF16("1@1.com"));
+  values.push_back(UTF8ToUTF16("2@1.com"));
+  profile1.SetMultiInfo(EMAIL_ADDRESS, values);
+
+  AutofillProfile profile2(kGuid2);
+  profile2.SetInfo(ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));
+
+  // |values| now is [ "1@1.com", "2@1.com", "3@1.com" ].
+  values.push_back(UTF8ToUTF16("3@1.com"));
+  profile2.SetMultiInfo(EMAIL_ADDRESS, values);
+
+  values.clear();
+  values.push_back(UTF8ToUTF16("John"));
+  profile1.SetMultiInfo(NAME_FIRST, values);
+  values.push_back(UTF8ToUTF16("Jane"));
+  profile2.SetMultiInfo(NAME_FIRST, values);
+
+  values.clear();
+  values.push_back(UTF8ToUTF16("Doe"));
+  profile1.SetMultiInfo(NAME_LAST, values);
+  values.push_back(UTF8ToUTF16("Other"));
+  profile2.SetMultiInfo(NAME_LAST, values);
+
+  values.clear();
+  values.push_back(UTF8ToUTF16("650234567"));
+  profile2.SetMultiInfo(PHONE_HOME_WHOLE_NUMBER, values);
+
+  EXPECT_FALSE(AutofillProfileSyncableService::MergeProfile(profile2,
+                                                            &profile1));
+
+  profile1.GetMultiInfo(NAME_FIRST, &values);
+  ASSERT_EQ(values.size(), 2U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("John"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("Jane"));
+
+  profile1.GetMultiInfo(NAME_LAST, &values);
+  ASSERT_EQ(values.size(), 2U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("Doe"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("Other"));
+
+  profile1.GetMultiInfo(EMAIL_ADDRESS, &values);
+  ASSERT_EQ(values.size(), 3U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("1@1.com"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("2@1.com"));
+  EXPECT_EQ(values[2], UTF8ToUTF16("3@1.com"));
+
+  profile1.GetMultiInfo(PHONE_HOME_WHOLE_NUMBER, &values);
+  ASSERT_EQ(values.size(), 1U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("650234567"));
+
+  AutofillProfile profile3(kGuid3);
+  profile3.SetInfo(ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));
+
+  values.clear();
+  values.push_back(UTF8ToUTF16("Jane"));
+  profile3.SetMultiInfo(NAME_FIRST, values);
+
+  values.clear();
+  values.push_back(UTF8ToUTF16("Doe"));
+  profile3.SetMultiInfo(NAME_LAST, values);
+
+  EXPECT_TRUE(AutofillProfileSyncableService::MergeProfile(profile3,
+                                                           &profile1));
+
+  profile1.GetMultiInfo(NAME_FIRST, &values);
+  ASSERT_EQ(values.size(), 3U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("John"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("Jane"));
+  EXPECT_EQ(values[2], UTF8ToUTF16("Jane"));
+
+  profile1.GetMultiInfo(NAME_LAST, &values);
+  ASSERT_EQ(values.size(), 3U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("Doe"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("Other"));
+  EXPECT_EQ(values[2], UTF8ToUTF16("Doe"));
+
+  // Middle name should have three entries as well.
+  profile1.GetMultiInfo(NAME_MIDDLE, &values);
+  ASSERT_EQ(values.size(), 3U);
+  EXPECT_TRUE(values[0].empty());
+  EXPECT_TRUE(values[1].empty());
+  EXPECT_TRUE(values[2].empty());
+
+  profile1.GetMultiInfo(EMAIL_ADDRESS, &values);
+  ASSERT_EQ(values.size(), 3U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("1@1.com"));
+  EXPECT_EQ(values[1], UTF8ToUTF16("2@1.com"));
+  EXPECT_EQ(values[2], UTF8ToUTF16("3@1.com"));
+
+  profile1.GetMultiInfo(PHONE_HOME_WHOLE_NUMBER, &values);
+  ASSERT_EQ(values.size(), 1U);
+  EXPECT_EQ(values[0], UTF8ToUTF16("650234567"));
+}