[Autofill] Sync Autofill profiles' origins.

BUG=170401
TEST=unit tests
R=estade@chromium.org

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

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

5 files changed, 363 insertions, 123 deletions

diff --git a/chrome/browser/webdata/autofill_profile_syncable_service.cc b/chrome/browser/webdata/autofill_profile_syncable_service.cc
index b5fe95a..ba1a082 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service.cc
+++ b/chrome/browser/webdata/autofill_profile_syncable_service.cc
@@ -319,6 +319,15 @@ bool AutofillProfileSyncableService::OverwriteProfileWithServerData(
     AutofillProfile* profile,
     const std::string& app_locale) {
   bool diff = false;
+  if (profile->origin() != specifics.origin()) {
+    bool was_verified = profile->IsVerified();
+    profile->set_origin(specifics.origin());
+    diff = true;
+
+    // Verified profiles should never be overwritten by unverified ones.
+    DCHECK(!was_verified || profile->IsVerified());
+  }
+
   diff = UpdateMultivaluedField(autofill::NAME_FIRST,
                                 specifics.name_first(), profile) || diff;
   diff = UpdateMultivaluedField(autofill::NAME_MIDDLE,
@@ -368,6 +377,8 @@ void AutofillProfileSyncableService::WriteAutofillProfile(
   specifics->clear_phone_home_whole_number();
 
   specifics->set_guid(profile.guid());
+  specifics->set_origin(profile.origin());
+
   std::vector<string16> values;
   profile.GetRawMultiInfo(autofill::NAME_FIRST, &values);
   for (size_t i = 0; i < values.size(); ++i) {
@@ -435,49 +446,59 @@ AutofillProfileSyncableService::CreateOrUpdateProfile(
   const sync_pb::AutofillProfileSpecifics& autofill_specifics(
       specifics.autofill_profile());
 
-  GUIDToProfileMap::iterator it = profile_map->find(
+  GUIDToProfileMap::iterator existing_profile = profile_map->find(
         autofill_specifics.guid());
-  if (it != profile_map->end()) {
-    // Some profile that already present is synced.
+  if (existing_profile != profile_map->end()) {
+    // The synced profile already exists locally.  It might need to be updated.
     if (OverwriteProfileWithServerData(
-            autofill_specifics, it->second, app_locale_)) {
-      bundle->profiles_to_update.push_back(it->second);
+            autofill_specifics, existing_profile->second, app_locale_)) {
+      bundle->profiles_to_update.push_back(existing_profile->second);
     }
-  } else {
-    // New profile synced.
-    // TODO(isherman): Read the origin from |autofill_specifics|.
-    AutofillProfile* new_profile(
-        new AutofillProfile(autofill_specifics.guid(), std::string()));
-    OverwriteProfileWithServerData(
-        autofill_specifics, new_profile, app_locale_);
-
-    // 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) {
-        bundle->profiles_to_delete.push_back(i->second->guid());
-        DVLOG(2) << "[AUTOFILL SYNC]"
-                 << "Found in sync db but with a different guid: "
-                 << UTF16ToUTF8(i->second->GetRawInfo(autofill::NAME_FIRST))
-                 << UTF16ToUTF8(i->second->GetRawInfo(autofill::NAME_LAST))
-                 << "New guid " << new_profile->guid()
-                 << ". Profile to be deleted " << i->second->guid();
-        profile_map->erase(i);
-        break;
-      } else if (!i->second->PrimaryValue().empty() &&
-                 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));
+    return existing_profile;
+  }
+
+
+  // New profile synced.
+  AutofillProfile* new_profile = new AutofillProfile(
+      autofill_specifics.guid(), autofill_specifics.origin());
+  OverwriteProfileWithServerData(autofill_specifics, new_profile, app_locale_);
+
+  // Check if profile appears under a different guid.
+  // Unverified profiles should never overwrite verified ones.
+  for (GUIDToProfileMap::iterator it = profile_map->begin();
+       it != profile_map->end(); ++it) {
+    AutofillProfile* local_profile = it->second;
+    if (local_profile->Compare(*new_profile) == 0) {
+      // Ensure that a verified profile can never revert back to an unverified
+      // one.
+      if (local_profile->IsVerified() && !new_profile->IsVerified()) {
+        new_profile->set_origin(local_profile->origin());
+        bundle->profiles_to_sync_back.push_back(new_profile);
       }
+
+      bundle->profiles_to_delete.push_back(local_profile->guid());
+      DVLOG(2) << "[AUTOFILL SYNC]"
+               << "Found in sync db but with a different guid: "
+               << UTF16ToUTF8(local_profile->GetRawInfo(autofill::NAME_FIRST))
+               << UTF16ToUTF8(local_profile->GetRawInfo(autofill::NAME_LAST))
+               << "New guid " << new_profile->guid()
+               << ". Profile to be deleted " << local_profile->guid();
+      profile_map->erase(it);
+      break;
+    } else if (!local_profile->IsVerified() &&
+               !new_profile->IsVerified() &&
+               !local_profile->PrimaryValue().empty() &&
+               local_profile->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(local_profile->guid(), new_profile));
     }
-    profiles_.push_back(new_profile);
-    it = profile_map->insert(std::make_pair(new_profile->guid(),
-                                            new_profile)).first;
-    bundle->profiles_to_add.push_back(new_profile);
   }
-  return it;
+  profiles_.push_back(new_profile);
+  bundle->profiles_to_add.push_back(new_profile);
+  return profile_map->insert(std::make_pair(new_profile->guid(),
+                                            new_profile)).first;
 }
 
 void AutofillProfileSyncableService::ActOnChange(
@@ -582,7 +603,8 @@ bool AutofillProfileSyncableService::MergeProfile(
     AutofillProfile* merge_into,
     const std::string& app_locale) {
   merge_into->OverwriteWithOrAddTo(merge_from, app_locale);
-  return (merge_into->Compare(merge_from) != 0);
+  return (merge_into->Compare(merge_from) != 0 ||
+          merge_into->origin() != merge_from.origin());
 }
 
 AutofillTable* AutofillProfileSyncableService::GetAutofillTable() const {
diff --git a/chrome/browser/webdata/autofill_profile_syncable_service.h b/chrome/browser/webdata/autofill_profile_syncable_service.h
index e01d159..498d3aa 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service.h
+++ b/chrome/browser/webdata/autofill_profile_syncable_service.h
@@ -96,26 +96,28 @@ class AutofillProfileSyncableService
   struct DataBundle;
 
   // Helper to query WebDatabase for the current autofill state.
-  // Made virtual for ease of mocking in the unit-test.
+  // Made virtual for ease of mocking in unit tests.
   // Caller owns returned |profiles|.
   virtual bool LoadAutofillData(
       std::vector<autofill::AutofillProfile*>* profiles);
 
   // Helper to persist any changes that occured during model association to
   // the WebDatabase.
-  // Made virtual for ease of mocking in the unit-test.
+  // Made virtual for ease of mocking in unit tests.
   virtual bool SaveChangesToWebData(const DataBundle& bundle);
 
+  // For unit tests.
+  AutofillProfileSyncableService();
+  void set_sync_processor(syncer::SyncChangeProcessor* sync_processor) {
+    sync_processor_.reset(sync_processor);
+  }
+
+  // Creates syncer::SyncData based on supplied |profile|.
+  // Exposed for unit tests.
+  static syncer::SyncData CreateData(const autofill::AutofillProfile& profile);
+
  private:
   friend class ProfileSyncServiceAutofillTest;
-  friend class MockAutofillProfileSyncableService;
-  FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
-                           MergeDataAndStartSyncing);
-  FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest, GetAllSyncData);
-  FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
-                           ProcessSyncChanges);
-  FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
-                           ActOnChange);
   FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
                            UpdateField);
   FRIEND_TEST_ALL_PREFIXES(AutofillProfileSyncableServiceTest,
@@ -156,9 +158,6 @@ class AutofillProfileSyncableService
   // Syncs |change| to the cloud.
   void ActOnChange(const autofill::AutofillProfileChange& change);
 
-  // Creates syncer::SyncData based on supplied |profile|.
-  static syncer::SyncData CreateData(const autofill::AutofillProfile& profile);
-
   autofill::AutofillTable* GetAutofillTable() const;
 
   // Helper to compare the local value and cloud value of a field, copy into
@@ -174,17 +173,14 @@ class AutofillProfileSyncableService
 
   // 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.
+  // multi-valued field values that are not in |merge_from| or if the origins
+  // of the two profiles differ, false otherwise.
+  // TODO(isherman): Seems like this should return |true| if |merge_into| was
+  // modified at all: http://crbug.com/248440
   static bool MergeProfile(const autofill::AutofillProfile& merge_from,
                            autofill::AutofillProfile* merge_into,
                            const std::string& app_locale);
 
-  // For unit-tests.
-  AutofillProfileSyncableService();
-  void set_sync_processor(syncer::SyncChangeProcessor* sync_processor) {
-    sync_processor_.reset(sync_processor);
-  }
-
   autofill::AutofillWebDataBackend* webdata_backend_;  // WEAK
   std::string app_locale_;
   ScopedObserver<autofill::AutofillWebDataBackend,
@@ -204,7 +200,7 @@ class AutofillProfileSyncableService
   DISALLOW_COPY_AND_ASSIGN(AutofillProfileSyncableService);
 };
 
