[Autofill] Set basic information when adding a new profiles and credit cards.

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

Index: components/autofill/core/browser/webdata/autofill_profile_syncable_service_unittest.cc
diff --git a/components/autofill/core/browser/webdata/autofill_profile_syncable_service_unittest.cc b/components/autofill/core/browser/webdata/autofill_profile_syncable_service_unittest.cc
index 352074e0eb00506a6cc5ac792656cb2f7eae656d..4a9d33e574488e6c61c1a84849b1e136ac9c659a 100644
--- a/components/autofill/core/browser/webdata/autofill_profile_syncable_service_unittest.cc
+++ b/components/autofill/core/browser/webdata/autofill_profile_syncable_service_unittest.cc
@@ -402,6 +402,8 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeSimilarProfiles) {
   // should never overwrite a verified one.
   AutofillProfile expected_profile(profile1);
   expected_profile.set_origin(origin_present1);
+  expected_profile.set_use_count(
+      2);  // Merging two profile add their user count.

[Mathieu, 2016/04/22 20:54:28]

nit: weird format. Let's move the comment to its own line

[sebsg, 2016/04/25 15:35:39]

Done.

   syncer::SyncChangeList expected_change_list;
   expected_change_list.push_back(
       syncer::SyncChange(FROM_HERE,
@@ -449,6 +451,8 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeDataEmptyOrigins) {
   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");
+  autofill_specifics->set_use_count(profile.use_count());
+  autofill_specifics->set_use_date(profile.use_date().ToTimeT());
   EXPECT_FALSE(autofill_specifics->has_origin());
 
   syncer::SyncDataList data_list;
@@ -595,13 +599,16 @@ TEST_F(AutofillProfileSyncableServiceTest, UpdateField) {
   EXPECT_EQ(profile.GetRawInfo(COMPANY_NAME), ASCIIToUTF16(company2));
 }
 

[Mathieu, 2016/04/22 20:54:28]

Mention what is tested here and possibly have a more descriptive test name,
perhaps MergeSimilarProfiles_AdditionalInfoInBothProfiles

[sebsg, 2016/04/25 15:35:39]

Done.

-TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
+TEST_F(AutofillProfileSyncableServiceTest, MergeSimilarProfile) {
   AutofillProfile profile1(kGuid1, kHttpOrigin);

[Mathieu, 2016/04/22 20:54:28]

rename to |from_profile| ?

[sebsg, 2016/04/25 15:35:39]

Isn't it the other way around?

   profile1.SetRawInfo(ADDRESS_HOME_LINE1, ASCIIToUTF16("111 First St."));
 
   AutofillProfile profile2(kGuid2, kHttpsOrigin);

[Mathieu, 2016/04/22 20:54:28]

rename to |into_profile|?

[sebsg, 2016/04/25 15:35:39]

Done.

   profile2.SetRawInfo(ADDRESS_HOME_LINE1, ASCIIToUTF16("111 First St."));
 
+  profile1.set_use_date(base::Time::FromTimeT(1234));
+  profile2.set_use_date(base::Time::FromTimeT(1234));
+
   profile1.SetRawInfo(EMAIL_ADDRESS, ASCIIToUTF16("1@1.com"));
   profile2.SetRawInfo(EMAIL_ADDRESS, ASCIIToUTF16("1@1.com"));
 
@@ -615,18 +622,18 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
 
   profile1.set_language_code("en");
 
-  EXPECT_FALSE(AutofillProfileSyncableService::MergeProfile(profile2, &profile1,
-                                                            "en-US"));
+  EXPECT_TRUE(AutofillProfileSyncableService::MergeSimilarProfile(

[Mathieu, 2016/04/22 20:54:28]

// We expect true because there is additional information in |from_profile| that
was transferred to |into_profile|.

[sebsg, 2016/04/25 15:35:39]

Done.

+      profile2, &profile1, "en-US"));
 

[Mathieu, 2016/04/22 20:54:28]

can you add some expectations for the content of |into_profile|?

[sebsg, 2016/04/25 15:35:39]

Done.

   // The more recent use_date is maintained and synced back.
   profile2.set_use_date(base::Time::FromTimeT(30));
   profile1.set_use_date(base::Time::FromTimeT(25));
-  EXPECT_FALSE(AutofillProfileSyncableService::MergeProfile(profile2, &profile1,
-                                                            "en-US"));
+  EXPECT_TRUE(AutofillProfileSyncableService::MergeSimilarProfile(

[Mathieu, 2016/04/22 20:54:28]

this should probably be in a different test because it assumes profile1 and
profile2 are in a certain state (if you leave it this way, how can you be sure
that it returns true here due to the different use dates?)

[sebsg, 2016/04/25 15:35:39]

Done.

+      profile2, &profile1, "en-US"));
   EXPECT_EQ(base::Time::FromTimeT(30), profile1.use_date());
   profile1.set_use_date(base::Time::FromTimeT(35));
-  EXPECT_TRUE(AutofillProfileSyncableService::MergeProfile(profile2, &profile1,
-                                                           "en-US"));
+  EXPECT_TRUE(AutofillProfileSyncableService::MergeSimilarProfile(
+      profile2, &profile1, "en-US"));
   EXPECT_EQ(base::Time::FromTimeT(35), profile1.use_date());
 
   EXPECT_EQ(ASCIIToUTF16("John"), profile1.GetRawInfo(NAME_FIRST));
@@ -642,9 +649,8 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeProfile) {
   profile3.SetRawInfo(NAME_FIRST, ASCIIToUTF16("Jane"));
   profile3.SetRawInfo(NAME_LAST, ASCIIToUTF16("Doe"));
 
-  EXPECT_TRUE(AutofillProfileSyncableService::MergeProfile(profile3,
-                                                           &profile1,
-                                                           "en-US"));
+  EXPECT_TRUE(AutofillProfileSyncableService::MergeSimilarProfile(
+      profile3, &profile1, "en-US"));
 
   EXPECT_EQ(ASCIIToUTF16("Jane"), profile1.GetRawInfo(NAME_FIRST));
   EXPECT_EQ(ASCIIToUTF16("Doe"), profile1.GetRawInfo(NAME_LAST));
@@ -780,6 +786,8 @@ TEST_F(AutofillProfileSyncableServiceTest, MergeDataEmptyStreetAddress) {
   autofill_specifics->add_phone_home_whole_number(std::string());
   autofill_specifics->set_address_home_line1("123 Example St.");
   autofill_specifics->set_address_home_line2("Apt. 42");
+  autofill_specifics->set_use_count(profile.use_count());
+  autofill_specifics->set_use_date(profile.use_date().ToTimeT());
   EXPECT_FALSE(autofill_specifics->has_address_home_street_address());
 
   syncer::SyncDataList data_list;
@@ -856,6 +864,8 @@ TEST_F(AutofillProfileSyncableServiceTest, NoLanguageCodeNoSync) {
   autofill_specifics->add_name_full(std::string());
   autofill_specifics->add_email_address(std::string());
   autofill_specifics->add_phone_home_whole_number(std::string());
+  autofill_specifics->set_use_count(profile.use_count());
+  autofill_specifics->set_use_date(profile.use_date().ToTimeT());
   EXPECT_FALSE(autofill_specifics->has_address_home_language_code());
 
   syncer::SyncDataList data_list;
@@ -1042,6 +1052,8 @@ TEST_F(AutofillProfileSyncableServiceTest, NoFullNameNoSync) {
   autofill_specifics->add_name_middle(std::string());
   autofill_specifics->add_name_last(std::string());
   autofill_specifics->add_email_address(std::string());
+  autofill_specifics->set_use_count(profile.use_count());
+  autofill_specifics->set_use_date(profile.use_date().ToTimeT());
   autofill_specifics->add_phone_home_whole_number(std::string());
 
   syncer::SyncDataList data_list;
@@ -1104,6 +1116,8 @@ TEST_F(AutofillProfileSyncableServiceTest, NoUsageStatsNoSync) {
   // Local autofill profile has 0 for use_count/use_date.
   AutofillProfile profile(kGuid1, kHttpsOrigin);
   profile.set_language_code("en");
+  profile.set_use_count(0);
+  profile.set_use_date(base::Time());
   EXPECT_EQ(0U, profile.use_count());
   EXPECT_EQ(base::Time(), profile.use_date());
   profiles_from_web_db.push_back(new AutofillProfile(profile));
@@ -1139,52 +1153,74 @@ TEST_F(AutofillProfileSyncableServiceTest, NoUsageStatsNoSync) {
 }
 
 // Usage stats should be updated by sync.
-TEST_F(AutofillProfileSyncableServiceTest, SyncUpdatesEmptyUsageStats) {
-  std::vector<AutofillProfile*> profiles_from_web_db;
-
-  // Local autofill profile has 0 for use_count/use_date.
-  AutofillProfile profile(kGuid1, kHttpsOrigin);
-  profile.set_language_code("en");
-  EXPECT_EQ(0U, profile.use_count());
-  EXPECT_EQ(base::Time(), profile.use_date());
-  profiles_from_web_db.push_back(new AutofillProfile(profile));
-
-  // Remote data has usage stats.
-  sync_pb::EntitySpecifics specifics;
-  sync_pb::AutofillProfileSpecifics* autofill_specifics =
-      specifics.mutable_autofill_profile();
-  autofill_specifics->set_guid(profile.guid());
-  autofill_specifics->set_origin(profile.origin());
-  autofill_specifics->add_name_first(std::string());
-  autofill_specifics->add_name_middle(std::string());
-  autofill_specifics->add_name_last(std::string());
-  autofill_specifics->add_name_full(std::string());
-  autofill_specifics->add_email_address(std::string());
-  autofill_specifics->add_phone_home_whole_number(std::string());
-  autofill_specifics->set_address_home_language_code("en");
-  autofill_specifics->set_use_count(9);
-  autofill_specifics->set_use_date(1423182153);
-  EXPECT_TRUE(autofill_specifics->has_use_count());
-  EXPECT_TRUE(autofill_specifics->has_use_date());
-
-  syncer::SyncDataList data_list;
-  data_list.push_back(
-      syncer::SyncData::CreateLocalData(
-          profile.guid(), profile.guid(), specifics));
-
-  // Expect the local autofill profile to have usage stats after sync.
-  MockAutofillProfileSyncableService::DataBundle expected_bundle;
-  AutofillProfile expected_profile = profile;
-  expected_profile.set_use_count(9U);
-  expected_profile.set_use_date(base::Time::FromTimeT(1423182153));
-  expected_bundle.profiles_to_update.push_back(&expected_profile);
-
-  // Expect no changes to remote data.
-  syncer::SyncChangeList expected_empty_change_list;
-
-  MergeDataAndStartSyncing(profiles_from_web_db, data_list,
-                           expected_bundle, expected_empty_change_list);
-  autofill_syncable_service_.StopSyncing(syncer::AUTOFILL_PROFILE);
+TEST_F(AutofillProfileSyncableServiceTest, SyncUpdatesUsageStats) {
+  typedef struct {
+    size_t local_use_count;
+    base::Time local_use_date;
+    size_t remote_use_count;
+    int remote_use_date;
+    size_t synced_use_count;
+    base::Time synced_use_date;
+  } TestCase;
+
+  TestCase test_cases[] = {
+      // Local profile with default stats.
+      {0U, base::Time(), 9U, 4321, 9U, base::Time::FromTimeT(4321)},
+      // Local profile has older stats than the server.
+      {3U, base::Time::FromTimeT(1234), 9U, 4321, 9U,
+       base::Time::FromTimeT(4321)},
+      // Local profile has newer stats than the server
+      {10U, base::Time::FromTimeT(9999), 9U, 4321, 9U,
+       base::Time::FromTimeT(4321)}};
+
+  for (const TestCase& test_case : test_cases) {
+    SetUp();
+    std::vector<AutofillProfile*> profiles_from_web_db;
+
+    AutofillProfile profile(kGuid1, kHttpsOrigin);
+    profile.set_language_code("en");
+    profile.set_use_count(test_case.local_use_count);
+    profile.set_use_date(test_case.local_use_date);
+    EXPECT_EQ(test_case.local_use_count, profile.use_count());
+    EXPECT_EQ(test_case.local_use_date, profile.use_date());
+    profiles_from_web_db.push_back(new AutofillProfile(profile));
+
+    // Remote data has usage stats.
+    sync_pb::EntitySpecifics specifics;
+    sync_pb::AutofillProfileSpecifics* autofill_specifics =
+        specifics.mutable_autofill_profile();
+    autofill_specifics->set_guid(profile.guid());
+    autofill_specifics->set_origin(profile.origin());
+    autofill_specifics->add_name_first(std::string());
+    autofill_specifics->add_name_middle(std::string());
+    autofill_specifics->add_name_last(std::string());
+    autofill_specifics->add_name_full(std::string());
+    autofill_specifics->add_email_address(std::string());
+    autofill_specifics->add_phone_home_whole_number(std::string());
+    autofill_specifics->set_address_home_language_code("en");
+    autofill_specifics->set_use_count(test_case.remote_use_count);
+    autofill_specifics->set_use_date(test_case.remote_use_date);
+    EXPECT_TRUE(autofill_specifics->has_use_count());
+    EXPECT_TRUE(autofill_specifics->has_use_date());
+
+    syncer::SyncDataList data_list;
+    data_list.push_back(syncer::SyncData::CreateLocalData(
+        profile.guid(), profile.guid(), specifics));
+
+    // Expect the local autofill profile to have usage stats after sync.
+    MockAutofillProfileSyncableService::DataBundle expected_bundle;
+    AutofillProfile expected_profile = profile;
+    expected_profile.set_use_count(test_case.synced_use_count);
+    expected_profile.set_use_date(test_case.synced_use_date);
+    expected_bundle.profiles_to_update.push_back(&expected_profile);
+
+    // Expect no changes to remote data.
+    syncer::SyncChangeList expected_empty_change_list;
+
+    MergeDataAndStartSyncing(profiles_from_web_db, data_list, expected_bundle,
+                             expected_empty_change_list);
+    autofill_syncable_service_.StopSyncing(syncer::AUTOFILL_PROFILE);
+  }
 }
 
 // Usage stats should be updated by the client.