0

sql: Add BindTime() and ColumnTime() to sql::Statement.

These helpers use the recommended base::Time serialization method. This
addition is intended to increase the likelihood that new features use
the recommended serialization method, and reduce logic duplication
throughout the codebase.

Follow-up CLs will replace explicit base::Time serialization in SQL
code with calls to these helpers.

This CL does not introduce any behavior changes.

Bug: 1195962
Change-Id: Ic32b318192df1fff39ce7fe555d73b429a5aef97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2805559
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Ayu Ishii <ayui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#869342}
This commit is contained in:
Victor Costan
2021-04-06 00:53:48 +00:00
committed by Chromium LUCI CQ
parent 04add837ed
commit ad6b0113cd
5 changed files with 109 additions and 81 deletions

@ -13,6 +13,7 @@
#include "base/strings/string_piece_forward.h"
#include "base/strings/string_util.h"
#include "base/strings/utf_string_conversions.h"
#include "base/time/time.h"
#include "build/build_config.h" // TODO(crbug.com/866218): Remove this include.
#include "third_party/sqlite/sqlite3.h"
@ -182,6 +183,17 @@ bool Statement::BindDouble(int col, double val) {
return is_valid() && CheckOk(sqlite3_bind_double(ref_->stmt(), col + 1, val));
}
bool Statement::BindTime(int col, base::Time val) {
#if !defined(OS_ANDROID) // TODO(crbug.com/866218): Remove this conditional
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
#endif // OS_ANDROID
DCHECK(!stepped_);
int64_t int_value = val.ToDeltaSinceWindowsEpoch().InMicroseconds();
return is_valid() &&
CheckOk(sqlite3_bind_int64(ref_->stmt(), col + 1, int_value));
}
bool Statement::BindCString(int col, const char* val) {
#if !defined(OS_ANDROID) // TODO(crbug.com/866218): Remove this conditional
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@ -290,6 +302,19 @@ double Statement::ColumnDouble(int col) const {
return sqlite3_column_double(ref_->stmt(), col);
}
base::Time Statement::ColumnTime(int col) const {
#if !defined(OS_ANDROID) // TODO(crbug.com/866218): Remove this conditional
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
#endif // OS_ANDROID
if (!CheckValid())
return base::Time();
int64_t int_value = sqlite3_column_int64(ref_->stmt(), col);
return base::Time::FromDeltaSinceWindowsEpoch(
base::TimeDelta::FromMicroseconds(int_value));
}
std::string Statement::ColumnString(int col) const {
#if !defined(OS_ANDROID) // TODO(crbug.com/866218): Remove this conditional
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

@ -15,6 +15,7 @@
#include "base/memory/ref_counted.h"
#include "base/sequence_checker.h"
#include "base/strings/string_piece_forward.h"
#include "base/time/time.h"
#include "build/build_config.h" // TODO(crbug.com/866218): Remove this include.
#include "sql/database.h"
@ -127,6 +128,20 @@ class COMPONENT_EXPORT(SQL) Statement {
bool BindString16(int col, base::StringPiece16 value);
bool BindBlob(int col, const void* value, int value_len);
// Conforms with base::Time serialization recommendations.
//
// This is equivalent to the following snippets, which should be replaced.
// * BindInt64(col, val.ToInternalValue())
// * BindInt64(col, val.ToDeltaSinceWindowsEpoch().InMicroseconds())
//
// Features that serialize base::Time in other ways, such as ToTimeT() or
// ToJavaTime(), will require a database migration to be converted to this
// (recommended) serialization method.
//
// TODO(crbug.com/1195962): Migrate all time serialization to this method, and
// then remove the migration details above.
bool BindTime(int col, base::Time time);
// Retrieving ----------------------------------------------------------------
// Returns the number of output columns in the result.
@ -148,6 +163,17 @@ class COMPONENT_EXPORT(SQL) Statement {
std::string ColumnString(int col) const;
std::u16string ColumnString16(int col) const;
// Conforms with base::Time serialization recommendations.
//
// This is equivalent to the following snippets, which should be replaced.
// * base::Time::FromInternalValue(ColumnInt64(col))
// * base::Time::FromDeltaSinceWindowsEpoch(
// base::TimeDelta::FromMicroseconds(ColumnInt64(col)))
//
// TODO(crbug.com/1195962): Migrate all time serialization to this method, and
// then remove the migration details above.
base::Time ColumnTime(int col) const;
// When reading a blob, you can get a raw pointer to the underlying data,
// along with the length, or you can just ask us to copy the blob into a
// vector. Danger! ColumnBlob may return nullptr if there is no data!

@ -192,10 +192,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, TimeToSqlValue(last_access_time));
statement.BindTime(4, last_access_time);
}
statement.BindInt(0, entry.used_count);
statement.BindInt64(1, TimeToSqlValue(last_access_time));
statement.BindTime(1, last_access_time);
statement.BindString(2, origin.GetURL().spec());
statement.BindInt(3, static_cast<int>(type));
@ -227,9 +227,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, TimeToSqlValue(last_modified_time));
statement.BindTime(3, last_modified_time);
}
statement.BindInt64(0, TimeToSqlValue(last_modified_time));
statement.BindTime(0, last_modified_time);
statement.BindString(1, origin.GetURL().spec());
statement.BindInt(2, static_cast<int>(type));
@ -261,7 +261,7 @@ bool QuotaDatabase::GetOriginLastEvictionTime(const url::Origin& origin,
if (!statement.Step())
return false;
*last_modified_time = TimeFromSqlValue(statement.ColumnInt64(0));
*last_modified_time = statement.ColumnTime(0);
return true;
}
@ -277,7 +277,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, TimeToSqlValue(last_modified_time));
statement.BindTime(0, last_modified_time);
statement.BindString(1, origin.GetURL().spec());
statement.BindInt(2, static_cast<int>(type));
@ -353,8 +353,7 @@ 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),
TimeFromSqlValue(statement.ColumnInt64(3)),
TimeFromSqlValue(statement.ColumnInt64(4)));
statement.ColumnTime(3), statement.ColumnTime(4));
return true;
}
@ -463,9 +462,9 @@ bool QuotaDatabase::GetOriginsModifiedBetween(StorageType type,
statement.Assign(db_->GetCachedStatement(SQL_FROM_HERE, kSqlQueryBetween));
}
statement.BindInt(0, static_cast<int>(type));
statement.BindInt64(1, TimeToSqlValue(begin));
statement.BindTime(1, begin);
if (!end.is_max())
statement.BindInt64(2, TimeToSqlValue(end));
statement.BindTime(2, end);
origins->clear();
while (statement.Step())
@ -758,8 +757,8 @@ bool QuotaDatabase::DumpOriginInfoTable(
OriginInfoTableEntry entry(
url::Origin::Create(GURL(statement.ColumnString(0))),
static_cast<StorageType>(statement.ColumnInt(1)),
statement.ColumnInt(2), TimeFromSqlValue(statement.ColumnInt64(3)),
TimeFromSqlValue(statement.ColumnInt64(4)));
statement.ColumnInt(2), statement.ColumnTime(3),
statement.ColumnTime(4));
if (!callback.Run(entry))
return true;
@ -768,17 +767,6 @@ 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) <

