Serialize date_created in PasswordStoreX properly

chrome/browser/password_manager/native_backend_kwallet_x_unittest.cc

 // 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.
 
 #include <algorithm>
 #include <map>
 #include <set>
 #include <string>
 #include <vector>
 
@@ skipping 139 matching lines @@
  protected:
   NativeBackendKWalletTestBase() {
     old_form_google_.origin = GURL("http://www.google.com/");
     old_form_google_.action = GURL("http://www.google.com/login");
     old_form_google_.username_element = UTF8ToUTF16("user");
     old_form_google_.username_value = UTF8ToUTF16("joeschmoe");
     old_form_google_.password_element = UTF8ToUTF16("pass");
     old_form_google_.password_value = UTF8ToUTF16("seekrit");
     old_form_google_.submit_element = UTF8ToUTF16("submit");
     old_form_google_.signon_realm = "Google";
+    old_form_google_.date_created = base::Time::Now();
 
     form_google_ = old_form_google_;
     form_google_.times_used = 3;
     form_google_.type = PasswordForm::TYPE_GENERATED;
     form_google_.form_data.name = UTF8ToUTF16("form_name");
     form_google_.form_data.user_submitted = true;
     form_google_.date_synced = base::Time::Now();
+    form_google_.date_created = old_form_google_.date_created;
     form_google_.display_name = UTF8ToUTF16("Joe Schmoe");
     form_google_.avatar_url = GURL("http://www.google.com/avatar");
     form_google_.federation_url = GURL("http://www.google.com/federation_url");
     form_google_.is_zero_click = true;
 
     form_isc_.origin = GURL("http://www.isc.org/");
     form_isc_.action = GURL("http://www.isc.org/auth");
     form_isc_.username_element = UTF8ToUTF16("id");
     form_isc_.username_value = UTF8ToUTF16("janedoe");
     form_isc_.password_element = UTF8ToUTF16("passwd");
     form_isc_.password_value = UTF8ToUTF16("ihazabukkit");
     form_isc_.submit_element = UTF8ToUTF16("login");
     form_isc_.signon_realm = "ISC";
     form_isc_.date_synced = base::Time::Now();
+    form_isc_.date_created = base::Time::Now();
   }
 
   static void CheckPasswordForm(const PasswordForm& expected,
-                                const PasswordForm& actual);
+                                const PasswordForm& actual,
+                                int version);
   static void CheckPasswordChanges(const PasswordStoreChangeList& expected,
                                    const PasswordStoreChangeList& actual);
   static void CheckPasswordChangesWithResult(
       const PasswordStoreChangeList* expected,
       const PasswordStoreChangeList* actual,
       bool result);
 
   PasswordForm old_form_google_;
   PasswordForm form_google_;
   PasswordForm form_isc_;
 };
 
 // static
 void NativeBackendKWalletTestBase::CheckPasswordForm(
-    const PasswordForm& expected, const PasswordForm& actual) {
+    const PasswordForm& expected,
+    const PasswordForm& actual,
+    int version) {

[vabr (Chromium), 2015/02/03 16:06:42]

I suggest adding a flag saying whether the date is in internal format or not,
rather than the pickle version. On places like line 615 the version argument
will stop begin the newest version ultimately, and it will be unclear whether it
needs to be bumped or not.

[dvadym, 2015/02/04 10:40:25]

Done.

   EXPECT_EQ(expected.origin, actual.origin);
   EXPECT_EQ(expected.password_value, actual.password_value);
   EXPECT_EQ(expected.action, actual.action);
   EXPECT_EQ(expected.username_element, actual.username_element);
   EXPECT_EQ(expected.username_value, actual.username_value);
   EXPECT_EQ(expected.password_element, actual.password_element);
   EXPECT_EQ(expected.submit_element, actual.submit_element);
   EXPECT_EQ(expected.signon_realm, actual.signon_realm);
   EXPECT_EQ(expected.ssl_valid, actual.ssl_valid);
   EXPECT_EQ(expected.preferred, actual.preferred);
+  if (version > 4) {
+    EXPECT_EQ(expected.date_created, actual.date_created);
+  } else {
   // We don't check the date created. It varies.
+  }
   EXPECT_EQ(expected.blacklisted_by_user, actual.blacklisted_by_user);
   EXPECT_EQ(expected.type, actual.type);
   EXPECT_EQ(expected.times_used, actual.times_used);
   EXPECT_EQ(expected.scheme, actual.scheme);
   EXPECT_EQ(expected.date_synced, actual.date_synced);
   EXPECT_EQ(expected.display_name, actual.display_name);
   EXPECT_EQ(expected.avatar_url, actual.avatar_url);
   EXPECT_EQ(expected.federation_url, actual.federation_url);
   EXPECT_EQ(expected.is_zero_click, actual.is_zero_click);
 }
 
 // static
 void NativeBackendKWalletTestBase::CheckPasswordChanges(
     const PasswordStoreChangeList& expected,
     const PasswordStoreChangeList& actual) {
   ASSERT_EQ(expected.size(), actual.size());
   for (size_t i = 0; i < expected.size(); ++i) {
     EXPECT_EQ(expected[i].type(), actual[i].type());
-    CheckPasswordForm(expected[i].form(), actual[i].form());
+    CheckPasswordForm(expected[i].form(), actual[i].form(), 5);
   }
 }
 
 // static
 void NativeBackendKWalletTestBase::CheckPasswordChangesWithResult(
     const PasswordStoreChangeList* expected,
     const PasswordStoreChangeList* actual,
     bool result) {
   EXPECT_TRUE(result);
   CheckPasswordChanges(*expected, *actual);
@@ skipping 118 matching lines @@
   NativeBackendKWalletStub backend(42);
   EXPECT_TRUE(backend.InitWithBus(mock_session_bus_));
 
   form_google_.date_synced = base::Time();
   form_isc_.date_synced = base::Time();
   form_google_.date_created = base::Time();
   form_isc_.date_created = base::Time();
   base::Time now = base::Time::Now();
   base::Time next_day = now + base::TimeDelta::FromDays(1);
   if (date_to_test == CREATED) {
-    // crbug/374132. Remove the next line once it's fixed.
-    next_day = base::Time::FromTimeT(next_day.ToTimeT());
     form_google_.date_created = now;
     form_isc_.date_created = next_day;
   } else {
     form_google_.date_synced = now;
     form_isc_.date_synced = next_day;
   }
 
   BrowserThread::PostTask(
       BrowserThread::DB,
       FROM_HERE,
@@ skipping 217 matching lines @@
   for (size_t i = 0; i < entries.size() && i < sorted_expected.size(); ++i) {
     EXPECT_EQ(sorted_expected[i].first, entries[i]);
     TestKWallet::Blob value;
     EXPECT_TRUE(wallet_.readEntry(folder, entries[i], &value));
     Pickle pickle(reinterpret_cast<const char*>(value.data()), value.size());
     ScopedVector<autofill::PasswordForm> forms;
     NativeBackendKWalletStub::DeserializeValue(entries[i], pickle, &forms);
     const std::vector<const PasswordForm*>& expect = sorted_expected[i].second;
     EXPECT_EQ(expect.size(), forms.size());
     for (size_t j = 0; j < forms.size() && j < expect.size(); ++j)
-      CheckPasswordForm(*expect[j], *forms[j]);
+      CheckPasswordForm(*expect[j], *forms[j], 5);
   }
 }
 
 TEST_F(NativeBackendKWalletTest, NotEnabled) {
   NativeBackendKWalletStub kwallet(42);
   kwallet_enabled_ = false;
   EXPECT_FALSE(kwallet.InitWithBus(mock_session_bus_));
   EXPECT_FALSE(klauncher_contacted_);
 }
 
@@ skipping 298 matching lines @@
 
 TEST_F(NativeBackendKWalletTest, RemoveLoginsSyncedBetween) {
   TestRemoveLoginsBetween(SYNCED);
 }
 
 // TODO(mdm): add more basic tests here at some point.
 // (For example tests for storing >1 password per realm pickle.)
 
 class NativeBackendKWalletPickleTest : public NativeBackendKWalletTestBase {
  protected:
+  void CreateVersion5Pickle(const PasswordForm& form, Pickle* pickle);
   void CreateVersion3Pickle(const PasswordForm& form, Pickle* pickle);
   void CreateVersion2Pickle(const PasswordForm& form, Pickle* pickle);
   void CreateVersion1Pickle(const PasswordForm& form, Pickle* pickle);
   void CreateVersion0Pickle(bool size_32,
                             const PasswordForm& form,
                             Pickle* pickle);
+  void CheckVersion5Pickle();
   void CheckVersion3Pickle();
   void CheckVersion2Pickle();
   void CheckVersion1Pickle();
   void CheckVersion0Pickle(bool size_32, PasswordForm::Scheme scheme);
 
  private:
-  void CreatePickle(bool size_32, const PasswordForm& form, Pickle* pickle);
+  void CreatePickle(bool size_32,
+                    bool date_created_internal,

[vabr (Chromium), 2015/02/03 16:06:42]

Please add a comment explaining the arguments, in particular
date_created_internal

[dvadym, 2015/02/04 10:40:25]

Done.

+                    const PasswordForm& form,
+                    Pickle* pickle);
 };
 
+void NativeBackendKWalletPickleTest::CreateVersion5Pickle(
+    const PasswordForm& form,
+    Pickle* pickle) {
+  pickle->WriteInt(5);
+  CreatePickle(false, true, form, pickle);
+  pickle->WriteInt(form.type);
+  pickle->WriteInt(form.times_used);
+  autofill::SerializeFormData(form.form_data, pickle);
+  pickle->WriteInt64(form.date_synced.ToInternalValue());
+}
+
 void NativeBackendKWalletPickleTest::CreateVersion3Pickle(
     const PasswordForm& form, Pickle* pickle) {
   pickle->WriteInt(3);
-  CreatePickle(false, form, pickle);
+  CreatePickle(false, false, form, pickle);
   pickle->WriteInt(form.type);
   pickle->WriteInt(form.times_used);
   autofill::SerializeFormData(form.form_data, pickle);
   pickle->WriteInt64(form.date_synced.ToInternalValue());
 }
 
 void NativeBackendKWalletPickleTest::CreateVersion2Pickle(
     const PasswordForm& form, Pickle* pickle) {
   pickle->WriteInt(2);
-  CreatePickle(false, form, pickle);
+  CreatePickle(false, false, form, pickle);
   pickle->WriteInt(form.type);
   pickle->WriteInt(form.times_used);
   autofill::SerializeFormData(form.form_data, pickle);
 }
 
 void NativeBackendKWalletPickleTest::CreateVersion1Pickle(
     const PasswordForm& form, Pickle* pickle) {
   pickle->WriteInt(1);
-  CreatePickle(false, form, pickle);
+  CreatePickle(false, false, form, pickle);
 }
 
 void NativeBackendKWalletPickleTest::CreateVersion0Pickle(
     bool size_32, const PasswordForm& form, Pickle* pickle) {
   pickle->WriteInt(0);
-  CreatePickle(size_32, form, pickle);
+  CreatePickle(size_32, false, form, pickle);
 }
 
-void NativeBackendKWalletPickleTest::CreatePickle(
-    bool size_32, const PasswordForm& form, Pickle* pickle) {
+void NativeBackendKWalletPickleTest::CreatePickle(bool size_32,
+                                                  bool date_created_internal,
+                                                  const PasswordForm& form,
+                                                  Pickle* pickle) {
   if (size_32)
     pickle->WriteUInt32(1);  // Size of form list. 32 bits.
   else
     pickle->WriteSizeT(1);  // Size of form list. 64 bits.
   pickle->WriteInt(form.scheme);
   pickle->WriteString(form.origin.spec());
   pickle->WriteString(form.action.spec());
   pickle->WriteString16(form.username_element);
   pickle->WriteString16(form.username_value);
   pickle->WriteString16(form.password_element);
   pickle->WriteString16(form.password_value);
   pickle->WriteString16(form.submit_element);
   pickle->WriteBool(form.ssl_valid);
   pickle->WriteBool(form.preferred);
   pickle->WriteBool(form.blacklisted_by_user);
-  pickle->WriteInt64(form.date_created.ToTimeT());
+  if (date_created_internal)
+    pickle->WriteInt64(form.date_created.ToInternalValue());
+  else
+    pickle->WriteInt64(form.date_created.ToTimeT());
+}
+
+void NativeBackendKWalletPickleTest::CheckVersion5Pickle() {
+  Pickle pickle;
+  PasswordForm form = form_google_;
+  CreateVersion5Pickle(form, &pickle);
+
+  ScopedVector<PasswordForm> form_list;
+  NativeBackendKWalletStub::DeserializeValue(form.signon_realm, pickle,
+                                             &form_list);
+
+  EXPECT_EQ(1u, form_list.size());
+  if (form_list.size() > 0)
+    CheckPasswordForm(form, *form_list[0], 5);
 }
 
 void NativeBackendKWalletPickleTest::CheckVersion3Pickle() {
   Pickle pickle;
   PasswordForm form = form_google_;
   // Remove the fields which were not present in version #3.
   form.display_name.clear();
   form.avatar_url = GURL();
   form.federation_url = GURL();
   form.is_zero_click = false;
   CreateVersion3Pickle(form, &pickle);
 
   ScopedVector<PasswordForm> form_list;
   NativeBackendKWalletStub::DeserializeValue(form.signon_realm, pickle,
                                              &form_list);
 
   EXPECT_EQ(1u, form_list.size());
   if (form_list.size() > 0)
-    CheckPasswordForm(form, *form_list[0]);
+    CheckPasswordForm(form, *form_list[0], 3);
 }
 
 void NativeBackendKWalletPickleTest::CheckVersion2Pickle() {
   Pickle pickle;
   PasswordForm form = old_form_google_;
   form.times_used = form_google_.times_used;
   form.type = form_google_.type;
   form.form_data = form_google_.form_data;
   CreateVersion2Pickle(form, &pickle);
 
   ScopedVector<PasswordForm> form_list;
   NativeBackendKWalletStub::DeserializeValue(form.signon_realm, pickle,
                                              &form_list);
 
   EXPECT_EQ(1u, form_list.size());
   if (form_list.size() > 0)
-    CheckPasswordForm(form, *form_list[0]);
+    CheckPasswordForm(form, *form_list[0], 2);
 }
 
 // Make sure that we can still read version 1 pickles.
 void NativeBackendKWalletPickleTest::CheckVersion1Pickle() {
   Pickle pickle;
   PasswordForm form = form_google_;
   CreateVersion1Pickle(form, &pickle);
 
   ScopedVector<autofill::PasswordForm> form_list;
   NativeBackendKWalletStub::DeserializeValue(form.signon_realm,
                                              pickle, &form_list);
 
   // This will match |old_form_google_| because not all the fields present in
   // |form_google_| will be deserialized.
   EXPECT_EQ(1u, form_list.size());
   if (form_list.size() > 0)
-    CheckPasswordForm(old_form_google_, *form_list[0]);
+    CheckPasswordForm(old_form_google_, *form_list[0], 1);
 }
 
 void NativeBackendKWalletPickleTest::CheckVersion0Pickle(
     bool size_32, PasswordForm::Scheme scheme) {
   Pickle pickle;
   PasswordForm form = old_form_google_;
   form.scheme = scheme;
   CreateVersion0Pickle(size_32, form, &pickle);
   ScopedVector<autofill::PasswordForm> form_list;
   NativeBackendKWalletStub::DeserializeValue(form.signon_realm,
                                              pickle, &form_list);
   EXPECT_EQ(1u, form_list.size());
   if (form_list.size() > 0)
-    CheckPasswordForm(form, *form_list[0]);
+    CheckPasswordForm(form, *form_list[0], 0);
 }
 
 // Check that if KWallet fails to respond, the backend propagates the error.
 TEST_F(NativeBackendKWalletTest, GetAllLoginsErrorHandling) {
   NativeBackendKWalletStub backend(42);
   EXPECT_TRUE(backend.InitWithBus(mock_session_bus_));
   // Make KWallet fail on calling readEntry.
   failing_methods_.insert("readEntry");
 
   // Store some non-blacklisted logins to be potentially returned.
@@ skipping 39 matching lines @@
   CheckVersion1Pickle();
 }
 
 TEST_F(NativeBackendKWalletPickleTest, CheckVersion2Pickle) {
   CheckVersion2Pickle();
 }
 
 TEST_F(NativeBackendKWalletPickleTest, CheckVersion3Pickle) {
   CheckVersion3Pickle();
 }
+
+TEST_F(NativeBackendKWalletPickleTest, CheckVersion5Pickle) {
+  CheckVersion3Pickle();

[vabr (Chromium), 2015/02/03 16:06:42]

3->5

[dvadym, 2015/02/04 10:40:25]

Done.

+}