QuotaDatabase: Wrap int/Time conversions in helper methods

As a follow on to https://crrev.com/c/1248328, wrap conversions
between base::Time and the particular format used for persistence in
the quota database (int64_t) in methods, instead of inlining the
conversion steps at every point.

Bug: 889242
Change-Id: I8d5b7b6a6e7bf9ccf507cd8c3ad750dae9e30206
Reviewed-on: https://chromium-review.googlesource.com/1254744
Commit-Queue: Joshua Bell <jsbell@chromium.org>
Reviewed-by: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#595589}

storage/browser/quota/quota_database.cc

@@ -211,12 +211,10 @@ bool QuotaDatabase::SetOriginLastAccessTime(const url::Origin& origin,
         " (used_count, last_access_time, origin, type, last_modified_time)"
         " VALUES (?, ?, ?, ?, ?)";
     statement.Assign(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
-    statement.BindInt64(
-        4, last_access_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+    statement.BindInt64(4, TimeToSqlValue(last_access_time));
   }
   statement.BindInt(0, entry.used_count);
-  statement.BindInt64(
-      1, last_access_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+  statement.BindInt64(1, TimeToSqlValue(last_access_time));
   statement.BindString(2, origin.GetURL().spec());
   statement.BindInt(3, static_cast<int>(type));
 
@@ -248,11 +246,9 @@ bool QuotaDatabase::SetOriginLastModifiedTime(const url::Origin& origin,
         "INSERT INTO OriginInfoTable"
         " (last_modified_time, origin, type, last_access_time)  VALUES (?, ?, ?, ?)";
     statement.Assign(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
-    statement.BindInt64(
-        3, last_modified_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+    statement.BindInt64(3, TimeToSqlValue(last_modified_time));
   }
-  statement.BindInt64(
-      0, last_modified_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+  statement.BindInt64(0, TimeToSqlValue(last_modified_time));
 
   statement.BindString(1, origin.GetURL().spec());
   statement.BindInt(2, static_cast<int>(type));
@@ -283,8 +279,7 @@ bool QuotaDatabase::GetOriginLastEvictionTime(const url::Origin& origin,
   if (!statement.Step())
     return false;
 
-  *last_modified_time = base::Time::FromDeltaSinceWindowsEpoch(
-      base::TimeDelta::FromMicroseconds(statement.ColumnInt64(0)));
+  *last_modified_time = TimeFromSqlValue(statement.ColumnInt64(0));
   return true;
 }
 
@@ -299,8 +294,7 @@ bool QuotaDatabase::SetOriginLastEvictionTime(const url::Origin& origin,
       " (last_eviction_time, origin, type)"
       " VALUES (?, ?, ?)";
   sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
-  statement.BindInt64(
-      0, last_modified_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+  statement.BindInt64(0, TimeToSqlValue(last_modified_time));
   statement.BindString(1, origin.GetURL().spec());
   statement.BindInt(2, static_cast<int>(type));
 
@@ -373,10 +367,8 @@ bool QuotaDatabase::GetOriginInfo(const url::Origin& origin,
   *entry = OriginInfoTableEntry(
       url::Origin::Create(GURL(statement.ColumnString(0))),
       static_cast<StorageType>(statement.ColumnInt(1)), statement.ColumnInt(2),
-      base::Time::FromDeltaSinceWindowsEpoch(
-          base::TimeDelta::FromMicroseconds(statement.ColumnInt64(3))),
-      base::Time::FromDeltaSinceWindowsEpoch(
-          base::TimeDelta::FromMicroseconds(statement.ColumnInt64(4))));
+      TimeFromSqlValue(statement.ColumnInt64(3)),
+      TimeFromSqlValue(statement.ColumnInt64(4)));
 
   return true;
 }
@@ -483,8 +475,7 @@ bool QuotaDatabase::GetOriginsModifiedSince(StorageType type,
 
   sql::Statement statement(db_->GetCachedStatement(SQL_FROM_HERE, kSql));
   statement.BindInt(0, static_cast<int>(type));
-  statement.BindInt64(
-      1, modified_since.ToDeltaSinceWindowsEpoch().InMicroseconds());
+  statement.BindInt64(1, TimeToSqlValue(modified_since));
 
   origins->clear();
   while (statement.Step())
@@ -766,11 +757,8 @@ bool QuotaDatabase::DumpOriginInfoTable(
     OriginInfoTableEntry entry(
         url::Origin::Create(GURL(statement.ColumnString(0))),
         static_cast<StorageType>(statement.ColumnInt(1)),
-        statement.ColumnInt(2),
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(statement.ColumnInt64(3))),
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(statement.ColumnInt64(4))));
+        statement.ColumnInt(2), TimeFromSqlValue(statement.ColumnInt64(3)),
+        TimeFromSqlValue(statement.ColumnInt64(4)));
 
     if (!callback.Run(entry))
       return true;
@@ -779,6 +767,17 @@ bool QuotaDatabase::DumpOriginInfoTable(
   return statement.Succeeded();
 }
 
+// static
+base::Time QuotaDatabase::TimeFromSqlValue(int64_t time) {
+  return base::Time::FromDeltaSinceWindowsEpoch(
+      base::TimeDelta::FromMicroseconds(time));
+}
+
+// static
+int64_t QuotaDatabase::TimeToSqlValue(const base::Time& time) {
+  return time.ToDeltaSinceWindowsEpoch().InMicroseconds();
+}
+
 bool operator<(const QuotaDatabase::QuotaTableEntry& lhs,
                const QuotaDatabase::QuotaTableEntry& rhs) {
   return std::tie(lhs.host, lhs.type, lhs.quota) <

storage/browser/quota/quota_database.h

@@ -195,6 +195,12 @@ class STORAGE_EXPORT QuotaDatabase {
   bool DumpQuotaTable(const QuotaTableCallback& callback);
   bool DumpOriginInfoTable(const OriginInfoTableCallback& callback);
 
+  // Serialize/deserialize base::Time objects to a stable representation for
+  // persistence in the database.
+  // TODO(pwnall): Add support for base::Time values to //sql directly.
+  static base::Time TimeFromSqlValue(int64_t time);
+  static int64_t TimeToSqlValue(const base::Time& time);
+
   base::FilePath db_file_path_;
 
   std::unique_ptr<sql::Database> db_;

storage/browser/quota/quota_database_unittest.cc

@@ -164,24 +164,16 @@ class QuotaDatabaseTest : public testing::Test {
     const url::Origin kOrigin4 = url::Origin::Create(GURL("http://p/"));
 
     // Adding three temporary storages, and
-    EXPECT_TRUE(
-        db.SetOriginLastAccessTime(kOrigin1, kTemporary,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(10))));
-    EXPECT_TRUE(
-        db.SetOriginLastAccessTime(kOrigin2, kTemporary,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(20))));
-    EXPECT_TRUE(
-        db.SetOriginLastAccessTime(kOrigin3, kTemporary,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(30))));
+    EXPECT_TRUE(db.SetOriginLastAccessTime(
+        kOrigin1, kTemporary, QuotaDatabase::TimeFromSqlValue(10)));
+    EXPECT_TRUE(db.SetOriginLastAccessTime(
+        kOrigin2, kTemporary, QuotaDatabase::TimeFromSqlValue(20)));
+    EXPECT_TRUE(db.SetOriginLastAccessTime(
+        kOrigin3, kTemporary, QuotaDatabase::TimeFromSqlValue(30)));
 
     // one persistent.
-    EXPECT_TRUE(
-        db.SetOriginLastAccessTime(kOrigin4, kPersistent,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(40))));
+    EXPECT_TRUE(db.SetOriginLastAccessTime(
+        kOrigin4, kPersistent, QuotaDatabase::TimeFromSqlValue(40)));
 
     EXPECT_TRUE(db.GetLRUOrigin(kTemporary, exceptions, nullptr, &origin));
     EXPECT_EQ(kOrigin1, origin);
@@ -243,17 +235,11 @@ class QuotaDatabaseTest : public testing::Test {
 
     // Report last mod time for the test origins.
     EXPECT_TRUE(db.SetOriginLastModifiedTime(
-        kOrigin1, kTemporary,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(0))));
+        kOrigin1, kTemporary, QuotaDatabase::TimeFromSqlValue(0)));
     EXPECT_TRUE(db.SetOriginLastModifiedTime(
-        kOrigin2, kTemporary,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(10))));
+        kOrigin2, kTemporary, QuotaDatabase::TimeFromSqlValue(10)));
     EXPECT_TRUE(db.SetOriginLastModifiedTime(
-        kOrigin3, kTemporary,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(20))));
+        kOrigin3, kTemporary, QuotaDatabase::TimeFromSqlValue(20)));
 
     EXPECT_TRUE(db.GetOriginsModifiedSince(kTemporary, &origins, base::Time()));
     EXPECT_EQ(3U, origins.size());