-// This object is used in unit-tests as well, so it defined here.
+// This object is used in unit tests as well, so it defined here.
 struct AutofillProfileSyncableService::DataBundle {
   DataBundle();
   ~DataBundle();
diff --git a/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc b/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
index fa55082..f5265a2 100644
--- a/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
+++ b/chrome/browser/webdata/autofill_profile_syncable_service_unittest.cc
@@ -36,10 +36,13 @@ const char kSettingsOrigin[] = "Chrome settings";
 class MockAutofillProfileSyncableService
     : public AutofillProfileSyncableService {
  public:
-  MockAutofillProfileSyncableService() {
-  }
+  MockAutofillProfileSyncableService() {}
   virtual ~MockAutofillProfileSyncableService() {}
 
+  using AutofillProfileSyncableService::DataBundle;
+  using AutofillProfileSyncableService::set_sync_processor;
+  using AutofillProfileSyncableService::CreateData;
+
   MOCK_METHOD1(LoadAutofillData, bool(std::vector<AutofillProfile*>*));
   MOCK_METHOD1(SaveChangesToWebData,
                bool(const AutofillProfileSyncableService::DataBundle&));
@@ -71,21 +74,18 @@ MATCHER_P(CheckSyncChanges, n_sync_changes_list, "") {
 MATCHER_P(DataBundleCheck, n_bundle, "") {
   if ((arg.profiles_to_delete.size() != n_bundle.profiles_to_delete.size()) ||
       (arg.profiles_to_update.size() != n_bundle.profiles_to_update.size()) ||
-      (arg.profiles_to_add.size() != n_bundle.profiles_to_add.size())) {
+      (arg.profiles_to_add.size() != n_bundle.profiles_to_add.size()))
     return false;
-  }
   for (size_t i = 0; i < arg.profiles_to_delete.size(); ++i) {
     if (arg.profiles_to_delete[i] != n_bundle.profiles_to_delete[i])
       return false;
   }
   for (size_t i = 0; i < arg.profiles_to_update.size(); ++i) {
-    if (arg.profiles_to_update[i]->guid() !=
-        n_bundle.profiles_to_update[i]->guid()) {
+    if (*arg.profiles_to_update[i] != *n_bundle.profiles_to_update[i])
       return false;
-    }
   }
   for (size_t i = 0; i < arg.profiles_to_add.size(); ++i) {
-    if (arg.profiles_to_add[i]->Compare(*n_bundle.profiles_to_add[i]))
+    if (*arg.profiles_to_add[i] != *n_bundle.profiles_to_add[i])
       return false;
   }
   return true;
@@ -98,7 +98,25 @@ class MockSyncChangeProcessor : public syncer::SyncChangeProcessor {
 
   MOCK_METHOD2(ProcessSyncChanges,
                syncer::SyncError(const tracked_objects::Location&,
-                         const syncer::SyncChangeList&));
+                                 const syncer::SyncChangeList&));
+};
+
+class TestSyncChangeProcessor : public syncer::SyncChangeProcessor {
+ public:
+  TestSyncChangeProcessor() {}
+  virtual ~TestSyncChangeProcessor() {}
+
+  virtual syncer::SyncError ProcessSyncChanges(
+      const tracked_objects::Location& location,
+      const syncer::SyncChangeList& changes) OVERRIDE {
+    changes_ = changes;
+    return syncer::SyncError();
+  }
+
+  const syncer::SyncChangeList& changes() { return changes_; }
+
+ private:
+  syncer::SyncChangeList changes_;
 };
 
 class AutofillProfileSyncableServiceTest : public testing::Test {
@@ -111,6 +129,35 @@ class AutofillProfileSyncableServiceTest : public testing::Test {
     sync_processor_.reset(new MockSyncChangeProcessor);
   }
 
+  // Wrapper around AutofillProfileSyncableService::MergeDataAndStartSyncing()
+  // that also verifies expectations.
+  void MergeDataAndStartSyncing(
+      const std::vector<AutofillProfile*>& profiles_from_web_db,
+      const syncer::SyncDataList& data_list,
+      const MockAutofillProfileSyncableService::DataBundle& expected_bundle,
+      const syncer::SyncChangeList& expected_change_list) {
+    EXPECT_CALL(autofill_syncable_service_, LoadAutofillData(_))
+        .Times(1)
+        .WillOnce(DoAll(CopyData(&profiles_from_web_db), Return(true)));
+    EXPECT_CALL(autofill_syncable_service_,
+                SaveChangesToWebData(DataBundleCheck(expected_bundle)))
+        .Times(1)
+        .WillOnce(Return(true));
+    ON_CALL(*sync_processor_, ProcessSyncChanges(_, _))
+        .WillByDefault(Return(syncer::SyncError()));
+    EXPECT_CALL(*sync_processor_,
+                ProcessSyncChanges(_, CheckSyncChanges(expected_change_list)))
+        .Times(1)
+        .WillOnce(Return(syncer::SyncError()));
+
+    // Takes ownership of sync_processor_.
+    autofill_syncable_service_.MergeDataAndStartSyncing(
+        syncer::AUTOFILL_PROFILE, data_list,
+        sync_processor_.PassAs<syncer::SyncChangeProcessor>(),
+        scoped_ptr<syncer::SyncErrorFactory>(
+            new syncer::SyncErrorFactoryMock()));
+  }
+
  protected:
   base::MessageLoop message_loop_;
   content::TestBrowserThread ui_thread_;
@@ -120,7 +167,7 @@ class AutofillProfileSyncableServiceTest : public testing::Test {
 };
 
 TEST_F(AutofillProfileSyncableServiceTest, MergeDataAndStartSyncing) {
-  std::vector<AutofillProfile *> profiles_from_web_db;
+  std::vector<AutofillProfile*> profiles_from_web_db;
   std::string guid_present1 = kGuid1;
   std::string guid_present2 = kGuid2;
   std::string guid_synced1 = kGuid3;
@@ -150,7 +197,7 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeDataAndStartSyncing) {
   AutofillProfile profile2(guid_synced2, origin_synced2);
   profile2.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Harry"));
   data_list.push_back(autofill_syncable_service_.CreateData(profile2));
-  // This one will have the name updated.
+  // This one will have the name and origin updated.
   AutofillProfile profile3(guid_present2, origin_synced2);
   profile3.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Tom Doe"));
   data_list.push_back(autofill_syncable_service_.CreateData(profile3));
@@ -159,14 +206,172 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeDataAndStartSyncing) {
   expected_change_list.push_back(
       syncer::SyncChange(FROM_HERE,
                          syncer::SyncChange::ACTION_ADD,
-                         AutofillProfileSyncableService::CreateData(
-                             (*profiles_from_web_db.front()))));
+                         MockAutofillProfileSyncableService::CreateData(
+                             *profiles_from_web_db.front())));
 
-  AutofillProfileSyncableService::DataBundle expected_bundle;
+  MockAutofillProfileSyncableService::DataBundle expected_bundle;
   expected_bundle.profiles_to_add.push_back(&profile1);
   expected_bundle.profiles_to_add.push_back(&profile2);
   expected_bundle.profiles_to_update.push_back(&profile3);
 
+  MergeDataAndStartSyncing(
+      profiles_from_web_db, data_list, expected_bundle, expected_change_list);
+  autofill_syncable_service_.StopSyncing(syncer::AUTOFILL_PROFILE);
+}
+
+TEST_F(AutofillProfileSyncableServiceTest, MergeIdenticalProfiles) {
+  std::vector<AutofillProfile*> profiles_from_web_db;
+  std::string guid_present1 = kGuid1;
+  std::string guid_present2 = kGuid2;
+  std::string guid_synced1 = kGuid3;
+  std::string guid_synced2 = kGuid4;
+  std::string origin_present1 = kHttpOrigin;
+  std::string origin_present2 = kSettingsOrigin;
+  std::string origin_synced1 = kHttpsOrigin;
+  std::string origin_synced2 = kHttpsOrigin;
+
+  profiles_from_web_db.push_back(
+      new AutofillProfile(guid_present1, origin_present1));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::NAME_FIRST, UTF8ToUTF16("John"));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("1 1st st"));
+  profiles_from_web_db.push_back(
+      new AutofillProfile(guid_present2, origin_present2));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::NAME_FIRST, UTF8ToUTF16("Tom"));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("2 2nd st"));
+
+  // The synced profiles are identical to the local ones, except that the guids
+  // are different.
+  syncer::SyncDataList data_list;
+  AutofillProfile profile1(guid_synced1, origin_synced1);
+  profile1.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("John"));
+  profile1.SetRawInfo(autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("1 1st st"));
+  data_list.push_back(autofill_syncable_service_.CreateData(profile1));
+  AutofillProfile profile2(guid_synced2, origin_synced2);
+  profile2.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Tom"));
+  profile2.SetRawInfo(autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("2 2nd st"));
+  data_list.push_back(autofill_syncable_service_.CreateData(profile2));
+
+  AutofillProfile expected_profile(profile2);
+  expected_profile.set_origin(kSettingsOrigin);
+  syncer::SyncChangeList expected_change_list;
+  expected_change_list.push_back(
+      syncer::SyncChange(FROM_HERE,
+                         syncer::SyncChange::ACTION_UPDATE,
+                         MockAutofillProfileSyncableService::CreateData(
+                             expected_profile)));
+
+  MockAutofillProfileSyncableService::DataBundle expected_bundle;
+  expected_bundle.profiles_to_delete.push_back(guid_present1);
+  expected_bundle.profiles_to_delete.push_back(guid_present2);
+  expected_bundle.profiles_to_add.push_back(&profile1);
+  expected_bundle.profiles_to_add.push_back(&expected_profile);
+
+  MergeDataAndStartSyncing(
+      profiles_from_web_db, data_list, expected_bundle, expected_change_list);
+  autofill_syncable_service_.StopSyncing(syncer::AUTOFILL_PROFILE);
+}
+
+TEST_F(AutofillProfileSyncableServiceTest, MergeSimilarProfiles) {
+  std::vector<AutofillProfile*> profiles_from_web_db;
+  std::string guid_present1 = kGuid1;
+  std::string guid_present2 = kGuid2;
+  std::string guid_synced1 = kGuid3;
+  std::string guid_synced2 = kGuid4;
+  std::string origin_present1 = kHttpOrigin;
+  std::string origin_present2 = kSettingsOrigin;
+  std::string origin_synced1 = kHttpsOrigin;
+  std::string origin_synced2 = kHttpsOrigin;
+
+  profiles_from_web_db.push_back(
+      new AutofillProfile(guid_present1, origin_present1));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::NAME_FIRST, UTF8ToUTF16("John"));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("1 1st st"));
+  profiles_from_web_db.push_back(
+      new AutofillProfile(guid_present2, origin_present2));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::NAME_FIRST, UTF8ToUTF16("Tom"));
+  profiles_from_web_db.back()->SetRawInfo(
+      autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("2 2nd st"));
+
+  // The synced profiles are identical to the local ones, except that the guids
+  // are different.
+  syncer::SyncDataList data_list;
+  AutofillProfile profile1(guid_synced1, origin_synced1);
+  profile1.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("John"));
+  profile1.SetRawInfo(autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("1 1st st"));
+  profile1.SetRawInfo(autofill::COMPANY_NAME, UTF8ToUTF16("Frobbers, Inc."));
+  data_list.push_back(autofill_syncable_service_.CreateData(profile1));
+  AutofillProfile profile2(guid_synced2, origin_synced2);
+  profile2.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Tom"));
+  profile2.SetRawInfo(autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("2 2nd st"));
+  profile2.SetRawInfo(autofill::COMPANY_NAME, UTF8ToUTF16("Fizzbang, LLC."));
+  data_list.push_back(autofill_syncable_service_.CreateData(profile2));
+
+  // The first profile should have its origin updated.
+  // The second profile should remain as-is, because an unverified profile
+  // should never overwrite a verified one.
+  AutofillProfile expected_profile(profile1);
+  expected_profile.set_origin(origin_present1);
+  syncer::SyncChangeList expected_change_list;
+  expected_change_list.push_back(
+      syncer::SyncChange(FROM_HERE,
+                         syncer::SyncChange::ACTION_ADD,
+                         MockAutofillProfileSyncableService::CreateData(
+                             *profiles_from_web_db.back())));
+  expected_change_list.push_back(
+      syncer::SyncChange(FROM_HERE,
+                         syncer::SyncChange::ACTION_UPDATE,
+                         MockAutofillProfileSyncableService::CreateData(
+                             expected_profile)));
+
+  MockAutofillProfileSyncableService::DataBundle expected_bundle;
+  expected_bundle.profiles_to_delete.push_back(guid_present1);
+  expected_bundle.profiles_to_add.push_back(&expected_profile);
+  expected_bundle.profiles_to_add.push_back(&profile2);
+
+  MergeDataAndStartSyncing(
+      profiles_from_web_db, data_list, expected_bundle, expected_change_list);
+  autofill_syncable_service_.StopSyncing(syncer::AUTOFILL_PROFILE);
+}
+
+// Ensure that no Sync events are generated to fill in missing origins from Sync
+// with explicitly present empty ones.  This ensures that the migration to add
+// origins to profiles does not generate lots of needless Sync updates.
+TEST_F(AutofillProfileSyncableServiceTest, MergeDataEmptyOrigins) {
+  std::vector<AutofillProfile*> profiles_from_web_db;
+
+  // Create a profile with an empty origin.
+  AutofillProfile profile(kGuid1, std::string());
+  profile.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("John"));
+  profile.SetRawInfo(autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("1 1st st"));
+
+  profiles_from_web_db.push_back(new AutofillProfile(profile));
+
+  // Create a Sync profile identical to |profile|, except with no origin set.
+  sync_pb::EntitySpecifics specifics;
+  sync_pb::AutofillProfileSpecifics* autofill_specifics =
+      specifics.mutable_autofill_profile();
+  autofill_specifics->set_guid(profile.guid());
+  autofill_specifics->add_name_first("John");
+  autofill_specifics->add_name_middle(std::string());
+  autofill_specifics->add_name_last(std::string());
+  autofill_specifics->add_email_address(std::string());
+  autofill_specifics->add_phone_home_whole_number(std::string());
+  autofill_specifics->set_address_home_line1("1 1st st");
+  EXPECT_FALSE(autofill_specifics->has_origin());
+
+  syncer::SyncDataList data_list;
+  data_list.push_back(
+      syncer::SyncData::CreateLocalData(
+          profile.guid(), profile.guid(), specifics));
+
+  MockAutofillProfileSyncableService::DataBundle expected_bundle;
   EXPECT_CALL(autofill_syncable_service_, LoadAutofillData(_))
       .Times(1)
       .WillOnce(DoAll(CopyData(&profiles_from_web_db), Return(true)));
