sql: Add precondition checks to sql::Connection.

This CL adds DCHECKs covering a few of the preconditions in
sql::Connection's API and removes sql::TimeSource, which duplicates
base::TickClock.

Bug: none
Change-Id: Ia62874ccf339cf2338e388a45a41d789e3ef89e3
Reviewed-on: https://chromium-review.googlesource.com/1143132
Commit-Queue: Victor Costan <pwnall@chromium.org>
Reviewed-by: Chris Mumford <cmumford@chromium.org>
Cr-Commit-Position: refs/heads/master@{#576589}
diff --git a/sql/connection.cc b/sql/connection.cc
index 5d8d2d2..6735720 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -9,8 +9,6 @@
 #include <stdint.h>
 #include <string.h>
 
-#include <utility>
-
 #include "base/debug/alias.h"
 #include "base/debug/dump_without_crashing.h"
 #include "base/files/file_path.h"
@@ -29,6 +27,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock.h"
+#include "base/time/default_tick_clock.h"
 #include "base/trace_event/memory_dump_manager.h"
 #include "build/build_config.h"
 #include "sql/connection_memory_dump_provider.h"
@@ -260,7 +259,7 @@
       autocommit_time_histogram_(nullptr),
       update_time_histogram_(nullptr),
       query_time_histogram_(nullptr),
-      clock_(new TimeSource()) {}
+      clock_(std::make_unique<base::DefaultTickClock>()) {}
 
 Connection::~Connection() {
   Close();
@@ -1237,9 +1236,9 @@
 
   // Collect the commit time manually, sql::Statement would register it as query
   // time only.
-  const base::TimeTicks before = Now();
+  const base::TimeTicks before = NowTicks();
   bool ret = commit.RunWithoutTimers();
-  const base::TimeDelta delta = Now() - before;
+  const base::TimeDelta delta = NowTicks() - before;
 
   RecordCommitTime(delta);
   RecordOneEvent(EVENT_COMMIT);
@@ -1298,7 +1297,7 @@
     sqlite3_stmt* stmt = nullptr;
     const char *leftover_sql;
 
-    const base::TimeTicks before = Now();
+    const base::TimeTicks before = NowTicks();
     rc = sqlite3_prepare_v2(db_, sql, -1, &stmt, &leftover_sql);
     sql = leftover_sql;
 
@@ -1336,7 +1335,7 @@
       sql++;
     }
 
-    const base::TimeDelta delta = Now() - before;
+    const base::TimeDelta delta = NowTicks() - before;
     RecordTimeAndChanges(delta, read_only);
   }
 
@@ -1378,20 +1377,20 @@
   return Execute(sql);
 }
 
-bool Connection::HasCachedStatement(StatementID id) const {
-  return statement_cache_.find(id) != statement_cache_.end();
-}
-
 scoped_refptr<Connection::StatementRef> Connection::GetCachedStatement(
     StatementID id,
     const char* sql) {
   auto it = statement_cache_.find(id);
   if (it != statement_cache_.end()) {
-    // Statement is in the cache. It should still be active (we're the only
-    // one invalidating cached statements, and we'll remove it from the cache
-    // if we do that. Make sure we reset it before giving out the cached one in
-    // case it still has some stuff bound.
+    // Statement is in the cache. It should still be active. We're the only
+    // one invalidating cached statements, and we remove them from the cache
+    // when we do that.
     DCHECK(it->second->is_valid());
+
+    DCHECK_EQ(std::string(sql), std::string(sqlite3_sql(it->second->stmt())))
+        << "GetCachedStatement used with same ID but different SQL";
+
+    // Reset the statement so it can be reused.
     sqlite3_reset(it->second->stmt());
     return it->second;
   }
@@ -1411,7 +1410,7 @@
     sql::Connection* tracking_db, const char* sql) const {
   AssertIOAllowed();
   DCHECK(sql);
-  DCHECK(!tracking_db || const_cast<Connection*>(tracking_db)==this);
+  DCHECK(!tracking_db || tracking_db == this);
 
   // Return inactive statement.
   if (!db_)
@@ -1438,7 +1437,7 @@
 std::string Connection::GetSchema() const {
   // The ORDER BY should not be necessary, but relying on organic
   // order for something like this is questionable.
-  const char* kSql =
+  static const char kSql[] =
       "SELECT type, name, tbl_name, sql "
       "FROM sqlite_master ORDER BY 1, 2, 3, 4";
   Statement statement(GetUntrackedStatement(kSql));
@@ -1795,9 +1794,9 @@
 
   // Collect the rollback time manually, sql::Statement would register it as
   // query time only.
-  const base::TimeTicks before = Now();
+  const base::TimeTicks before = NowTicks();
   rollback.RunWithoutTimers();
-  const base::TimeDelta delta = Now() - before;
+  const base::TimeDelta delta = NowTicks() - before;
 
   RecordUpdateTime(delta);
   RecordOneEvent(EVENT_ROLLBACK);
@@ -1824,6 +1823,7 @@
 
 void Connection::set_histogram_tag(const std::string& tag) {
   DCHECK(!is_open());
+
   histogram_tag_ = tag;
 }
 
@@ -1965,8 +1965,4 @@
          memory_dump_provider_->ReportMemoryUsage(pmd, dump_name);
 }
 
-base::TimeTicks TimeSource::Now() {
-  return base::TimeTicks::Now();
-}
-
 }  // namespace sql