@@ -261,41 +247,31 @@ class QuotaDatabaseTest : public testing::Test {
     EXPECT_EQ(1U, origins.count(kOrigin2));
     EXPECT_EQ(1U, origins.count(kOrigin3));
 
-    EXPECT_TRUE(
-        db.GetOriginsModifiedSince(kTemporary, &origins,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(5))));
+    EXPECT_TRUE(db.GetOriginsModifiedSince(kTemporary, &origins,
+                                           QuotaDatabase::TimeFromSqlValue(5)));
     EXPECT_EQ(2U, origins.size());
     EXPECT_EQ(0U, origins.count(kOrigin1));
     EXPECT_EQ(1U, origins.count(kOrigin2));
     EXPECT_EQ(1U, origins.count(kOrigin3));
 
-    EXPECT_TRUE(
-        db.GetOriginsModifiedSince(kTemporary, &origins,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(15))));
+    EXPECT_TRUE(db.GetOriginsModifiedSince(
+        kTemporary, &origins, QuotaDatabase::TimeFromSqlValue(15)));
     EXPECT_EQ(1U, origins.size());
     EXPECT_EQ(0U, origins.count(kOrigin1));
     EXPECT_EQ(0U, origins.count(kOrigin2));
     EXPECT_EQ(1U, origins.count(kOrigin3));
 