@@ -174,23 +379,19 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeDataAndStartSyncing) {
               SaveChangesToWebData(DataBundleCheck(expected_bundle)))
       .Times(1)
       .WillOnce(Return(true));
-  ON_CALL(*sync_processor_, ProcessSyncChanges(_, _))
-      .WillByDefault(Return(syncer::SyncError()));
-  EXPECT_CALL(*sync_processor_,
-              ProcessSyncChanges(_, CheckSyncChanges(expected_change_list)))
-      .Times(1)
-      .WillOnce(Return(syncer::SyncError()));
+  EXPECT_CALL(*sync_processor_, ProcessSyncChanges(_, _)).Times(0);
 
   // Takes ownership of sync_processor_.
   autofill_syncable_service_.MergeDataAndStartSyncing(
       syncer::AUTOFILL_PROFILE, data_list,
       sync_processor_.PassAs<syncer::SyncChangeProcessor>(),
       scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+
   autofill_syncable_service_.StopSyncing(syncer::AUTOFILL_PROFILE);
 }
 
 TEST_F(AutofillProfileSyncableServiceTest, GetAllSyncData) {
-  std::vector<AutofillProfile *> profiles_from_web_db;
+  std::vector<AutofillProfile*> profiles_from_web_db;
   std::string guid_present1 = kGuid1;
   std::string guid_present2 = kGuid2;
 
@@ -203,27 +404,22 @@ TEST_F(AutofillProfileSyncableServiceTest, GetAllSyncData) {
   profiles_from_web_db.back()->SetRawInfo(
       autofill::NAME_FIRST, UTF8ToUTF16("Jane"));
 
-  EXPECT_CALL(autofill_syncable_service_, LoadAutofillData(_))
-      .Times(1)
-      .WillOnce(DoAll(CopyData(&profiles_from_web_db), Return(true)));
-  EXPECT_CALL(autofill_syncable_service_, SaveChangesToWebData(_))
-      .Times(1)
-      .WillOnce(Return(true));
-  ON_CALL(*sync_processor_, ProcessSyncChanges(_, _))
-      .WillByDefault(Return(syncer::SyncError()));
-  EXPECT_CALL(*sync_processor_,
-              ProcessSyncChanges(
-                  _,
-                  Property(&syncer::SyncChangeList::size, Eq(2U))))
-      .Times(1)
-      .WillOnce(Return(syncer::SyncError()));
+  syncer::SyncChangeList expected_change_list;
+  expected_change_list.push_back(
+      syncer::SyncChange(FROM_HERE,
+                         syncer::SyncChange::ACTION_ADD,
+                         MockAutofillProfileSyncableService::CreateData(
+                             *profiles_from_web_db.front())));
+  expected_change_list.push_back(
+      syncer::SyncChange(FROM_HERE,
+                         syncer::SyncChange::ACTION_ADD,
+                         MockAutofillProfileSyncableService::CreateData(
+                             *profiles_from_web_db.back())));
 
+  MockAutofillProfileSyncableService::DataBundle expected_bundle;
   syncer::SyncDataList data_list;
-  // Takes ownership of sync_processor_.
-  autofill_syncable_service_.MergeDataAndStartSyncing(
-      syncer::AUTOFILL_PROFILE, data_list,
-      sync_processor_.PassAs<syncer::SyncChangeProcessor>(),
-      scoped_ptr<syncer::SyncErrorFactory>(new syncer::SyncErrorFactoryMock()));
+  MergeDataAndStartSyncing(
+      profiles_from_web_db, data_list, expected_bundle, expected_change_list);
 
   syncer::SyncDataList data =
       autofill_syncable_service_.GetAllSyncData(syncer::AUTOFILL_PROFILE);
@@ -231,12 +427,10 @@ TEST_F(AutofillProfileSyncableServiceTest, GetAllSyncData) {
   ASSERT_EQ(2U, data.size());
   EXPECT_EQ(guid_present1, data[0].GetSpecifics().autofill_profile().guid());
   EXPECT_EQ(guid_present2, data[1].GetSpecifics().autofill_profile().guid());
-  // TODO(isherman): Verify that the origins match once they are saved and read
-  // from the database and included in the Sync protocol buffers.
-  // http://crbug.com/170401
-  // EXPECT_EQ(kHttpOrigin, data[0].GetSpecifics().autofill_profile().origin());
-  // EXPECT_EQ(kHttpsOrigin,
-  //           data[1].GetSpecifics().autofill_profile().origin());
+  EXPECT_EQ(kHttpOrigin, data[0].GetSpecifics().autofill_profile().origin());
+  EXPECT_EQ(kHttpsOrigin, data[1].GetSpecifics().autofill_profile().origin());
+
+  autofill_syncable_service_.StopSyncing(syncer::AUTOFILL_PROFILE);
 }
 
 TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
@@ -248,17 +442,18 @@ TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
   AutofillProfile profile(guid_synced, kHttpOrigin);
   profile.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Jane"));
   change_list.push_back(
-      syncer::SyncChange(FROM_HERE,
-                         syncer::SyncChange::ACTION_ADD,
-                         AutofillProfileSyncableService::CreateData(profile)));
+      syncer::SyncChange(
+          FROM_HERE,
+          syncer::SyncChange::ACTION_ADD,
+          MockAutofillProfileSyncableService::CreateData(profile)));
   AutofillProfile empty_profile(guid_present, kHttpsOrigin);
   change_list.push_back(
       syncer::SyncChange(
           FROM_HERE,
           syncer::SyncChange::ACTION_DELETE,
-          AutofillProfileSyncableService::CreateData(empty_profile)));
+          MockAutofillProfileSyncableService::CreateData(empty_profile)));
 