@ -191,11 +191,6 @@ class COMPONENT_EXPORT(STORAGE_BROWSER) 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);
const base::FilePath db_file_path_;

@ -132,16 +132,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, QuotaDatabase::TimeFromSqlValue(10)));
EXPECT_TRUE(db.SetOriginLastAccessTime(
kOrigin2, kTemporary, QuotaDatabase::TimeFromSqlValue(20)));
EXPECT_TRUE(db.SetOriginLastAccessTime(
kOrigin3, kTemporary, QuotaDatabase::TimeFromSqlValue(30)));
EXPECT_TRUE(db.SetOriginLastAccessTime(kOrigin1, kTemporary,
base::Time::FromJavaTime(10)));
EXPECT_TRUE(db.SetOriginLastAccessTime(kOrigin2, kTemporary,
base::Time::FromJavaTime(20)));
EXPECT_TRUE(db.SetOriginLastAccessTime(kOrigin3, kTemporary,
base::Time::FromJavaTime(30)));
// one persistent.
EXPECT_TRUE(db.SetOriginLastAccessTime(
kOrigin4, kPersistent, QuotaDatabase::TimeFromSqlValue(40)));
EXPECT_TRUE(db.SetOriginLastAccessTime(kOrigin4, kPersistent,
base::Time::FromJavaTime(40)));
EXPECT_TRUE(db.GetLRUOrigin(kTemporary, exceptions, nullptr, &origin));
EXPECT_EQ(kOrigin1, origin);
@ -203,12 +203,12 @@ class QuotaDatabaseTest : public testing::Test {
const url::Origin kOrigin3 = url::Origin::Create(GURL("http://c/"));
// Report last mod time for the test origins.
EXPECT_TRUE(db.SetOriginLastModifiedTime(
kOrigin1, kTemporary, QuotaDatabase::TimeFromSqlValue(0)));
EXPECT_TRUE(db.SetOriginLastModifiedTime(
kOrigin2, kTemporary, QuotaDatabase::TimeFromSqlValue(10)));
EXPECT_TRUE(db.SetOriginLastModifiedTime(
kOrigin3, kTemporary, QuotaDatabase::TimeFromSqlValue(20)));
EXPECT_TRUE(db.SetOriginLastModifiedTime(kOrigin1, kTemporary,
base::Time::FromJavaTime(0)));
EXPECT_TRUE(db.SetOriginLastModifiedTime(kOrigin2, kTemporary,
base::Time::FromJavaTime(10)));
EXPECT_TRUE(db.SetOriginLastModifiedTime(kOrigin3, kTemporary,
base::Time::FromJavaTime(20)));
EXPECT_TRUE(db.GetOriginsModifiedBetween(kTemporary, &origins, base::Time(),
base::Time::Max()));
@ -217,71 +217,67 @@ class QuotaDatabaseTest : public testing::Test {
EXPECT_EQ(1U, origins.count(kOrigin2));
EXPECT_EQ(1U, origins.count(kOrigin3));
EXPECT_TRUE(db.GetOriginsModifiedBetween(kTemporary, &origins,
QuotaDatabase::TimeFromSqlValue(5),
base::Time::Max()));
EXPECT_TRUE(db.GetOriginsModifiedBetween(
kTemporary, &origins, base::Time::FromJavaTime(5), base::Time::Max()));
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.GetOriginsModifiedBetween(
kTemporary, &origins, QuotaDatabase::TimeFromSqlValue(15),
base::Time::Max()));
kTemporary, &origins, base::Time::FromJavaTime(15), base::Time::Max()));
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.GetOriginsModifiedBetween(
kTemporary, &origins, QuotaDatabase::TimeFromSqlValue(25),
base::Time::Max()));
kTemporary, &origins, base::Time::FromJavaTime(25), base::Time::Max()));
EXPECT_TRUE(origins.empty());
EXPECT_TRUE(db.GetOriginsModifiedBetween(
kTemporary, &origins, QuotaDatabase::TimeFromSqlValue(5),
QuotaDatabase::TimeFromSqlValue(15)));
EXPECT_TRUE(db.GetOriginsModifiedBetween(kTemporary, &origins,
base::Time::FromJavaTime(5),
base::Time::FromJavaTime(15)));
EXPECT_EQ(1U, origins.size());
EXPECT_EQ(0U, origins.count(kOrigin1));
EXPECT_EQ(1U, origins.count(kOrigin2));
EXPECT_EQ(0U, origins.count(kOrigin3));
EXPECT_TRUE(db.GetOriginsModifiedBetween(
kTemporary, &origins, QuotaDatabase::TimeFromSqlValue(0),
QuotaDatabase::TimeFromSqlValue(20)));
EXPECT_TRUE(db.GetOriginsModifiedBetween(kTemporary, &origins,
base::Time::FromJavaTime(0),
base::Time::FromJavaTime(20)));
EXPECT_EQ(2U, origins.size());
EXPECT_EQ(1U, origins.count(kOrigin1));
EXPECT_EQ(1U, origins.count(kOrigin2));
EXPECT_EQ(0U, origins.count(kOrigin3));
// Update origin1's mod time but for persistent storage.
EXPECT_TRUE(db.SetOriginLastModifiedTime(
kOrigin1, kPersistent, QuotaDatabase::TimeFromSqlValue(30)));
EXPECT_TRUE(db.SetOriginLastModifiedTime(kOrigin1, kPersistent,
base::Time::FromJavaTime(30)));
// Must have no effects on temporary origins info.
EXPECT_TRUE(db.GetOriginsModifiedBetween(kTemporary, &origins,
QuotaDatabase::TimeFromSqlValue(5),
base::Time::Max()));
EXPECT_TRUE(db.GetOriginsModifiedBetween(
kTemporary, &origins, base::Time::FromJavaTime(5), base::Time::Max()));
EXPECT_EQ(2U, origins.size());
EXPECT_EQ(0U, origins.count(kOrigin1));
EXPECT_EQ(1U, origins.count(kOrigin2));
EXPECT_EQ(1U, origins.count(kOrigin3));
// One more update for persistent origin2.
EXPECT_TRUE(db.SetOriginLastModifiedTime(
kOrigin2, kPersistent, QuotaDatabase::TimeFromSqlValue(40)));
EXPECT_TRUE(db.SetOriginLastModifiedTime(kOrigin2, kPersistent,
base::Time::FromJavaTime(40)));
EXPECT_TRUE(db.GetOriginsModifiedBetween(
kPersistent, &origins, QuotaDatabase::TimeFromSqlValue(25),
base::Time::Max()));
EXPECT_TRUE(db.GetOriginsModifiedBetween(kPersistent, &origins,
base::Time::FromJavaTime(25),
base::Time::Max()));
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.GetOriginsModifiedBetween(
kPersistent, &origins, QuotaDatabase::TimeFromSqlValue(35),
base::Time::Max()));
EXPECT_TRUE(db.GetOriginsModifiedBetween(kPersistent, &origins,
base::Time::FromJavaTime(35),
base::Time::Max()));
EXPECT_EQ(1U, origins.size());
EXPECT_EQ(0U, origins.count(kOrigin1));
EXPECT_EQ(1U, origins.count(kOrigin2));
@ -302,22 +298,22 @@ class QuotaDatabaseTest : public testing::Test {
EXPECT_EQ(base::Time(), last_eviction_time);
// Report last eviction time for the test origins.
EXPECT_TRUE(db.SetOriginLastEvictionTime(
kOrigin1, kTemporary, QuotaDatabase::TimeFromSqlValue(10)));
EXPECT_TRUE(db.SetOriginLastEvictionTime(
kOrigin2, kTemporary, QuotaDatabase::TimeFromSqlValue(20)));
EXPECT_TRUE(db.SetOriginLastEvictionTime(
kOrigin3, kTemporary, QuotaDatabase::TimeFromSqlValue(30)));
EXPECT_TRUE(db.SetOriginLastEvictionTime(kOrigin1, kTemporary,
base::Time::FromJavaTime(10)));
EXPECT_TRUE(db.SetOriginLastEvictionTime(kOrigin2, kTemporary,
base::Time::FromJavaTime(20)));
EXPECT_TRUE(db.SetOriginLastEvictionTime(kOrigin3, kTemporary,
base::Time::FromJavaTime(30)));
EXPECT_TRUE(db.GetOriginLastEvictionTime(kOrigin1, kTemporary,
&last_eviction_time));
EXPECT_EQ(QuotaDatabase::TimeFromSqlValue(10), last_eviction_time);
EXPECT_EQ(base::Time::FromJavaTime(10), last_eviction_time);
EXPECT_TRUE(db.GetOriginLastEvictionTime(kOrigin2, kTemporary,
&last_eviction_time));
EXPECT_EQ(QuotaDatabase::TimeFromSqlValue(20), last_eviction_time);
EXPECT_EQ(base::Time::FromJavaTime(20), last_eviction_time);
EXPECT_TRUE(db.GetOriginLastEvictionTime(kOrigin3, kTemporary,
&last_eviction_time));
EXPECT_EQ(QuotaDatabase::TimeFromSqlValue(30), last_eviction_time);
EXPECT_EQ(base::Time::FromJavaTime(30), last_eviction_time);
// Delete last eviction times for the test origins.
EXPECT_TRUE(db.DeleteOriginLastEvictionTime(kOrigin1, kTemporary));
@ -497,10 +493,8 @@ 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(
4, QuotaDatabase::TimeToSqlValue(itr->last_modified_time));
statement.BindTime(3, itr->last_access_time);
statement.BindTime(4, itr->last_modified_time);
EXPECT_TRUE(statement.Run());
}
}