Merge from Chromium at DEPS revision r190564
This commit was generated by merge_to_master.py.
Change-Id: Icadecbce29854b8fa25fd335b2c1949b5ca5d170
diff --git a/sql/connection.cc b/sql/connection.cc
index 1de9fc5..5f6b590 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -6,8 +6,9 @@
#include <string.h>
-#include "base/file_path.h"
+#include "base/files/file_path.h"
#include "base/logging.h"
+#include "base/metrics/histogram.h"
#include "base/string_util.h"
#include "base/stringprintf.h"
#include "base/utf_string_conversions.h"
@@ -73,30 +74,23 @@
ErrorDelegate::~ErrorDelegate() {
}
-Connection::StatementRef::StatementRef()
- : connection_(NULL),
- stmt_(NULL) {
-}
-
-Connection::StatementRef::StatementRef(sqlite3_stmt* stmt)
- : connection_(NULL),
- stmt_(stmt) {
-}
-
Connection::StatementRef::StatementRef(Connection* connection,
- sqlite3_stmt* stmt)
+ sqlite3_stmt* stmt,
+ bool was_valid)
: connection_(connection),
- stmt_(stmt) {
- connection_->StatementRefCreated(this);
+ stmt_(stmt),
+ was_valid_(was_valid) {
+ if (connection)
+ connection_->StatementRefCreated(this);
}
Connection::StatementRef::~StatementRef() {
if (connection_)
connection_->StatementRefDeleted(this);
- Close();
+ Close(false);
}
-void Connection::StatementRef::Close() {
+void Connection::StatementRef::Close(bool forced) {
if (stmt_) {
// Call to AssertIOAllowed() cannot go at the beginning of the function
// because Close() is called unconditionally from destructor to clean
@@ -110,6 +104,11 @@
stmt_ = NULL;
}
connection_ = NULL; // The connection may be getting deleted.
+
+ // Forced close is expected to happen from a statement error
+ // handler. In that case maintain the sense of |was_valid_| which
+ // previously held for this ref.
+ was_valid_ = was_valid_ && forced;
}
Connection::Connection()
@@ -120,6 +119,7 @@
transaction_nesting_(0),
needs_rollback_(false),
in_memory_(false),
+ poisoned_(false),
error_delegate_(NULL) {
}
@@ -127,7 +127,7 @@
Close();
}
-bool Connection::Open(const FilePath& path) {
+bool Connection::Open(const base::FilePath& path) {
#if defined(OS_WIN)
return OpenInternal(WideToUTF8(path.value()));
#elif defined(OS_POSIX)
@@ -140,22 +140,28 @@
return OpenInternal(":memory:");
}
-void Connection::Close() {
+void Connection::CloseInternal(bool forced) {
// TODO(shess): Calling "PRAGMA journal_mode = DELETE" at this point
// will delete the -journal file. For ChromiumOS or other more
// embedded systems, this is probably not appropriate, whereas on
// desktop it might make some sense.
// sqlite3_close() needs all prepared statements to be finalized.
- // Release all cached statements, then assert that the client has
- // released all statements.
- statement_cache_.clear();
- DCHECK(open_statements_.empty());
- // Additionally clear the prepared statements, because they contain
- // weak references to this connection. This case has come up when
- // error-handling code is hit in production.
- ClearCache();
+ // Release cached statements.
+ statement_cache_.clear();
+
+ // With cached statements released, in-use statements will remain.
+ // Closing the database while statements are in use is an API
+ // violation, except for forced close (which happens from within a
+ // statement's error handler).
+ DCHECK(forced || open_statements_.empty());
+
+ // Deactivate any outstanding statements so sqlite3_close() works.
+ for (StatementRefSet::iterator i = open_statements_.begin();
+ i != open_statements_.end(); ++i)
+ (*i)->Close(forced);
+ open_statements_.clear();
if (db_) {
// Call to AssertIOAllowed() cannot go at the beginning of the function
@@ -171,11 +177,23 @@
}
}
+void Connection::Close() {
+ // If the database was already closed by RazeAndClose(), then no
+ // need to close again. Clear the |poisoned_| bit so that incorrect
+ // API calls are caught.
+ if (poisoned_) {
+ poisoned_ = false;
+ return;
+ }
+
+ CloseInternal(false);
+}
+
void Connection::Preload() {
AssertIOAllowed();
if (!db_) {
- DLOG(FATAL) << "Cannot preload null db";
+ DLOG_IF(FATAL, !poisoned_) << "Cannot preload null db";
return;
}
@@ -201,7 +219,7 @@
AssertIOAllowed();
if (!db_) {
- DLOG(FATAL) << "Cannot raze null db";
+ DLOG_IF(FATAL, !poisoned_) << "Cannot raze null db";
return false;
}
@@ -222,7 +240,8 @@
<< " page_size_ " << page_size_ << " is not a power of two.";
const int kSqliteMaxPageSize = 32768; // from sqliteLimit.h
DCHECK_LE(page_size_, kSqliteMaxPageSize);
- const std::string sql = StringPrintf("PRAGMA page_size=%d", page_size_);
+ const std::string sql =
+ base::StringPrintf("PRAGMA page_size=%d", page_size_);
if (!null_db.Execute(sql.c_str()))
return false;
}
@@ -291,7 +310,7 @@
bool Connection::RazeWithTimout(base::TimeDelta timeout) {
if (!db_) {
- DLOG(FATAL) << "Cannot raze null db";
+ DLOG_IF(FATAL, !poisoned_) << "Cannot raze null db";
return false;
}
@@ -300,6 +319,29 @@
return Raze();
}
+bool Connection::RazeAndClose() {
+ if (!db_) {
+ DLOG_IF(FATAL, !poisoned_) << "Cannot raze null db";
+ return false;
+ }
+
+ // Raze() cannot run in a transaction.
+ while (transaction_nesting_) {
+ RollbackTransaction();
+ }
+
+ bool result = Raze();
+
+ CloseInternal(true);
+
+ // Mark the database so that future API calls fail appropriately,
+ // but don't DCHECK (because after calling this function they are
+ // expected to fail).
+ poisoned_ = true;
+
+ return result;
+}
+
bool Connection::BeginTransaction() {
if (needs_rollback_) {
DCHECK_GT(transaction_nesting_, 0);
@@ -323,7 +365,7 @@
void Connection::RollbackTransaction() {
if (!transaction_nesting_) {
- DLOG(FATAL) << "Rolling back a nonexistent transaction";
+ DLOG_IF(FATAL, !poisoned_) << "Rolling back a nonexistent transaction";
return;
}
@@ -340,7 +382,7 @@
bool Connection::CommitTransaction() {
if (!transaction_nesting_) {
- DLOG(FATAL) << "Rolling back a nonexistent transaction";
+ DLOG_IF(FATAL, !poisoned_) << "Rolling back a nonexistent transaction";
return false;
}
transaction_nesting_--;
@@ -361,12 +403,19 @@
int Connection::ExecuteAndReturnErrorCode(const char* sql) {
AssertIOAllowed();
- if (!db_)
- return false;
+ if (!db_) {
+ DLOG_IF(FATAL, !poisoned_) << "Illegal use of connection without a db";
+ return SQLITE_ERROR;
+ }
return sqlite3_exec(db_, sql, NULL, NULL, NULL);
}
bool Connection::Execute(const char* sql) {
+ if (!db_) {
+ DLOG_IF(FATAL, !poisoned_) << "Illegal use of connection without a db";
+ return false;
+ }
+
int error = ExecuteAndReturnErrorCode(sql);
if (error != SQLITE_OK)
error = OnSqliteError(error, NULL);
@@ -380,8 +429,10 @@
}
bool Connection::ExecuteWithTimeout(const char* sql, base::TimeDelta timeout) {
- if (!db_)
+ if (!db_) {
+ DLOG_IF(FATAL, !poisoned_) << "Illegal use of connection without a db";
return false;
+ }
ScopedBusyTimeout busy_timeout(db_);
busy_timeout.SetTimeout(timeout);
@@ -416,8 +467,9 @@
const char* sql) {
AssertIOAllowed();
+ // Return inactive statement.
if (!db_)
- return new StatementRef(); // Return inactive statement.
+ return new StatementRef(NULL, NULL, poisoned_);
sqlite3_stmt* stmt = NULL;
int rc = sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL);
@@ -427,28 +479,34 @@
// It could also be database corruption.
OnSqliteError(rc, NULL);
- return new StatementRef();
+ return new StatementRef(NULL, NULL, false);
}
- return new StatementRef(this, stmt);
+ return new StatementRef(this, stmt, true);
}
scoped_refptr<Connection::StatementRef> Connection::GetUntrackedStatement(
const char* sql) const {
+ // Return inactive statement.
if (!db_)
- return new StatementRef(); // Return inactive statement.
+ return new StatementRef(NULL, NULL, poisoned_);
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(NULL, NULL, false);
}
- return new StatementRef(stmt);
+ return new StatementRef(NULL, stmt, true);
}
bool Connection::IsSQLValid(const char* sql) {
AssertIOAllowed();
+ if (!db_) {
+ DLOG_IF(FATAL, !poisoned_) << "Illegal use of connection without a db";
+ return false;
+ }
+
sqlite3_stmt* stmt = NULL;
if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK)
return false;
@@ -491,7 +549,7 @@
int64 Connection::GetLastInsertRowId() const {
if (!db_) {
- DLOG(FATAL) << "Illegal use of connection without a db";
+ DLOG_IF(FATAL, !poisoned_) << "Illegal use of connection without a db";
return 0;
}
return sqlite3_last_insert_rowid(db_);
@@ -499,7 +557,7 @@
int Connection::GetLastChangeCount() const {
if (!db_) {
- DLOG(FATAL) << "Illegal use of connection without a db";
+ DLOG_IF(FATAL, !poisoned_) << "Illegal use of connection without a db";
return 0;
}
return sqlite3_changes(db_);
@@ -536,14 +594,36 @@
return false;
}
+ // If |poisoned_| is set, it means an error handler called
+ // RazeAndClose(). Until regular Close() is called, the caller
+ // should be treating the database as open, but is_open() currently
+ // only considers the sqlite3 handle's state.
+ // TODO(shess): Revise is_open() to consider poisoned_, and review
+ // to see if any non-testing code even depends on it.
+ DLOG_IF(FATAL, poisoned_) << "sql::Connection is already open.";
+
int err = sqlite3_open(file_name.c_str(), &db_);
if (err != SQLITE_OK) {
+ // Histogram failures specific to initial open for debugging
+ // purposes.
+ UMA_HISTOGRAM_ENUMERATION("Sqlite.OpenFailure", err & 0xff, 50);
+
OnSqliteError(err, NULL);
Close();
db_ = NULL;
return false;
}
+ // sqlite3_open() does not actually read the database file (unless a
+ // hot journal is found). Successfully executing this pragma on an
+ // existing database requires a valid header on page 1.
+ // TODO(shess): For now, just probing to see what the lay of the
+ // land is. If it's mostly SQLITE_NOTADB, then the database should
+ // be razed.
+ err = ExecuteAndReturnErrorCode("PRAGMA auto_vacuum");
+ if (err != SQLITE_OK)
+ UMA_HISTOGRAM_ENUMERATION("Sqlite.OpenProbeFailure", err & 0xff, 50);
+
// Enable extended result codes to provide more color on I/O errors.
// Not having extended result codes is not a fatal problem, as
// Chromium code does not attempt to handle I/O errors anyhow. The
@@ -587,13 +667,15 @@
<< " page_size_ " << page_size_ << " is not a power of two.";
const int kSqliteMaxPageSize = 32768; // from sqliteLimit.h
DCHECK_LE(page_size_, kSqliteMaxPageSize);
- const std::string sql = StringPrintf("PRAGMA page_size=%d", page_size_);
+ const std::string sql =
+ base::StringPrintf("PRAGMA page_size=%d", page_size_);
if (!ExecuteWithTimeout(sql.c_str(), kBusyTimeout))
DLOG(FATAL) << "Could not set page size: " << GetErrorMessage();
}
if (cache_size_ != 0) {
- const std::string sql = StringPrintf("PRAGMA cache_size=%d", cache_size_);
+ const std::string sql =
+ base::StringPrintf("PRAGMA cache_size=%d", cache_size_);
if (!ExecuteWithTimeout(sql.c_str(), kBusyTimeout))
DLOG(FATAL) << "Could not set cache size: " << GetErrorMessage();
}
@@ -626,20 +708,35 @@
open_statements_.erase(i);
}
-void Connection::ClearCache() {
- statement_cache_.clear();
-
- // The cache clear will get most statements. There may be still be references
- // to some statements that are held by others (including one-shot statements).
- // This will deactivate them so they can't be used again.
- for (StatementRefSet::iterator i = open_statements_.begin();
- i != open_statements_.end(); ++i)
- (*i)->Close();
-}
-
int Connection::OnSqliteError(int err, sql::Statement *stmt) {
+ // Strip extended error codes.
+ int base_err = err&0xff;
+
+ static size_t kSqliteErrorMax = 50;
+ UMA_HISTOGRAM_ENUMERATION("Sqlite.Error", base_err, kSqliteErrorMax);
+ if (!error_histogram_name_.empty()) {
+ // TODO(shess): The histogram macros create a bit of static
+ // storage for caching the histogram object. Since SQLite is
+ // being used for I/O, generally without error, this code
+ // shouldn't execute often enough for such caching to be crucial.
+ // If it becomes an issue, the object could be cached alongside
+ // error_histogram_name_.
+ base::HistogramBase* histogram =
+ base::LinearHistogram::FactoryGet(
+ error_histogram_name_, 1, kSqliteErrorMax, kSqliteErrorMax + 1,
+ base::HistogramBase::kUmaTargetedHistogramFlag);
+ if (histogram)
+ histogram->Add(base_err);
+ }
+
+ // Always log the error.
+ LOG(ERROR) << "sqlite error " << err
+ << ", errno " << GetLastErrno()
+ << ": " << GetErrorMessage();
+
if (error_delegate_.get())
return error_delegate_->OnError(err, this, stmt);
+
// The default handling is to assert on debug and to ignore on release.
DLOG(FATAL) << GetErrorMessage();
return err;
diff --git a/sql/connection.h b/sql/connection.h
index 8cf4d71..cebc774 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -17,10 +17,13 @@
#include "base/time.h"
#include "sql/sql_export.h"
-class FilePath;
struct sqlite3;
struct sqlite3_stmt;
+namespace base {
+class FilePath;
+}
+
namespace sql {
class Statement;
@@ -143,18 +146,25 @@
error_delegate_.reset(delegate);
}
+ // SQLite error codes for errors on all connections are logged to
+ // enum histogram "Sqlite.Error". Setting this additionally logs
+ // errors to the histogram |name|.
+ void set_error_histogram_name(const std::string& name) {
+ error_histogram_name_ = name;
+ }
+
// Initialization ------------------------------------------------------------
// Initializes the SQL connection for the given file, returning true if the
// file could be opened. You can call this or OpenInMemory.
- bool Open(const FilePath& path) WARN_UNUSED_RESULT;
+ bool Open(const base::FilePath& path) WARN_UNUSED_RESULT;
// Initializes the SQL connection for a temporary in-memory database. There
// will be no associated file on disk, and the initial database will be
// empty. You can call this or Open.
bool OpenInMemory() WARN_UNUSED_RESULT;
- // Returns trie if the database has been successfully opened.
+ // Returns true if the database has been successfully opened.
bool is_open() const { return !!db_; }
// Closes the database. This is automatically performed on destruction for
@@ -211,6 +221,16 @@
bool Raze();
bool RazeWithTimout(base::TimeDelta timeout);
+ // Breaks all outstanding transactions (as initiated by
+ // BeginTransaction()), calls Raze() to destroy the database, then
+ // closes the database. After this is called, any operations
+ // against the connections (or statements prepared by the
+ // connection) should fail safely.
+ //
+ // The value from Raze() is returned, with Close() called in all
+ // cases.
+ bool RazeAndClose();
+
// Transactions --------------------------------------------------------------
// Transaction management. We maintain a virtual transaction stack to emulate
@@ -331,6 +351,10 @@
// sqlite3_open. The string can also be sqlite's special ":memory:" string.
bool OpenInternal(const std::string& file_name);
+ // Internal close function used by Close() and RazeAndClose().
+ // |forced| indicates that orderly-shutdown checks should not apply.
+ void CloseInternal(bool forced);
+
// Check whether the current thread is allowed to make IO calls, but only
// if database wasn't open in memory. Function is inlined to be a no-op in
// official build.
@@ -355,16 +379,27 @@
// should always check validity before using.
class SQL_EXPORT StatementRef : public base::RefCounted<StatementRef> {
public:
- // Default constructor initializes to an invalid statement.
- StatementRef();
- explicit StatementRef(sqlite3_stmt* stmt);
- StatementRef(Connection* connection, sqlite3_stmt* stmt);
+ // |connection| is the sql::Connection instance associated with
+ // the statement, and is used for tracking outstanding statements
+ // and for error handling. Set to NULL for invalid or untracked
+ // refs. |stmt| is the actual statement, and should only be NULL
+ // to create an invalid ref. |was_valid| indicates whether the
+ // statement should be considered valid for diagnistic purposes.
+ // |was_valid| can be true for NULL |stmt| if the connection has
+ // been forcibly closed by an error handler.
+ StatementRef(Connection* connection, sqlite3_stmt* stmt, bool was_valid);
// When true, the statement can be used.
bool is_valid() const { return !!stmt_; }
- // If we've not been linked to a connection, this will be NULL. Guaranteed
- // non-NULL when is_valid().
+ // When true, the statement is either currently valid, or was
+ // previously valid but the connection was forcibly closed. Used
+ // for diagnostic checks.
+ bool was_valid() const { return was_valid_; }
+
+ // If we've not been linked to a connection, this will be NULL.
+ // TODO(shess): connection_ can be NULL in case of GetUntrackedStatement(),
+ // which prevents Statement::OnError() from forwarding errors.
Connection* connection() const { return connection_; }
// Returns the sqlite statement if any. If the statement is not active,
@@ -372,8 +407,9 @@
sqlite3_stmt* stmt() const { return stmt_; }
// Destroys the compiled statement and marks it NULL. The statement will
- // no longer be active.
- void Close();
+ // no longer be active. |forced| is used to indicate if orderly-shutdown
+ // checks should apply (see Connection::RazeAndClose()).
+ void Close(bool forced);
// Check whether the current thread is allowed to make IO calls, but only
// if database wasn't open in memory.
@@ -386,6 +422,7 @@
Connection* connection_;
sqlite3_stmt* stmt_;
+ bool was_valid_;
DISALLOW_COPY_AND_ASSIGN(StatementRef);
};
@@ -400,9 +437,6 @@
void StatementRefCreated(StatementRef* ref);
void StatementRefDeleted(StatementRef* ref);
- // Frees all cached statements from statement_cache_.
- void ClearCache();
-
// Called by Statement objects when an sqlite function returns an error.
// The return value is the error code reflected back to client code.
int OnSqliteError(int err, Statement* stmt);
@@ -453,10 +487,19 @@
// with Open().
bool in_memory_;
+ // |true| if the connection was closed using RazeAndClose(). Used
+ // to enable diagnostics to distinguish calls to never-opened
+ // databases (incorrect use of the API) from calls to once-valid
+ // databases.
+ bool poisoned_;
+
// This object handles errors resulting from all forms of executing sqlite
// commands or statements. It can be null which means default handling.
scoped_ptr<ErrorDelegate> error_delegate_;
+ // Auxiliary error-code histogram.
+ std::string error_histogram_name_;
+
DISALLOW_COPY_AND_ASSIGN(Connection);
};
diff --git a/sql/connection_unittest.cc b/sql/connection_unittest.cc
index 8036409..2aaeb27 100644
--- a/sql/connection_unittest.cc
+++ b/sql/connection_unittest.cc
@@ -3,10 +3,11 @@
// found in the LICENSE file.
#include "base/file_util.h"
-#include "base/scoped_temp_dir.h"
+#include "base/files/scoped_temp_dir.h"
+#include "base/logging.h"
#include "sql/connection.h"
-#include "sql/statement.h"
#include "sql/meta_table.h"
+#include "sql/statement.h"
#include "testing/gtest/include/gtest/gtest.h"
#include "third_party/sqlite/sqlite3.h"
@@ -14,23 +15,23 @@
public:
SQLConnectionTest() {}
- void SetUp() {
+ virtual void SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
ASSERT_TRUE(db_.Open(db_path()));
}
- void TearDown() {
+ virtual void TearDown() {
db_.Close();
}
sql::Connection& db() { return db_; }
- FilePath db_path() {
+ base::FilePath db_path() {
return temp_dir_.path().AppendASCII("SQLConnectionTest.db");
}
private:
- ScopedTempDir temp_dir_;
+ base::ScopedTempDir temp_dir_;
sql::Connection db_;
};
@@ -279,6 +280,111 @@
ASSERT_TRUE(db().Raze());
}
+// Basic test of RazeAndClose() operation.
+TEST_F(SQLConnectionTest, RazeAndClose) {
+ const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
+ const char* kPopulateSql = "INSERT INTO foo (value) VALUES (12)";
+
+ // Test that RazeAndClose() closes the database, and that the
+ // database is empty when re-opened.
+ ASSERT_TRUE(db().Execute(kCreateSql));
+ ASSERT_TRUE(db().Execute(kPopulateSql));
+ ASSERT_TRUE(db().RazeAndClose());
+ ASSERT_FALSE(db().is_open());
+ db().Close();
+ ASSERT_TRUE(db().Open(db_path()));
+ {
+ sql::Statement s(db().GetUniqueStatement("SELECT * FROM sqlite_master"));
+ ASSERT_FALSE(s.Step());
+ }
+
+ // Test that RazeAndClose() can break transactions.
+ ASSERT_TRUE(db().Execute(kCreateSql));
+ ASSERT_TRUE(db().Execute(kPopulateSql));
+ ASSERT_TRUE(db().BeginTransaction());
+ ASSERT_TRUE(db().RazeAndClose());
+ ASSERT_FALSE(db().is_open());
+ ASSERT_FALSE(db().CommitTransaction());
+ db().Close();
+ ASSERT_TRUE(db().Open(db_path()));
+ {
+ sql::Statement s(db().GetUniqueStatement("SELECT * FROM sqlite_master"));
+ ASSERT_FALSE(s.Step());
+ }
+}
+
+// Test that various operations fail without crashing after
+// RazeAndClose().
+TEST_F(SQLConnectionTest, RazeAndCloseDiagnostics) {
+ const char* kCreateSql = "CREATE TABLE foo (id INTEGER PRIMARY KEY, value)";
+ const char* kPopulateSql = "INSERT INTO foo (value) VALUES (12)";
+ const char* kSimpleSql = "SELECT 1";
+
+ ASSERT_TRUE(db().Execute(kCreateSql));
+ ASSERT_TRUE(db().Execute(kPopulateSql));
+
+ // Test baseline expectations.
+ db().Preload();
+ ASSERT_TRUE(db().DoesTableExist("foo"));
+ ASSERT_TRUE(db().IsSQLValid(kSimpleSql));
+ ASSERT_EQ(SQLITE_OK, db().ExecuteAndReturnErrorCode(kSimpleSql));
+ ASSERT_TRUE(db().Execute(kSimpleSql));
+ ASSERT_TRUE(db().is_open());
+ {
+ sql::Statement s(db().GetUniqueStatement(kSimpleSql));
+ ASSERT_TRUE(s.Step());
+ }
+ {
+ sql::Statement s(db().GetCachedStatement(SQL_FROM_HERE, kSimpleSql));
+ ASSERT_TRUE(s.Step());
+ }
+ ASSERT_TRUE(db().BeginTransaction());
+ ASSERT_TRUE(db().CommitTransaction());
+ ASSERT_TRUE(db().BeginTransaction());
+ db().RollbackTransaction();
+
+ ASSERT_TRUE(db().RazeAndClose());
+
+ // At this point, they should all fail, but not crash.
+ db().Preload();
+ ASSERT_FALSE(db().DoesTableExist("foo"));
+ ASSERT_FALSE(db().IsSQLValid(kSimpleSql));
+ ASSERT_EQ(SQLITE_ERROR, db().ExecuteAndReturnErrorCode(kSimpleSql));
+ ASSERT_FALSE(db().Execute(kSimpleSql));
+ ASSERT_FALSE(db().is_open());
+ {
+ sql::Statement s(db().GetUniqueStatement(kSimpleSql));
+ ASSERT_FALSE(s.Step());
+ }
+ {
+ sql::Statement s(db().GetCachedStatement(SQL_FROM_HERE, kSimpleSql));
+ ASSERT_FALSE(s.Step());
+ }
+ ASSERT_FALSE(db().BeginTransaction());
+ ASSERT_FALSE(db().CommitTransaction());
+ ASSERT_FALSE(db().BeginTransaction());
+ db().RollbackTransaction();
+
+ // Close normally to reset the poisoned flag.
+ db().Close();
+
+ // DEATH tests not supported on Android or iOS.
+#if !defined(OS_ANDROID) && !defined(OS_IOS)
+ // Once the real Close() has been called, various calls enforce API
+ // usage by becoming fatal in debug mode. Since DEATH tests are
+ // expensive, just test one of them.
+ if (DLOG_IS_ON(FATAL)) {
+ ASSERT_DEATH({
+ db().IsSQLValid(kSimpleSql);
+ }, "Illegal use of connection without a db");
+ }
+#endif
+}
+
+// TODO(shess): Spin up a background thread to hold other_db, to more
+// closely match real life. That would also allow testing
+// RazeWithTimeout().
+
#if defined(OS_ANDROID)
TEST_F(SQLConnectionTest, SetTempDirForSQL) {
@@ -291,7 +397,3 @@
ASSERT_TRUE(meta_table.Init(&db(), 4, 4));
}
#endif
-
-// TODO(shess): Spin up a background thread to hold other_db, to more
-// closely match real life. That would also allow testing
-// RazeWithTimeout().
diff --git a/sql/run_all_unittests.cc b/sql/run_all_unittests.cc
index 969b091..2c8d29c 100644
--- a/sql/run_all_unittests.cc
+++ b/sql/run_all_unittests.cc
@@ -2,10 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#include "base/test/main_hook.h"
#include "base/test/test_suite.h"
int main(int argc, char** argv) {
- MainHook hook(main, argc, argv);
return base::TestSuite(argc, argv).Run();
}
diff --git a/sql/sql.gyp b/sql/sql.gyp
index 86962b3..0d8f8be 100644
--- a/sql/sql.gyp
+++ b/sql/sql.gyp
@@ -29,6 +29,8 @@
'transaction.cc',
'transaction.h',
],
+ # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
+ 'msvs_disabled_warnings': [4267, ],
},
{
'target_name': 'sql_unittests',
@@ -64,6 +66,8 @@
],
}],
],
+ # TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
+ 'msvs_disabled_warnings': [4267, ],
},
],
'conditions': [
diff --git a/sql/sql.target.mk b/sql/sql.target.linux-arm.mk
similarity index 86%
rename from sql/sql.target.mk
rename to sql/sql.target.linux-arm.mk
index 029beef..046707e 100644
--- a/sql/sql.target.mk
+++ b/sql/sql.target.linux-arm.mk
@@ -32,6 +32,8 @@
# Flags passed to both C and C++ files.
MY_CFLAGS := \
+ -fstack-protector \
+ --param=ssp-buffer-size=4 \
-Werror \
-fno-exceptions \
-fno-strict-aliasing \
@@ -41,15 +43,9 @@
-fvisibility=hidden \
-pipe \
-fPIC \
- -mthumb \
- -march=armv7-a \
- -mtune=cortex-a8 \
- -mfloat-abi=softfp \
- -mfpu=vfpv3-d16 \
-fno-tree-sra \
-fuse-ld=gold \
-Wno-psabi \
- -mthumb-interwork \
-ffunction-sections \
-funwind-tables \
-g \
@@ -57,11 +53,10 @@
-fno-short-enums \
-finline-limit=64 \
-Wa,--noexecstack \
- -Wno-error=extra \
- -Wno-error=ignored-qualifiers \
- -Wno-error=type-limits \
- -Wno-error=non-virtual-dtor \
- -Wno-error=sign-promo \
+ -U_FORTIFY_SOURCE \
+ -Wno-extra \
+ -Wno-ignored-qualifiers \
+ -Wno-type-limits \
-Os \
-g \
-fomit-frame-pointer \
@@ -71,7 +66,9 @@
MY_CFLAGS_C :=
MY_DEFS := \
+ '-DUSE_SKIA' \
'-D_FILE_OFFSET_BITS=64' \
+ '-DUSE_LINUX_BREAKPAD' \
'-DNO_TCMALLOC' \
'-DDISABLE_NACL' \
'-DCHROMIUM_BUILD' \
@@ -81,7 +78,7 @@
'-DENABLE_GPU=1' \
'-DUSE_OPENSSL=1' \
'-DENABLE_EGLIMAGE=1' \
- '-DUSE_SKIA=1' \
+ '-DENABLE_LANGUAGE_DETECTION=1' \
'-DSQL_IMPLEMENTATION' \
'-D__STDC_CONSTANT_MACROS' \
'-D__STDC_FORMAT_MACROS' \
@@ -89,8 +86,7 @@
'-D__GNU_SOURCE=1' \
'-DUSE_STLPORT=1' \
'-D_STLP_USE_PTR_SPECIALIZATIONS=1' \
- '-DCHROME_SYMBOLS_ID=""' \
- '-DANDROID_UPSTREAM_BRINGUP=1' \
+ '-DCHROME_BUILD_ID=""' \
'-DDYNAMIC_ANNOTATIONS_ENABLED=1' \
'-DWTF_USE_DYNAMIC_ANNOTATIONS=1' \
'-D_DEBUG'
@@ -99,6 +95,8 @@
# Include paths placed before CFLAGS/CPPFLAGS
LOCAL_C_INCLUDES := \
+ $(gyp_shared_intermediate_dir)/shim_headers/icui18n/target \
+ $(gyp_shared_intermediate_dir)/shim_headers/icuuc/target \
$(LOCAL_PATH) \
$(LOCAL_PATH)/third_party/sqlite \
$(GYP_ABS_ANDROID_TOP_DIR)/frameworks/wilhelm/include \
@@ -114,11 +112,15 @@
-fvisibility-inlines-hidden \
-Wsign-compare \
-Wno-abi \
- -Wno-error=c++0x-compat
+ -Wno-error=c++0x-compat \
+ -Wno-non-virtual-dtor \
+ -Wno-sign-promo
### Rules for final target.
LOCAL_LDFLAGS := \
+ -Wl,-z,now \
+ -Wl,-z,relro \
-Wl,-z,noexecstack \
-fPIC \
-Wl,-z,relro \
diff --git a/sql/sql.target.mk b/sql/sql.target.linux-x86.mk
similarity index 84%
copy from sql/sql.target.mk
copy to sql/sql.target.linux-x86.mk
index 029beef..1b5acd2 100644
--- a/sql/sql.target.mk
+++ b/sql/sql.target.linux-x86.mk
@@ -32,6 +32,7 @@
# Flags passed to both C and C++ files.
MY_CFLAGS := \
+ --param=ssp-buffer-size=4 \
-Werror \
-fno-exceptions \
-fno-strict-aliasing \
@@ -41,27 +42,22 @@
-fvisibility=hidden \
-pipe \
-fPIC \
- -mthumb \
- -march=armv7-a \
- -mtune=cortex-a8 \
- -mfloat-abi=softfp \
- -mfpu=vfpv3-d16 \
- -fno-tree-sra \
- -fuse-ld=gold \
- -Wno-psabi \
- -mthumb-interwork \
+ -m32 \
+ -mmmx \
+ -march=pentium4 \
+ -msse2 \
+ -mfpmath=sse \
-ffunction-sections \
-funwind-tables \
-g \
- -fstack-protector \
-fno-short-enums \
-finline-limit=64 \
-Wa,--noexecstack \
- -Wno-error=extra \
- -Wno-error=ignored-qualifiers \
- -Wno-error=type-limits \
- -Wno-error=non-virtual-dtor \
- -Wno-error=sign-promo \
+ -U_FORTIFY_SOURCE \
+ -Wno-extra \
+ -Wno-ignored-qualifiers \
+ -Wno-type-limits \
+ -fno-stack-protector \
-Os \
-g \
-fomit-frame-pointer \
@@ -71,7 +67,9 @@
MY_CFLAGS_C :=
MY_DEFS := \
+ '-DUSE_SKIA' \
'-D_FILE_OFFSET_BITS=64' \
+ '-DUSE_LINUX_BREAKPAD' \
'-DNO_TCMALLOC' \
'-DDISABLE_NACL' \
'-DCHROMIUM_BUILD' \
@@ -81,7 +79,7 @@
'-DENABLE_GPU=1' \
'-DUSE_OPENSSL=1' \
'-DENABLE_EGLIMAGE=1' \
- '-DUSE_SKIA=1' \
+ '-DENABLE_LANGUAGE_DETECTION=1' \
'-DSQL_IMPLEMENTATION' \
'-D__STDC_CONSTANT_MACROS' \
'-D__STDC_FORMAT_MACROS' \
@@ -89,8 +87,7 @@
'-D__GNU_SOURCE=1' \
'-DUSE_STLPORT=1' \
'-D_STLP_USE_PTR_SPECIALIZATIONS=1' \
- '-DCHROME_SYMBOLS_ID=""' \
- '-DANDROID_UPSTREAM_BRINGUP=1' \
+ '-DCHROME_BUILD_ID=""' \
'-DDYNAMIC_ANNOTATIONS_ENABLED=1' \
'-DWTF_USE_DYNAMIC_ANNOTATIONS=1' \
'-D_DEBUG'
@@ -99,6 +96,8 @@
# Include paths placed before CFLAGS/CPPFLAGS
LOCAL_C_INCLUDES := \
+ $(gyp_shared_intermediate_dir)/shim_headers/icui18n/target \
+ $(gyp_shared_intermediate_dir)/shim_headers/icuuc/target \
$(LOCAL_PATH) \
$(LOCAL_PATH)/third_party/sqlite \
$(GYP_ABS_ANDROID_TOP_DIR)/frameworks/wilhelm/include \
@@ -113,21 +112,21 @@
-fno-threadsafe-statics \
-fvisibility-inlines-hidden \
-Wsign-compare \
- -Wno-abi \
- -Wno-error=c++0x-compat
+ -Wno-error=c++0x-compat \
+ -Wno-non-virtual-dtor \
+ -Wno-sign-promo
### Rules for final target.
LOCAL_LDFLAGS := \
+ -Wl,-z,now \
+ -Wl,-z,relro \
-Wl,-z,noexecstack \
-fPIC \
- -Wl,-z,relro \
- -Wl,-z,now \
- -fuse-ld=gold \
+ -m32 \
-nostdlib \
-Wl,--no-undefined \
-Wl,--exclude-libs=ALL \
- -Wl,--icf=safe \
-Wl,-O1 \
-Wl,--as-needed \
-Wl,--gc-sections
diff --git a/sql/sqlite_features_unittest.cc b/sql/sqlite_features_unittest.cc
index 7e750e9..2f6a49e 100644
--- a/sql/sqlite_features_unittest.cc
+++ b/sql/sqlite_features_unittest.cc
@@ -5,7 +5,7 @@
#include <string>
#include "base/file_util.h"
-#include "base/scoped_temp_dir.h"
+#include "base/files/scoped_temp_dir.h"
#include "sql/connection.h"
#include "sql/statement.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -42,7 +42,7 @@
public:
SQLiteFeaturesTest() : error_(SQLITE_OK) {}
- void SetUp() {
+ virtual void SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
ASSERT_TRUE(db_.Open(temp_dir_.path().AppendASCII("SQLStatementTest.db")));
@@ -51,7 +51,7 @@
db_.set_error_delegate(new StatementErrorHandler(&error_, &sql_text_));
}
- void TearDown() {
+ virtual void TearDown() {
// If any error happened the original sql statement can be found in
// |sql_text_|.
EXPECT_EQ(SQLITE_OK, error_);
@@ -65,7 +65,7 @@
}
private:
- ScopedTempDir temp_dir_;
+ base::ScopedTempDir temp_dir_;
sql::Connection db_;
// The error code of the most recent error.
diff --git a/sql/statement.cc b/sql/statement.cc
index 84dfd2e..d92be01 100644
--- a/sql/statement.cc
+++ b/sql/statement.cc
@@ -15,7 +15,7 @@
// we don't have to NULL-check the ref_ to see if the statement is valid: we
// only have to check the ref's validity bit.
Statement::Statement()
- : ref_(new Connection::StatementRef),
+ : ref_(new Connection::StatementRef(NULL, NULL, false)),
succeeded_(false) {
}
@@ -37,13 +37,15 @@
}
void Statement::Clear() {
- Assign(new Connection::StatementRef);
+ Assign(new Connection::StatementRef(NULL, NULL, false));
succeeded_ = false;
}
bool Statement::CheckValid() const {
- if (!is_valid())
- DLOG(FATAL) << "Cannot call mutating statements on an invalid statement.";
+ // Allow operations to fail silently if a statement was invalidated
+ // because the database was closed by an error handler.
+ DLOG_IF(FATAL, !ref_->was_valid())
+ << "Cannot call mutating statements on an invalid statement.";
return is_valid();
}
@@ -306,7 +308,7 @@
int Statement::CheckError(int err) {
// Please don't add DCHECKs here, OnSqliteError() already has them.
succeeded_ = (err == SQLITE_OK || err == SQLITE_ROW || err == SQLITE_DONE);
- if (!succeeded_ && is_valid())
+ if (!succeeded_ && ref_ && ref_->connection())
return ref_->connection()->OnSqliteError(err, this);
return err;
}
diff --git a/sql/statement_unittest.cc b/sql/statement_unittest.cc
index a7a23d8..ffa8aef 100644
--- a/sql/statement_unittest.cc
+++ b/sql/statement_unittest.cc
@@ -5,7 +5,7 @@
#include <string>
#include "base/file_util.h"
-#include "base/scoped_temp_dir.h"
+#include "base/files/scoped_temp_dir.h"
#include "sql/connection.h"
#include "sql/statement.h"
#include "testing/gtest/include/gtest/gtest.h"
@@ -40,7 +40,7 @@
public:
SQLStatementTest() : error_(SQLITE_OK) {}
- void SetUp() {
+ virtual void SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
ASSERT_TRUE(db_.Open(temp_dir_.path().AppendASCII("SQLStatementTest.db")));
// The error delegate will set |error_| and |sql_text_| when any sqlite
@@ -48,7 +48,7 @@
db_.set_error_delegate(new StatementErrorHandler(&error_, &sql_text_));
}
- void TearDown() {
+ virtual void TearDown() {
// If any error happened the original sql statement can be found in
// |sql_text_|.
EXPECT_EQ(SQLITE_OK, error_);
@@ -65,7 +65,7 @@
}
private:
- ScopedTempDir temp_dir_;
+ base::ScopedTempDir temp_dir_;
sql::Connection db_;
// The error code of the most recent error.
diff --git a/sql/transaction_unittest.cc b/sql/transaction_unittest.cc
index f306a5c..ceaa4db 100644
--- a/sql/transaction_unittest.cc
+++ b/sql/transaction_unittest.cc
@@ -3,7 +3,7 @@
// found in the LICENSE file.
#include "base/file_util.h"
-#include "base/scoped_temp_dir.h"
+#include "base/files/scoped_temp_dir.h"
#include "sql/connection.h"
#include "sql/statement.h"
#include "sql/transaction.h"
@@ -14,7 +14,7 @@
public:
SQLTransactionTest() {}
- void SetUp() {
+ virtual void SetUp() {
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
ASSERT_TRUE(db_.Open(
temp_dir_.path().AppendASCII("SQLTransactionTest.db")));
@@ -22,7 +22,7 @@
ASSERT_TRUE(db().Execute("CREATE TABLE foo (a, b)"));
}
- void TearDown() {
+ virtual void TearDown() {
db_.Close();
}
@@ -36,7 +36,7 @@
}
private:
- ScopedTempDir temp_dir_;
+ base::ScopedTempDir temp_dir_;
sql::Connection db_;
};