diff --git a/sql/connection.h b/sql/connection.h
index 4c87858..055c290 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -10,6 +10,7 @@
 #include <memory>
 #include <set>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/callback.h"
@@ -20,7 +21,7 @@
 #include "base/memory/ref_counted.h"
 #include "base/sequence_checker.h"
 #include "base/threading/thread_restrictions.h"
-#include "base/time/time.h"
+#include "base/time/tick_clock.h"
 #include "sql/sql_export.h"
 #include "sql/statement_id.h"
 
@@ -32,8 +33,8 @@
 class HistogramBase;
 namespace trace_event {
 class ProcessMemoryDump;
-}
-}
+}  // namespace trace_event
+}  // namespace base
 
 namespace sql {
 
@@ -47,24 +48,18 @@
 class ScopedErrorExpecter;
 class ScopedScalarFunction;
 class ScopedMockTimeSource;
-}
+}  // namespace test
 
 class Connection;
 
-// Abstract the source of timing information for metrics (RecordCommitTime, etc)
-// to allow testing control.
-class SQL_EXPORT TimeSource {
- public:
-  TimeSource() {}
-  virtual ~TimeSource() {}
-
-  // Return the current time (by default base::TimeTicks::Now()).
-  virtual base::TimeTicks Now();
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(TimeSource);
-};
-
+// Handle to an open SQLite database.
+//
+// Instances of this class are thread-unsafe and DCHECK that they are accessed
+// on the same sequence.
+//
+// TODO(pwnall): This should be renamed to Database. Class instances are
+// typically named "db_" / "db", and the class' equivalents in other systems
+// used by Chrome are named LevelDB::DB and blink::IDBDatabase.
 class SQL_EXPORT Connection {
  private:
   class StatementRef;  // Forward declaration, see real one below.
@@ -84,12 +79,22 @@
   // From sqlite.org: "The page size must be a power of two greater than or
   // equal to 512 and less than or equal to SQLITE_MAX_PAGE_SIZE. The maximum
   // value for SQLITE_MAX_PAGE_SIZE is 32768."
-  void set_page_size(int page_size) { page_size_ = page_size; }
+  void set_page_size(int page_size) {
+    DCHECK(!page_size || (page_size >= 512));
+    DCHECK(!page_size || !(page_size & (page_size - 1)))
+        << "page_size must be a power of two";
+
+    page_size_ = page_size;
+  }
 
   // Sets the number of pages that will be cached in memory by sqlite. The
   // total cache size in bytes will be page_size * cache_size. This must be
   // called before Open() to have an effect.
-  void set_cache_size(int cache_size) { cache_size_ = cache_size; }
+  void set_cache_size(int cache_size) {
+    DCHECK_GE(cache_size, 0);
+
+    cache_size_ = cache_size;
+  }
 
   // Call to put the database in exclusive locking mode. There is no "back to
   // normal" flag because of some additional requirements sqlite puts on this
@@ -105,8 +110,8 @@
 
   // Call to cause Open() to restrict access permissions of the
   // database file to only the owner.
-  // TODO(shess): Currently only supported on OS_POSIX, is a noop on
-  // other platforms.
+  //
+  // This is only supported on OS_POSIX and is a noop on other platforms.
   void set_restrict_to_user() { restrict_to_user_ = true; }
 
   // Call to use alternative status-tracking for mmap.  Usually this is tracked
@@ -114,7 +119,7 @@
   // TODO(shess): Maybe just have all databases use the alt option?
   void set_mmap_alt_status() { mmap_alt_status_ = true; }
 
-  // Call to opt out of memory-mapped file I/O.
+  // Opt out of memory-mapped file I/O.
   void set_mmap_disabled() { mmap_disabled_ = true; }
 
   // Set an error-handling callback.  On errors, the error number (and
@@ -126,12 +131,8 @@
   void set_error_callback(const ErrorCallback& callback) {
     error_callback_ = callback;
   }
-  bool has_error_callback() const {
-    return !error_callback_.is_null();
-  }
-  void reset_error_callback() {
-    error_callback_.Reset();
-  }
+  bool has_error_callback() const { return !error_callback_.is_null(); }
+  void reset_error_callback() { error_callback_.Reset(); }
 
   // Set this to enable additional per-connection histogramming.  Must be called
   // before Open().
@@ -191,9 +192,7 @@
     EVENT_MAX_VALUE
   };
   void RecordEvent(Events event, size_t count);