-    EXPECT_TRUE(
-        db.GetOriginsModifiedSince(kTemporary, &origins,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(25))));
+    EXPECT_TRUE(db.GetOriginsModifiedSince(
+        kTemporary, &origins, QuotaDatabase::TimeFromSqlValue(25)));
     EXPECT_TRUE(origins.empty());
 
     // Update origin1's mod time but for persistent storage.
     EXPECT_TRUE(db.SetOriginLastModifiedTime(
-        kOrigin1, kPersistent,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(30))));
+        kOrigin1, kPersistent, QuotaDatabase::TimeFromSqlValue(30)));
 
     // Must have no effects on temporary origins info.
-    EXPECT_TRUE(
-        db.GetOriginsModifiedSince(kTemporary, &origins,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(5))));
+    EXPECT_TRUE(db.GetOriginsModifiedSince(kTemporary, &origins,
+                                           QuotaDatabase::TimeFromSqlValue(5)));
     EXPECT_EQ(2U, origins.size());
     EXPECT_EQ(0U, origins.count(kOrigin1));
     EXPECT_EQ(1U, origins.count(kOrigin2));
@@ -303,23 +279,17 @@ class QuotaDatabaseTest : public testing::Test {
 
     // One more update for persistent origin2.
     EXPECT_TRUE(db.SetOriginLastModifiedTime(
-        kOrigin2, kPersistent,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(40))));
+        kOrigin2, kPersistent, QuotaDatabase::TimeFromSqlValue(40)));
 
