Use higher precision to store the login database's date_created field.

components/password_manager/core/browser/login_database_unittest.cc

Index: components/password_manager/core/browser/login_database_unittest.cc
diff --git a/components/password_manager/core/browser/login_database_unittest.cc b/components/password_manager/core/browser/login_database_unittest.cc
index e3fe7e516f743ba63db4d0d0715bc9b1d29f5d63..5d5eb024e0add7fd95fc4192ddcf2dbe58721a98 100644
--- a/components/password_manager/core/browser/login_database_unittest.cc
+++ b/components/password_manager/core/browser/login_database_unittest.cc
@@ -14,6 +14,8 @@
 #include "base/time/time.h"
 #include "components/autofill/core/common/password_form.h"
 #include "components/password_manager/core/browser/psl_matching_helper.h"
+#include "sql/connection.h"
+#include "sql/statement.h"
 #include "testing/gmock/include/gmock/gmock.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -928,8 +930,6 @@ TEST_F(LoginDatabaseTest, UpdateLogin) {
   form.submit_element = ASCIIToUTF16("submit_element");
   form.date_synced = base::Time::Now();
   form.date_created = base::Time::Now() - base::TimeDelta::FromDays(1);
-  // Remove this line after crbug/374132 is fixed.
-  form.date_created = base::Time::FromTimeT(form.date_created.ToTimeT());
   form.blacklisted_by_user = true;
   form.scheme = PasswordForm::SCHEME_BASIC;
   form.type = PasswordForm::TYPE_GENERATED;
@@ -963,4 +963,73 @@ TEST_F(LoginDatabaseTest, FilePermissions) {
 }
 #endif  // defined(OS_POSIX)
 
+class LoginDatabaseMigrationTest : public testing::Test {
+ protected:
+  void SetUp() override {
+    PathService::Get(base::DIR_SOURCE_ROOT, &original_database_path_);
+    original_database_path_ = original_database_path_.AppendASCII("components")
+                                  .AppendASCII("test")
+                                  .AppendASCII("data")
+                                  .AppendASCII("password_manager")
+                                  .AppendASCII("login_db_v8.sqlite3");

[Garrett Casto, 2014/11/06 23:17:16]

Instead of storing the actual database, can you store the output of .dump and
use CreateDatabaseFromSQL() to recreate? Makes it easier to see what is being
tested.

[erikchen, 2014/11/07 03:24:04]

Good call. Done.

+
+    // Make a temporary copy of the original database.
+    ASSERT_TRUE(CreateTemporaryFile(&database_path_));
+    ASSERT_TRUE(base::CopyFile(original_database_path_, database_path_));
+  }
+
+  void TearDown() override {
+    if (!database_path_.empty())
+      base::DeleteFile(database_path_, false);
+  }
+
+  // Requires that the login table in the database has exactly one entry.
+  // Returns -1 on failure. Otherwise returns the value of the date_created
+  // column of the single entry.
+  int64_t GetDateCreated(const base::FilePath& db_path) {

[Garrett Casto, 2014/11/06 23:17:15]

Nit: Maybe have 2 elements in the SQL database to make sure that we properly
convert multiple entries? 

[erikchen, 2014/11/07 03:24:04]

Done.

+    sql::Connection db;
+    if (!db.Open(db_path))
+      return -1;
+
+    sql::Statement s(db.GetCachedStatement(SQL_FROM_HERE,
+                                           "SELECT date_created from logins"));
+    if (!s.is_valid()) {
+      db.Close();
+      return -1;
+    }
+    int64_t date_created = 0;
+    while (s.Step()) {
+      date_created = s.ColumnInt64(0);
+    }
+    db.Close();
+    return date_created;
+  }
+
+  base::FilePath database_path_;
+
+ private:
+  base::FilePath original_database_path_;
+};
+
+// Tests the migration of the login database from version 8 to version 9.
+TEST_F(LoginDatabaseMigrationTest, MigrationV8ToV9) {
+  // Original date, in seconds since UTC epoch.
+  int64_t date_created = GetDateCreated(database_path_);
+  ASSERT_EQ(1402955745, date_created);

[Garrett Casto, 2014/11/06 23:17:16]

Nit: I don't think that you need to check the actual values here. We only care
that the base::Time values are equivalent.

[erikchen, 2014/11/07 03:24:04]

I disagree. If we don't check actual values, and someone changes the internals
of time.h (e.g. the value of kTimeTToMicrosecondsOffset), then this test will
still succeed. However, previous migrations will have different behavior from
future migrations, which is an error that we want to catch.

[Garrett Casto, 2014/11/09 09:15:23]

Since we generally depend on kTimeTToMicrosecondsOffset being fixed (or more
precisely that FromTimeT() and ToTimeT() are stable), I would think that if we
wanted to verify this it would be it's own test. It's not a huge deal either way
though.

+
+  // Assert that the database was successfully opened and migrated.
+  {
+    LoginDatabase db;
+    ASSERT_TRUE(db.Init(database_path_));
+  }
+
+  // New date, in microseconds since platform independent epoch.
+  int64_t new_date_created = GetDateCreated(database_path_);
+  ASSERT_EQ(13047429345000000, new_date_created);
+
+  // Check that the two dates match up.
+  EXPECT_EQ(new_date_created,
+            base::Time::FromTimeT(date_created).ToInternalValue());

[Garrett Casto, 2014/11/06 23:17:15]

Nit: Can you make this compare in base::Time() since that is the format we care
about. So EXPECT_EQ(base::Time::FromInternalValue(new_date_created),
base::Time::FromTimeT(date_created))

[erikchen, 2014/11/07 03:24:04]

Done.

+}
+
 }  // namespace password_manager