-  AutofillProfileSyncableService::DataBundle expected_bundle;
+  MockAutofillProfileSyncableService::DataBundle expected_bundle;
   expected_bundle.profiles_to_delete.push_back(guid_present);
   expected_bundle.profiles_to_add.push_back(&profile);
 
@@ -274,20 +469,43 @@ TEST_F(AutofillProfileSyncableServiceTest, ProcessSyncChanges) {
   EXPECT_FALSE(error.IsSet());
 }
 
-TEST_F(AutofillProfileSyncableServiceTest, ActOnChange) {
-  AutofillProfile profile(kGuid1, std::string());
+TEST_F(AutofillProfileSyncableServiceTest, AutofillProfileAdded) {
+  // Will be owned by the syncable service.  Keep a reference available here for
+  // verifying test expectations.
+  TestSyncChangeProcessor* sync_change_processor = new TestSyncChangeProcessor;
+  autofill_syncable_service_.set_sync_processor(sync_change_processor);
+
+  AutofillProfile profile(kGuid1, kHttpsOrigin);
   profile.SetRawInfo(autofill::NAME_FIRST, UTF8ToUTF16("Jane"));
-  AutofillProfileChange change1(AutofillProfileChange::ADD, kGuid1, &profile);
-  AutofillProfileChange change2(AutofillProfileChange::REMOVE, kGuid2, NULL);
-  ON_CALL(*sync_processor_, ProcessSyncChanges(_, _))
-      .WillByDefault(
-          Return(syncer::SyncError(FROM_HERE, std::string("an error"),
-                                   syncer::AUTOFILL_PROFILE)));
-  EXPECT_CALL(*sync_processor_, ProcessSyncChanges(_, _)).Times(2);
+  AutofillProfileChange change(AutofillProfileChange::ADD, kGuid1, &profile);
+  autofill_syncable_service_.AutofillProfileChanged(change);
+
+  ASSERT_EQ(1U, sync_change_processor->changes().size());
+  syncer::SyncChange result = sync_change_processor->changes()[0];
+  EXPECT_EQ(syncer::SyncChange::ACTION_ADD, result.change_type());
+
+  sync_pb::AutofillProfileSpecifics specifics =
+      result.sync_data().GetSpecifics().autofill_profile();
+  EXPECT_EQ(kGuid1, specifics.guid());
+  EXPECT_EQ(kHttpsOrigin, specifics.origin());
+  EXPECT_THAT(specifics.name_first(), testing::ElementsAre("Jane"));
+}
 
-  autofill_syncable_service_.set_sync_processor(sync_processor_.release());
-  autofill_syncable_service_.ActOnChange(change1);
-  autofill_syncable_service_.ActOnChange(change2);
+TEST_F(AutofillProfileSyncableServiceTest, AutofillProfileDeleted) {
+  // Will be owned by the syncable service.  Keep a reference available here for
+  // verifying test expectations.
+  TestSyncChangeProcessor* sync_change_processor = new TestSyncChangeProcessor;
+  autofill_syncable_service_.set_sync_processor(sync_change_processor);
+
+  AutofillProfileChange change(AutofillProfileChange::REMOVE, kGuid2, NULL);
+  autofill_syncable_service_.AutofillProfileChanged(change);
+
+  ASSERT_EQ(1U, sync_change_processor->changes().size());
+  syncer::SyncChange result = sync_change_processor->changes()[0];
+  EXPECT_EQ(syncer::SyncChange::ACTION_DELETE, result.change_type());
+  sync_pb::AutofillProfileSpecifics specifics =
+      result.sync_data().GetSpecifics().autofill_profile();
+  EXPECT_EQ(kGuid2, specifics.guid());
 }
 
 TEST_F(AutofillProfileSyncableServiceTest, UpdateField) {
@@ -350,7 +568,7 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
   values.push_back(UTF8ToUTF16("2@1.com"));
   profile1.SetRawMultiInfo(autofill::EMAIL_ADDRESS, values);
 
-  AutofillProfile profile2(kGuid2, std::string());
+  AutofillProfile profile2(kGuid2, kHttpsOrigin);
   profile2.SetRawInfo(
       autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));
 
@@ -398,6 +616,8 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
   ASSERT_EQ(values.size(), 1U);
   EXPECT_EQ(values[0], UTF8ToUTF16("650234567"));
 
+  EXPECT_EQ(profile2.origin(), profile1.origin());
+
   AutofillProfile profile3(kGuid3, kHttpOrigin);
   profile3.SetRawInfo(
       autofill::ADDRESS_HOME_LINE1, UTF8ToUTF16("111 First St."));
diff --git a/sync/protocol/autofill_specifics.proto b/sync/protocol/autofill_specifics.proto
index 4ca1102..f816e90 100644
--- a/sync/protocol/autofill_specifics.proto
+++ b/sync/protocol/autofill_specifics.proto
@@ -19,6 +19,7 @@ package sync_pb;
 // An AutofillProfile.
 message AutofillProfileSpecifics {
   optional string guid = 15;
+  optional string origin = 16;
 
   // Contact info.
   repeated string name_first = 2;
diff --git a/sync/protocol/proto_value_conversions.cc b/sync/protocol/proto_value_conversions.cc
index 1b09864..f0a2011 100644
--- a/sync/protocol/proto_value_conversions.cc
+++ b/sync/protocol/proto_value_conversions.cc
@@ -286,6 +286,7 @@ base::DictionaryValue* AutofillProfileSpecificsToValue(
     const sync_pb::AutofillProfileSpecifics& proto) {
   base::DictionaryValue* value = new base::DictionaryValue();
   SET_STR(guid);
+  SET_STR(origin);
 
   SET_STR_REP(name_first);
   SET_STR_REP(name_middle);