diff options
-rw-r--r-- | chrome/browser/net/sqlite_server_bound_cert_store.cc | 18 | ||||
-rw-r--r-- | chrome/browser/password_manager/password_store_factory.cc | 13 | ||||
-rw-r--r-- | sql/connection.cc | 29 | ||||
-rw-r--r-- | sql/connection.h | 17 | ||||
-rw-r--r-- | sql/statement.cc | 3 |
5 files changed, 66 insertions, 14 deletions
diff --git a/chrome/browser/net/sqlite_server_bound_cert_store.cc b/chrome/browser/net/sqlite_server_bound_cert_store.cc index 0a297ee..75f9318 100644 --- a/chrome/browser/net/sqlite_server_bound_cert_store.cc +++ b/chrome/browser/net/sqlite_server_bound_cert_store.cc @@ -159,16 +159,16 @@ bool SQLiteServerBoundCertStore::Backend::Load( // This function should be called only once per instance. DCHECK(!db_.get()); + // TODO(paivanof@gmail.com): We do a lot of disk access in this function, + // thus we do an exception to allow IO on the UI thread. This code will be + // moved to the DB thread as part of http://crbug.com/89665. + base::ThreadRestrictions::ScopedAllowIO allow_io; + // Ensure the parent directory for storing certs is created before reading - // from it. We make an exception to allow IO on the UI thread here because - // we are going to disk anyway in db_->Open. (This code will be moved to the - // DB thread as part of http://crbug.com/52909.) - { - base::ThreadRestrictions::ScopedAllowIO allow_io; - const FilePath dir = path_.DirName(); - if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir)) - return false; - } + // from it. + const FilePath dir = path_.DirName(); + if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir)) + return false; db_.reset(new sql::Connection); if (!db_->Open(path_)) { diff --git a/chrome/browser/password_manager/password_store_factory.cc b/chrome/browser/password_manager/password_store_factory.cc index bee596d..b110cb5d 100644 --- a/chrome/browser/password_manager/password_store_factory.cc +++ b/chrome/browser/password_manager/password_store_factory.cc @@ -99,10 +99,15 @@ PasswordStoreFactory::BuildServiceInstanceFor(Profile* profile) const { FilePath login_db_file_path = profile->GetPath(); login_db_file_path = login_db_file_path.Append(chrome::kLoginDataFileName); LoginDatabase* login_db = new LoginDatabase(); - if (!login_db->Init(login_db_file_path)) { - LOG(ERROR) << "Could not initialize login database."; - delete login_db; - return NULL; + { + // TODO(paivanof@gmail.com): execution of login_db->Init() should go + // to DB thread. http://crbug.com/138903 + base::ThreadRestrictions::ScopedAllowIO allow_io; + if (!login_db->Init(login_db_file_path)) { + LOG(ERROR) << "Could not initialize login database."; + delete login_db; + return NULL; + } } #if defined(OS_WIN) ps = new PasswordStoreWin( diff --git a/sql/connection.cc b/sql/connection.cc index 2d7baf2..c7666f9 100644 --- a/sql/connection.cc +++ b/sql/connection.cc @@ -81,6 +81,14 @@ Connection::StatementRef::~StatementRef() { void Connection::StatementRef::Close() { if (stmt_) { + // Call to AssertIOAllowed() cannot go at the beginning of the function + // because Close() is called unconditionally from destructor to clean + // connection_. And if this is inactive statement this won't cause any + // disk access and destructor most probably will be called on thread + // not allowing disk access. + // TODO(paivanof@gmail.com): This should move to the beginning + // of the function. http://crbug.com/136655. + AssertIOAllowed(); sqlite3_finalize(stmt_); stmt_ = NULL; } @@ -93,7 +101,8 @@ Connection::Connection() cache_size_(0), exclusive_locking_(false), transaction_nesting_(0), - needs_rollback_(false) { + needs_rollback_(false), + in_memory_(false) { } Connection::~Connection() { @@ -109,6 +118,7 @@ bool Connection::Open(const FilePath& path) { } bool Connection::OpenInMemory() { + in_memory_ = true; return OpenInternal(":memory:"); } @@ -130,6 +140,13 @@ void Connection::Close() { ClearCache(); if (db_) { + // Call to AssertIOAllowed() cannot go at the beginning of the function + // because Close() must be called from destructor to clean + // statement_cache_, it won't cause any disk access and it most probably + // will happen on thread not allowing disk access. + // TODO(paivanof@gmail.com): This should move to the beginning + // of the function. http://crbug.com/136655. + AssertIOAllowed(); // TODO(shess): Histogram for failure. sqlite3_close(db_); db_ = NULL; @@ -137,6 +154,8 @@ void Connection::Close() { } void Connection::Preload() { + AssertIOAllowed(); + if (!db_) { DLOG(FATAL) << "Cannot preload null db"; return; @@ -161,6 +180,8 @@ void Connection::Preload() { // Create an in-memory database with the existing database's page // size, then backup that database over the existing database. bool Connection::Raze() { + AssertIOAllowed(); + if (!db_) { DLOG(FATAL) << "Cannot raze null db"; return false; @@ -297,6 +318,7 @@ bool Connection::CommitTransaction() { } int Connection::ExecuteAndReturnErrorCode(const char* sql) { + AssertIOAllowed(); if (!db_) return false; return sqlite3_exec(db_, sql, NULL, NULL, NULL); @@ -347,6 +369,8 @@ scoped_refptr<Connection::StatementRef> Connection::GetCachedStatement( scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement( const char* sql) { + AssertIOAllowed(); + if (!db_) return new StatementRef(); // Return inactive statement. @@ -375,6 +399,7 @@ scoped_refptr<Connection::StatementRef> Connection::GetUntrackedStatement( } bool Connection::IsSQLValid(const char* sql) { + AssertIOAllowed(); sqlite3_stmt* stmt = NULL; if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK) return false; @@ -455,6 +480,8 @@ const char* Connection::GetErrorMessage() const { } bool Connection::OpenInternal(const std::string& file_name) { + AssertIOAllowed(); + if (db_) { DLOG(FATAL) << "sql::Connection is already open."; return false; diff --git a/sql/connection.h b/sql/connection.h index e5c9469..65020a0 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/threading/thread_restrictions.h" #include "base/time.h" #include "sql/sql_export.h" @@ -320,6 +321,14 @@ class SQL_EXPORT Connection { // sqlite3_open. The string can also be sqlite's special ":memory:" string. bool OpenInternal(const std::string& file_name); + // 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. + void AssertIOAllowed() { + if (!in_memory_) + base::ThreadRestrictions::AssertIOAllowed(); + } + // Internal helper for DoesTableExist and DoesIndexExist. bool DoesTableOrIndexExist(const char* name, const char* type) const; @@ -356,6 +365,10 @@ class SQL_EXPORT Connection { // no longer be active. void Close(); + // Check whether the current thread is allowed to make IO calls, but only + // if database wasn't open in memory. + void AssertIOAllowed() { if (connection_) connection_->AssertIOAllowed(); } + private: friend class base::RefCounted<StatementRef>; @@ -426,6 +439,10 @@ class SQL_EXPORT Connection { // a rollback instead of a commit. bool needs_rollback_; + // True if database is open with OpenInMemory(), False if database is open + // with Open(). + bool in_memory_; + // 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_; diff --git a/sql/statement.cc b/sql/statement.cc index 3616dcc..84dfd2e 100644 --- a/sql/statement.cc +++ b/sql/statement.cc @@ -48,6 +48,7 @@ bool Statement::CheckValid() const { } bool Statement::Run() { + ref_->AssertIOAllowed(); if (!CheckValid()) return false; @@ -55,6 +56,7 @@ bool Statement::Run() { } bool Statement::Step() { + ref_->AssertIOAllowed(); if (!CheckValid()) return false; @@ -62,6 +64,7 @@ bool Statement::Step() { } void Statement::Reset(bool clear_bound_vars) { + ref_->AssertIOAllowed(); if (is_valid()) { // We don't call CheckError() here because sqlite3_reset() returns // the last error that Step() caused thereby generating a second |