-  void RecordOneEvent(Events event) {
-    RecordEvent(event, 1);
-  }
+  void RecordOneEvent(Events event) { RecordEvent(event, 1); }
 
   // Run "PRAGMA integrity_check" and post each line of
   // results into |messages|.  Returns the success of running the
@@ -232,7 +231,7 @@
   bool OpenTemporary() WARN_UNUSED_RESULT;
 
   // Returns true if the database has been successfully opened.
-  bool is_open() const { return !!db_; }
+  bool is_open() const { return static_cast<bool>(db_); }
 
   // Closes the database. This is automatically performed on destruction for
   // you, but this allows you to close the database early. You must not call
@@ -372,12 +371,6 @@
   // Like Execute(), but returns the error code given by SQLite.
   int ExecuteAndReturnErrorCode(const char* sql) WARN_UNUSED_RESULT;
 
-  // Returns true if we have a statement with the given identifier already
-  // cached. This is normally not necessary to call, but can be useful if the
-  // caller has to dynamically build up SQL to avoid doing so if it's already
-  // cached.
-  bool HasCachedStatement(StatementID id) const;
-
   // Returns a statement for the given SQL using the statement cache. It can
   // take a nontrivial amount of work to parse and compile a statement, so
   // keeping commonly-used ones around for future use is important for
@@ -466,6 +459,16 @@
   // crash server.
   void ReportDiagnosticInfo(int extended_error, Statement* stmt);
 
+  // Helper to return the current time from the time source.
+  base::TimeTicks NowTicks() const { return clock_->NowTicks(); }
+
+  // Intended for tests to inject a mock time source.
+  //
+  // Inlined to avoid generating code in the production binary.
+  inline void set_clock_for_testing(std::unique_ptr<base::TickClock> clock) {
+    clock_ = std::move(clock);
+  }
+
  private:
   // For recovery module.
   friend class Recovery;
@@ -481,6 +484,7 @@
   friend class test::ScopedScalarFunction;
   friend class test::ScopedMockTimeSource;
 
+  FRIEND_TEST_ALL_PREFIXES(SQLConnectionTest, CachedStatement);
   FRIEND_TEST_ALL_PREFIXES(SQLConnectionTest, CollectDiagnosticInfo);
   FRIEND_TEST_ALL_PREFIXES(SQLConnectionTest, GetAppropriateMmapSize);
   FRIEND_TEST_ALL_PREFIXES(SQLConnectionTest, GetAppropriateMmapSizeAltStatus);
