[sql] Refactor to remove use of const_cast<>.

The comments in these cases used incorrect reasoning for why the
const_cast<> was safe.  Whether the SQL statements modify the database
is irrelevant.  Add an internal helper to allow them to correctly
accomplish things in a const situation.

BUG=none
TEST=none


Review URL: https://chromiumcodereview.appspot.com/10736042

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@148031 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 35 insertions, 12 deletions

diff --git a/sql/connection.cc b/sql/connection.cc
index a9c4974..2d7baf2 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -61,6 +61,11 @@ Connection::StatementRef::StatementRef()
       stmt_(NULL) {
 }
 
+Connection::StatementRef::StatementRef(sqlite3_stmt* stmt)
+    : connection_(NULL),
+      stmt_(stmt) {
+}
+
 Connection::StatementRef::StatementRef(Connection* connection,
                                        sqlite3_stmt* stmt)
     : connection_(connection),
@@ -343,17 +348,32 @@ scoped_refptr<Connection::StatementRef> Connection::GetCachedStatement(
 scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
     const char* sql) {
   if (!db_)
-    return new StatementRef(this, NULL);  // Return inactive statement.
+    return new StatementRef();  // Return inactive statement.
 
   sqlite3_stmt* stmt = NULL;
   if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK) {
     // This is evidence of a syntax error in the incoming SQL.
     DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
-    return new StatementRef(this, NULL);
+    return new StatementRef();
   }
   return new StatementRef(this, stmt);
 }
 
+scoped_refptr<Connection::StatementRef> Connection::GetUntrackedStatement(
+    const char* sql) const {
+  if (!db_)
+    return new StatementRef();  // Return inactive statement.
+
+  sqlite3_stmt* stmt = NULL;
+  int rc = sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL);
+  if (rc != SQLITE_OK) {
+    // This is evidence of a syntax error in the incoming SQL.
+    DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
+    return new StatementRef();
+  }
+  return new StatementRef(stmt);
+}
+
 bool Connection::IsSQLValid(const char* sql) {
   sqlite3_stmt* stmt = NULL;
   if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK)
@@ -373,11 +393,8 @@ bool Connection::DoesIndexExist(const char* index_name) const {
 
 bool Connection::DoesTableOrIndexExist(
     const char* name, const char* type) const {
-  // GetUniqueStatement can't be const since statements may modify the
-  // database, but we know ours doesn't modify it, so the cast is safe.
-  Statement statement(const_cast<Connection*>(this)->GetUniqueStatement(
-      "SELECT name FROM sqlite_master "
-      "WHERE type=? AND name=?"));
+  const char* kSql = "SELECT name FROM sqlite_master WHERE type=? AND name=?";
+  Statement statement(GetUntrackedStatement(kSql));
   statement.BindString(0, type);
   statement.BindString(1, name);
 
@@ -390,10 +407,7 @@ bool Connection::DoesColumnExist(const char* table_name,
   sql.append(table_name);
   sql.append(")");
 
-  // Our SQL is non-mutating, so this cast is OK.
-  Statement statement(const_cast<Connection*>(this)->GetUniqueStatement(
-      sql.c_str()));
-
+  Statement statement(GetUntrackedStatement(sql.c_str()));
   while (statement.Step()) {
     if (!statement.ColumnString(1).compare(column_name))
       return true;
diff --git a/sql/connection.h b/sql/connection.h
index fba0326..e5c9469 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2011 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -338,6 +338,7 @@ class SQL_EXPORT Connection {
    public:
     // Default constructor initializes to an invalid statement.
     StatementRef();
+    explicit StatementRef(sqlite3_stmt* stmt);
     StatementRef(Connection* connection, sqlite3_stmt* stmt);
 
     // When true, the statement can be used.
@@ -387,6 +388,14 @@ class SQL_EXPORT Connection {
   bool ExecuteWithTimeout(const char* sql, base::TimeDelta ms_timeout)
       WARN_UNUSED_RESULT;
 
+  // Internal helper for const functions.  Like GetUniqueStatement(),
+  // except the statement is not entered into open_statements_,
+  // allowing this function to be const.  Open statements can block
+  // closing the database, so only use in cases where the last ref is
+  // released before close could be called (which should always be the
+  // case for const functions).
+  scoped_refptr<StatementRef> GetUntrackedStatement(const char* sql) const;
+
   // The actual sqlite database. Will be NULL before Init has been called or if
   // Init resulted in an error.
   sqlite3* db_;