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,
+    bool check_date_created) {
   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);
-  // We don't check the date created. It varies.
+  if (check_date_created) {
+    EXPECT_EQ(expected.date_created, actual.date_created);
+  }
   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(), true);
   }
 }
 
 // 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], true);
   }
 }
 
 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);
+  // |size_32| stores size as 32/64 bit integer, should be false for version>0.
+  // |date_created_internal| store date_created as base::Time's internal value.

[vabr (Chromium), 2015/02/04 12:23:51]

Please make this comment into full sentences, and clarify vague terms.
Suggestion:

Creates a Pickle from |form|. If |size_32| is true, stores the number of forms
in the pickle as a 32bit uint, otherwise as size_t. The latter should be the
case for versions > 0. If |date_created_internal| is true, stores date_created
as base::Time's internal value, otherwise as time_t.

[dvadym, 2015/02/04 14:24:00]

Done.

+  void CreatePickle(bool size_32,
+                    bool date_created_internal,
+                    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], true);
 }
 
 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], false);
 }
 
 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], false);
 }
 
 // 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], false);
 }
 
 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], false);
 }
 
 // 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) {
+  CheckVersion5Pickle();
+}