Remove ref counting on sql::ErrorDelegate

BUG=151841
Test=None

R=shess
TBR=jamesr,erikwright

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

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

5 files changed, 70 insertions, 68 deletions

diff --git a/sql/connection.cc b/sql/connection.cc
index ea3dacb..94a8cc0 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -50,9 +50,6 @@ bool StatementID::operator<(const StatementID& other) const {
   return strcmp(str_, other.str_) < 0;
 }
 
-ErrorDelegate::ErrorDelegate() {
-}
-
 ErrorDelegate::~ErrorDelegate() {
 }
 
@@ -102,7 +99,8 @@ Connection::Connection()
       exclusive_locking_(false),
       transaction_nesting_(0),
       needs_rollback_(false),
-      in_memory_(false) {
+      in_memory_(false),
+      error_delegate_(NULL) {
 }
 
 Connection::~Connection() {
diff --git a/sql/connection.h b/sql/connection.h
index 65020a0..1aef064 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -12,6 +12,7 @@
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/time.h"
 #include "sql/sql_export.h"
@@ -78,9 +79,9 @@ class Connection;
 // the OnError() callback.
 // The tipical usage is to centralize the code designed to handle database
 // corruption, low-level IO errors or locking violations.
-class SQL_EXPORT ErrorDelegate : public base::RefCounted<ErrorDelegate> {
+class SQL_EXPORT ErrorDelegate {
  public:
-  ErrorDelegate();
+  virtual ~ErrorDelegate();
 
   // |error| is an sqlite result code as seen in sqlite\preprocessed\sqlite3.h
   // |connection| is db connection where the error happened and |stmt| is
@@ -94,11 +95,6 @@ class SQL_EXPORT ErrorDelegate : public base::RefCounted<ErrorDelegate> {
   // re-tried then returning SQLITE_OK is appropiate; otherwise is recomended
   // that you return the original |error| or the appropiae error code.
   virtual int OnError(int error, Connection* connection, Statement* stmt) = 0;
-
- protected:
-  friend class base::RefCounted<ErrorDelegate>;
-
-  virtual ~ErrorDelegate();
 };
 
 class SQL_EXPORT Connection {
@@ -142,8 +138,9 @@ class SQL_EXPORT Connection {
   // Sets the object that will handle errors. Recomended that it should be set
   // before calling Open(). If not set, the default is to ignore errors on
   // release and assert on debug builds.
+  // Takes ownership of |delegate|.
   void set_error_delegate(ErrorDelegate* delegate) {
-    error_delegate_ = delegate;
+    error_delegate_.reset(delegate);
   }
 
   // Initialization ------------------------------------------------------------
@@ -445,7 +442,7 @@ class SQL_EXPORT Connection {
 
   // This object handles errors resulting from all forms of executing sqlite
   // commands or statements. It can be null which means default handling.
-  scoped_refptr<ErrorDelegate> error_delegate_;
+  scoped_ptr<ErrorDelegate> error_delegate_;
 
   DISALLOW_COPY_AND_ASSIGN(Connection);
 };
diff --git a/sql/diagnostic_error_delegate.h b/sql/diagnostic_error_delegate.h
index e0c42ba..4b8ce32 100644
--- a/sql/diagnostic_error_delegate.h
+++ b/sql/diagnostic_error_delegate.h
@@ -24,6 +24,8 @@ namespace sql {
 template <class UniqueT>
 class DiagnosticErrorDelegate : public ErrorDelegate {
  public:
+  DiagnosticErrorDelegate() {}
+  virtual ~DiagnosticErrorDelegate() {}
 
   virtual int OnError(int error, Connection* connection,
                       Statement* stmt) {
@@ -43,6 +45,8 @@ class DiagnosticErrorDelegate : public ErrorDelegate {
     // 26 currently but 50 gives them room to grow.
     UMA_HISTOGRAM_ENUMERATION(UniqueT::name(), error, 50);
   }
+
+ DISALLOW_COPY_AND_ASSIGN(DiagnosticErrorDelegate);
 };
 
 }  // namespace sql
diff --git a/sql/sqlite_features_unittest.cc b/sql/sqlite_features_unittest.cc
index d3ad7b0..7e750e9 100644
--- a/sql/sqlite_features_unittest.cc
+++ b/sql/sqlite_features_unittest.cc
@@ -15,65 +15,63 @@
 
 namespace {
 
-
 class StatementErrorHandler : public sql::ErrorDelegate {
  public:
-  StatementErrorHandler() : error_(SQLITE_OK) {}
+  StatementErrorHandler(int* error, std::string* sql_text)
+    : error_(error),
+      sql_text_(sql_text) {}
+
+  virtual ~StatementErrorHandler() {}
 
   virtual int OnError(int error, sql::Connection* connection,
                       sql::Statement* stmt) OVERRIDE {
-    error_ = error;
+    *error_ = error;
     const char* sql_txt = stmt ? stmt->GetSQLStatement() : NULL;
-    sql_text_ = sql_txt ? sql_txt : "no statement available";
+    *sql_text_ = sql_txt ? sql_txt : "no statement available";
     return error;
   }
 
-  int error() const { return error_; }
-
-  void reset_error() {
-    sql_text_.clear();
-    error_ = SQLITE_OK;
-  }
-
-  const char* sql_statement() const { return sql_text_.c_str(); }
-
- protected:
-  virtual ~StatementErrorHandler() {}
-
  private:
-  int error_;
-  std::string sql_text_;
+  int* error_;
+  std::string* sql_text_;
+
+ DISALLOW_COPY_AND_ASSIGN(StatementErrorHandler);
 };
 
 class SQLiteFeaturesTest : public testing::Test {
  public:
-  SQLiteFeaturesTest() : error_handler_(new StatementErrorHandler) {}
+  SQLiteFeaturesTest() : error_(SQLITE_OK) {}
 
   void SetUp() {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     ASSERT_TRUE(db_.Open(temp_dir_.path().AppendASCII("SQLStatementTest.db")));
 
-    // The |error_handler_| will be called if any sqlite statement operation
-    // returns an error code.
-    db_.set_error_delegate(error_handler_);
+    // The error delegate will set |error_| and |sql_text_| when any sqlite
+    // statement operation returns an error code.
+    db_.set_error_delegate(new StatementErrorHandler(&error_, &sql_text_));
   }
 
   void TearDown() {
     // If any error happened the original sql statement can be found in
-    // error_handler_->sql_statement().
-    EXPECT_EQ(SQLITE_OK, error_handler_->error());
+    // |sql_text_|.
+    EXPECT_EQ(SQLITE_OK, error_);
     db_.Close();
   }
 
   sql::Connection& db() { return db_; }
 
-  int sqlite_error() const { return error_handler_->error(); }
-  void reset_error() const { error_handler_->reset_error(); }
+  int sqlite_error() const {
+    return error_;
+  }
 
  private:
   ScopedTempDir temp_dir_;
   sql::Connection db_;
-  scoped_refptr<StatementErrorHandler> error_handler_;
+
+  // The error code of the most recent error.
+  int error_;
+  // Original statement which has caused the error.
+  std::string sql_text_;
 };
 
 // Do not include fts1 support, it is not useful, and nobody is
diff --git a/sql/statement_unittest.cc b/sql/statement_unittest.cc
index 963c45e..a7a23d8 100644
--- a/sql/statement_unittest.cc
+++ b/sql/statement_unittest.cc
@@ -11,66 +11,71 @@
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/sqlite/sqlite3.h"
 
+namespace {
+
 class StatementErrorHandler : public sql::ErrorDelegate {
  public:
-  StatementErrorHandler() : error_(SQLITE_OK) {}
+  StatementErrorHandler(int* error, std::string* sql_text)
+    : error_(error),
+      sql_text_(sql_text) {}
+
+  virtual ~StatementErrorHandler() {}
 
   virtual int OnError(int error, sql::Connection* connection,
                       sql::Statement* stmt) OVERRIDE {
-    error_ = error;
+    *error_ = error;
     const char* sql_txt = stmt ? stmt->GetSQLStatement() : NULL;
-    sql_text_ = sql_txt ? sql_txt : "no statement available";
+    *sql_text_ = sql_txt ? sql_txt : "no statement available";
     return error;
   }
 
-  int error() const { return error_; }
-
-  void reset_error() {
-    sql_text_.clear();
-    error_ = SQLITE_OK;
-  }
-
-  const char* sql_statement() const { return sql_text_.c_str(); }
-
- protected:
-  virtual ~StatementErrorHandler() {}
-
  private:
-  int error_;
-  std::string sql_text_;
+  int* error_;
+  std::string* sql_text_;
+
+ DISALLOW_COPY_AND_ASSIGN(StatementErrorHandler);
 };
 
 class SQLStatementTest : public testing::Test {
  public:
-  SQLStatementTest() : error_handler_(new StatementErrorHandler) {}
+  SQLStatementTest() : error_(SQLITE_OK) {}
 
   void SetUp() {
     ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
     ASSERT_TRUE(db_.Open(temp_dir_.path().AppendASCII("SQLStatementTest.db")));
-
-    // The |error_handler_| will be called if any sqlite statement operation
-    // returns an error code.
-    db_.set_error_delegate(error_handler_);
+    // The error delegate will set |error_| and |sql_text_| when any sqlite
+    // statement operation returns an error code.
+    db_.set_error_delegate(new StatementErrorHandler(&error_, &sql_text_));
   }
 
   void TearDown() {
     // If any error happened the original sql statement can be found in
-    // error_handler_->sql_statement().
-    EXPECT_EQ(SQLITE_OK, error_handler_->error());
+    // |sql_text_|.
+    EXPECT_EQ(SQLITE_OK, error_);
     db_.Close();
   }
 
   sql::Connection& db() { return db_; }
 
-  int sqlite_error() const { return error_handler_->error(); }
-  void reset_error() const { error_handler_->reset_error(); }
+  int sqlite_error() const { return error_; }
+
+  void ResetError() {
+    error_ = SQLITE_OK;
+    sql_text_.clear();
+  }
 
  private:
   ScopedTempDir temp_dir_;
   sql::Connection db_;
-  scoped_refptr<StatementErrorHandler> error_handler_;
+
+  // The error code of the most recent error.
+  int error_;
+  // Original statement which caused the error.
+  std::string sql_text_;
 };
 
+}  // namespace
+
 TEST_F(SQLStatementTest, Assign) {
   sql::Statement s;
   EXPECT_FALSE(s.is_valid());
@@ -121,7 +126,7 @@ TEST_F(SQLStatementTest, BasicErrorCallback) {
   s.BindCString(0, "bad bad");
   EXPECT_FALSE(s.Run());
   EXPECT_EQ(SQLITE_MISMATCH, sqlite_error());
-  reset_error();
+  ResetError();
 }
 
 TEST_F(SQLStatementTest, Reset) {