-    EXPECT_TRUE(
-        db.GetOriginsModifiedSince(kPersistent, &origins,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(25))));
+    EXPECT_TRUE(db.GetOriginsModifiedSince(
+        kPersistent, &origins, QuotaDatabase::TimeFromSqlValue(25)));
     EXPECT_EQ(2U, origins.size());
     EXPECT_EQ(1U, origins.count(kOrigin1));
     EXPECT_EQ(1U, origins.count(kOrigin2));
     EXPECT_EQ(0U, origins.count(kOrigin3));
 
-    EXPECT_TRUE(
-        db.GetOriginsModifiedSince(kPersistent, &origins,
-                                   base::Time::FromDeltaSinceWindowsEpoch(
-                                       base::TimeDelta::FromMicroseconds(35))));
+    EXPECT_TRUE(db.GetOriginsModifiedSince(
+        kPersistent, &origins, QuotaDatabase::TimeFromSqlValue(35)));
     EXPECT_EQ(1U, origins.size());
     EXPECT_EQ(0U, origins.count(kOrigin1));
     EXPECT_EQ(1U, origins.count(kOrigin2));
@@ -341,33 +311,21 @@ class QuotaDatabaseTest : public testing::Test {
 
     // Report last eviction time for the test origins.
     EXPECT_TRUE(db.SetOriginLastEvictionTime(
-        kOrigin1, kTemporary,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(10))));
+        kOrigin1, kTemporary, QuotaDatabase::TimeFromSqlValue(10)));
     EXPECT_TRUE(db.SetOriginLastEvictionTime(
-        kOrigin2, kTemporary,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(20))));
+        kOrigin2, kTemporary, QuotaDatabase::TimeFromSqlValue(20)));
     EXPECT_TRUE(db.SetOriginLastEvictionTime(
-        kOrigin3, kTemporary,
-        base::Time::FromDeltaSinceWindowsEpoch(
-            base::TimeDelta::FromMicroseconds(30))));
+        kOrigin3, kTemporary, QuotaDatabase::TimeFromSqlValue(30)));
 
     EXPECT_TRUE(db.GetOriginLastEvictionTime(kOrigin1, kTemporary,
                                              &last_eviction_time));
-    EXPECT_EQ(base::Time::FromDeltaSinceWindowsEpoch(
-                  base::TimeDelta::FromMicroseconds(10)),
-              last_eviction_time);
+    EXPECT_EQ(QuotaDatabase::TimeFromSqlValue(10), last_eviction_time);
     EXPECT_TRUE(db.GetOriginLastEvictionTime(kOrigin2, kTemporary,
                                              &last_eviction_time));
-    EXPECT_EQ(base::Time::FromDeltaSinceWindowsEpoch(
-                  base::TimeDelta::FromMicroseconds(20)),
-              last_eviction_time);
+    EXPECT_EQ(QuotaDatabase::TimeFromSqlValue(20), last_eviction_time);
     EXPECT_TRUE(db.GetOriginLastEvictionTime(kOrigin3, kTemporary,
                                              &last_eviction_time));
-    EXPECT_EQ(base::Time::FromDeltaSinceWindowsEpoch(
-                  base::TimeDelta::FromMicroseconds(30)),
-              last_eviction_time);
+    EXPECT_EQ(QuotaDatabase::TimeFromSqlValue(30), last_eviction_time);
 
     // Delete last eviction times for the test origins.
     EXPECT_TRUE(db.DeleteOriginLastEvictionTime(kOrigin1, kTemporary));
@@ -547,11 +505,10 @@ class QuotaDatabaseTest : public testing::Test {
       statement.BindString(0, itr->origin.GetURL().spec());
       statement.BindInt(1, static_cast<int>(itr->type));
       statement.BindInt(2, itr->used_count);
+      statement.BindInt64(3,
+                          QuotaDatabase::TimeToSqlValue(itr->last_access_time));
       statement.BindInt64(
-          3, itr->last_access_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
-      statement.BindInt64(
-          4,
-          itr->last_modified_time.ToDeltaSinceWindowsEpoch().InMicroseconds());
+          4, QuotaDatabase::TimeToSqlValue(itr->last_modified_time));
       EXPECT_TRUE(statement.Run());
     }
   }