@@ -658,11 +662,6 @@
   // EVENT_CHANGES_AUTOCOMMIT or EVENT_CHANGES_COMMIT.
   void RecordTimeAndChanges(const base::TimeDelta& delta, bool read_only);
 
-  // Helper to return the current time from the time source.
-  base::TimeTicks Now() {
-    return clock_->Now();
-  }
-
   // Release page-cache memory if memory-mapped I/O is enabled and the database
   // was changed.  Passing true for |implicit_change_performed| allows
   // overriding the change detection for cases like DDL (CREATE, DROP, etc),
@@ -788,7 +787,7 @@
 
   // Source for timing information, provided to allow tests to inject time
   // changes.
-  std::unique_ptr<TimeSource> clock_;
+  std::unique_ptr<base::TickClock> clock_;
 
   // Stores the dump provider object when db is open.
   std::unique_ptr<ConnectionMemoryDumpProvider> memory_dump_provider_;
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index 2a91731..2d32bfa 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -13,6 +13,7 @@
 #include "base/macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/test/metrics/histogram_tester.h"
+#include "base/test/simple_test_tick_clock.h"
 #include "base/trace_event/process_memory_dump.h"
 #include "build/build_config.h"
 #include "sql/connection.h"
@@ -29,60 +30,6 @@
 namespace sql {
 namespace test {
 
-// Replaces the database time source with an object that steps forward 1ms on
-// each check, and which can be jumped forward an arbitrary amount of time
-// programmatically.
-class ScopedMockTimeSource {
- public:
-  ScopedMockTimeSource(Connection& db)
-      : db_(db),
-        delta_(base::TimeDelta::FromMilliseconds(1)) {
-    // Save the current source and replace it.
-    save_.swap(db_.clock_);
-    db_.clock_.reset(new MockTimeSource(*this));
-  }
-  ~ScopedMockTimeSource() {
-    // Put original source back.
-    db_.clock_.swap(save_);
-  }
-
-  void adjust(const base::TimeDelta& delta) {
-    current_time_ += delta;
-  }
-
- private:
-  class MockTimeSource : public TimeSource {
-   public:
-    MockTimeSource(ScopedMockTimeSource& owner)
-        : owner_(owner) {
-    }
-    ~MockTimeSource() override = default;
-
-    base::TimeTicks Now() override {
-      base::TimeTicks ret(owner_.current_time_);
-      owner_.current_time_ += owner_.delta_;
-      return ret;
-    }
-
-   private:
-    ScopedMockTimeSource& owner_;
-    DISALLOW_COPY_AND_ASSIGN(MockTimeSource);
-  };
-
-  Connection& db_;
-
-  // Saves original source from |db_|.
-  std::unique_ptr<TimeSource> save_;
-
-  // Current time returned by mock.
-  base::TimeTicks current_time_;
-
-  // How far to jump on each Now() call.
-  base::TimeDelta delta_;
-
-  DISALLOW_COPY_AND_ASSIGN(ScopedMockTimeSource);
-};
-
 // Allow a test to add a SQLite function in a scoped context.
 class ScopedScalarFunction {
  public:
@@ -224,18 +171,20 @@
 #endif  // defined(OS_POSIX)
 
 // SQLite function to adjust mock time by |argv[0]| milliseconds.
-void sqlite_adjust_millis(sql::test::ScopedMockTimeSource* time_mock,
+void sqlite_adjust_millis(base::SimpleTestTickClock* mock_clock,
                           sqlite3_context* context,
-                          int argc, sqlite3_value** argv) {
-  int64_t milliseconds = argc > 0 ? sqlite3_value_int64(argv[0]) : 1000;
-  time_mock->adjust(base::TimeDelta::FromMilliseconds(milliseconds));
+                          int argc,
+                          sqlite3_value** argv) {
+  CHECK_EQ(argc, 1);
+  int64_t milliseconds = sqlite3_value_int64(argv[0]);
+  mock_clock->Advance(base::TimeDelta::FromMilliseconds(milliseconds));
   sqlite3_result_int64(context, milliseconds);
 }
 
 // Adjust mock time by |milliseconds| on commit.
-int adjust_commit_hook(sql::test::ScopedMockTimeSource* time_mock,
+int adjust_commit_hook(base::SimpleTestTickClock* mock_clock,
                        int64_t milliseconds) {
-  time_mock->adjust(base::TimeDelta::FromMilliseconds(milliseconds));
+  mock_clock->Advance(base::TimeDelta::FromMilliseconds(milliseconds));
   return SQLITE_OK;
 }
 
@@ -271,34 +220,57 @@
 
 TEST_F(SQLConnectionTest, CachedStatement) {
   sql::StatementID id1 = SQL_FROM_HERE;
+  sql::StatementID id2 = SQL_FROM_HERE;
+  static const char kId1Sql[] = "SELECT a FROM foo";
+  static const char kId2Sql[] = "SELECT b FROM foo";
 
   ASSERT_TRUE(db().Execute("CREATE TABLE foo (a, b)"));
   ASSERT_TRUE(db().Execute("INSERT INTO foo(a, b) VALUES (12, 13)"));
 
-  // Create a new cached statement.
+  sqlite3_stmt* raw_id1_statement;
+  sqlite3_stmt* raw_id2_statement;
   {
-    sql::Statement s(db().GetCachedStatement(id1, "SELECT a FROM foo"));
-    ASSERT_TRUE(s.is_valid());
+    scoped_refptr<sql::Connection::StatementRef> ref_from_id1 =
+        db().GetCachedStatement(id1, kId1Sql);
+    raw_id1_statement = ref_from_id1->stmt();
 
-    ASSERT_TRUE(s.Step());
-    EXPECT_EQ(12, s.ColumnInt(0));
+    sql::Statement from_id1(std::move(ref_from_id1));
+    ASSERT_TRUE(from_id1.is_valid());
+    ASSERT_TRUE(from_id1.Step());
+    EXPECT_EQ(12, from_id1.ColumnInt(0));
+
+    scoped_refptr<sql::Connection::StatementRef> ref_from_id2 =
+        db().GetCachedStatement(id2, kId2Sql);
+    raw_id2_statement = ref_from_id2->stmt();
+    EXPECT_NE(raw_id1_statement, raw_id2_statement);
+
+    sql::Statement from_id2(std::move(ref_from_id2));
+    ASSERT_TRUE(from_id2.is_valid());
+    ASSERT_TRUE(from_id2.Step());
+    EXPECT_EQ(13, from_id2.ColumnInt(0));
   }
 
-  // The statement should be cached still.
-  EXPECT_TRUE(db().HasCachedStatement(id1));
-
   {
-    // Get the same statement using different SQL. This should ignore our
-    // SQL and use the cached one (so it will be valid).
-    sql::Statement s(db().GetCachedStatement(id1, "something invalid("));
-    ASSERT_TRUE(s.is_valid());
+    scoped_refptr<sql::Connection::StatementRef> ref_from_id1 =
+        db().GetCachedStatement(id1, kId1Sql);
+    EXPECT_EQ(raw_id1_statement, ref_from_id1->stmt())
+        << "statement was not cached";
 
-    ASSERT_TRUE(s.Step());
-    EXPECT_EQ(12, s.ColumnInt(0));
+    sql::Statement from_id1(std::move(ref_from_id1));
+    ASSERT_TRUE(from_id1.is_valid());
+    ASSERT_TRUE(from_id1.Step()) << "cached statement was not reset";
+    EXPECT_EQ(12, from_id1.ColumnInt(0));
+
+    scoped_refptr<sql::Connection::StatementRef> ref_from_id2 =
+        db().GetCachedStatement(id2, kId2Sql);
+    EXPECT_EQ(raw_id2_statement, ref_from_id2->stmt())
+        << "statement was not cached";
+
+    sql::Statement from_id2(std::move(ref_from_id2));
+    ASSERT_TRUE(from_id2.is_valid());
+    ASSERT_TRUE(from_id2.Step()) << "cached statement was not reset";
+    EXPECT_EQ(13, from_id2.ColumnInt(0));
   }
-
-  // Make sure other statements aren't marked as cached.
-  EXPECT_FALSE(db().HasCachedStatement(SQL_FROM_HERE));
 }
 
 TEST_F(SQLConnectionTest, IsSQLValidTest) {
@@ -1296,7 +1268,10 @@
   db().set_histogram_tag("Test");
   ASSERT_TRUE(db().OpenInMemory());
 
-  sql::test::ScopedMockTimeSource time_mock(db());
+  auto mock_clock = std::make_unique<base::SimpleTestTickClock>();
+  // Retaining the pointer is safe because the connection keeps it alive.
+  base::SimpleTestTickClock* mock_clock_ptr = mock_clock.get();
+  db().set_clock_for_testing(std::move(mock_clock));
 
   const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
   EXPECT_TRUE(db().Execute(kCreateSql));
@@ -1304,7 +1279,7 @@
   // Function to inject pauses into statements.
   sql::test::ScopedScalarFunction scoper(
       db(), "milliadjust", 1,
-      base::BindRepeating(&sqlite_adjust_millis, &time_mock));
+      base::BindRepeating(&sqlite_adjust_millis, mock_clock_ptr));
 
   base::HistogramTester tester;
 
@@ -1313,8 +1288,7 @@
   std::unique_ptr<base::HistogramSamples> samples(
       tester.GetHistogramSamplesSinceCreation(kQueryTime));
   ASSERT_TRUE(samples);
-  // 10 for the adjust, 1 for the measurement.
-  EXPECT_EQ(11, samples->sum());
+  EXPECT_EQ(10, samples->sum());
 
   samples = tester.GetHistogramSamplesSinceCreation(kUpdateTime);
   EXPECT_EQ(0, samples->sum());
@@ -1335,7 +1309,10 @@
   db().set_histogram_tag("Test");
   ASSERT_TRUE(db().OpenInMemory());
 
-  sql::test::ScopedMockTimeSource time_mock(db());
+  auto mock_clock = std::make_unique<base::SimpleTestTickClock>();
+  // Retaining the pointer is safe because the connection keeps it alive.
+  base::SimpleTestTickClock* mock_clock_ptr = mock_clock.get();
+  db().set_clock_for_testing(std::move(mock_clock));
 
   const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
   EXPECT_TRUE(db().Execute(kCreateSql));
@@ -1343,7 +1320,7 @@
   // Function to inject pauses into statements.
   sql::test::ScopedScalarFunction scoper(
       db(), "milliadjust", 1,
-      base::BindRepeating(&sqlite_adjust_millis, &time_mock));
+      base::BindRepeating(&sqlite_adjust_millis, mock_clock_ptr));
 
   base::HistogramTester tester;
 
@@ -1352,21 +1329,18 @@
   std::unique_ptr<base::HistogramSamples> samples(
       tester.GetHistogramSamplesSinceCreation(kQueryTime));
   ASSERT_TRUE(samples);
-  // 10 for the adjust, 1 for the measurement.
-  EXPECT_EQ(11, samples->sum());
+  EXPECT_EQ(10, samples->sum());
 
   samples = tester.GetHistogramSamplesSinceCreation(kUpdateTime);
   ASSERT_TRUE(samples);
-  // 10 for the adjust, 1 for the measurement.
-  EXPECT_EQ(11, samples->sum());
+  EXPECT_EQ(10, samples->sum());
 
   samples = tester.GetHistogramSamplesSinceCreation(kCommitTime);
   EXPECT_EQ(0, samples->sum());
 
   samples = tester.GetHistogramSamplesSinceCreation(kAutoCommitTime);
   ASSERT_TRUE(samples);
-  // 10 for the adjust, 1 for the measurement.
-  EXPECT_EQ(11, samples->sum());
+  EXPECT_EQ(10, samples->sum());
 }
 
 // Update with explicit transaction allocates time to QueryTime, UpdateTime, and
@@ -1378,7 +1352,10 @@
   db().set_histogram_tag("Test");
   ASSERT_TRUE(db().OpenInMemory());
 
-  sql::test::ScopedMockTimeSource time_mock(db());
+  auto mock_clock = std::make_unique<base::SimpleTestTickClock>();
+  // Retaining the pointer is safe because the connection keeps it alive.
+  base::SimpleTestTickClock* mock_clock_ptr = mock_clock.get();
+  db().set_clock_for_testing(std::move(mock_clock));
 
   const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
   EXPECT_TRUE(db().Execute(kCreateSql));
@@ -1386,39 +1363,35 @@
   // Function to inject pauses into statements.
   sql::test::ScopedScalarFunction scoper(
       db(), "milliadjust", 1,
-      base::BindRepeating(&sqlite_adjust_millis, &time_mock));
+      base::BindRepeating(&sqlite_adjust_millis, mock_clock_ptr));
 
   base::HistogramTester tester;
 
   {
     // Make the commit slow.
     sql::test::ScopedCommitHook scoped_hook(
-        db(), base::BindRepeating(adjust_commit_hook, &time_mock, 100));
+        db(), base::BindRepeating(adjust_commit_hook, mock_clock_ptr, 1000));
     ASSERT_TRUE(db().BeginTransaction());
     EXPECT_TRUE(db().Execute(
         "INSERT INTO foo VALUES (11, milliadjust(10))"));
-    EXPECT_TRUE(db().Execute(
-        "UPDATE foo SET value = milliadjust(10) WHERE id = 11"));
+    EXPECT_TRUE(
+        db().Execute("UPDATE foo SET value = milliadjust(100) WHERE id = 11"));
     EXPECT_TRUE(db().CommitTransaction());
   }
 
   std::unique_ptr<base::HistogramSamples> samples(
       tester.GetHistogramSamplesSinceCreation(kQueryTime));
   ASSERT_TRUE(samples);
-  // 10 for insert adjust, 10 for update adjust, 100 for commit adjust, 1 for
-  // measuring each of BEGIN, INSERT, UPDATE, and COMMIT.
-  EXPECT_EQ(124, samples->sum());
+  // 10 for insert, 100 for update, 1000 for commit
+  EXPECT_EQ(1110, samples->sum());
 
   samples = tester.GetHistogramSamplesSinceCreation(kUpdateTime);
   ASSERT_TRUE(samples);
-  // 10 for insert adjust, 10 for update adjust, 100 for commit adjust, 1 for
-  // measuring each of INSERT, UPDATE, and COMMIT.
-  EXPECT_EQ(123, samples->sum());
+  EXPECT_EQ(1110, samples->sum());
 
   samples = tester.GetHistogramSamplesSinceCreation(kCommitTime);
   ASSERT_TRUE(samples);
-  // 100 for commit adjust, 1 for measuring COMMIT.
-  EXPECT_EQ(101, samples->sum());
+  EXPECT_EQ(1000, samples->sum());
 
   samples = tester.GetHistogramSamplesSinceCreation(kAutoCommitTime);
   EXPECT_EQ(0, samples->sum());
diff --git a/sql/statement.cc b/sql/statement.cc
index 3f0dbbfa..ddaa4b7 100644
--- a/sql/statement.cc
+++ b/sql/statement.cc
@@ -67,9 +67,9 @@
     if (!timer_flag) {
       ret = sqlite3_step(ref_->stmt());
     } else {
-      const base::TimeTicks before = ref_->connection()->Now();
+      const base::TimeTicks before = ref_->connection()->NowTicks();
       ret = sqlite3_step(ref_->stmt());
-      const base::TimeTicks after = ref_->connection()->Now();
+      const base::TimeTicks after = ref_->connection()->NowTicks();
       const bool read_only = !!sqlite3_stmt_readonly(ref_->stmt());
       ref_->connection()->RecordTimeAndChanges(after - before, read_only);
     }