Revert 147309 - Annotate calls to SQLite functions - they have to be …

…executed on a thread allowing IO access.

Expand scope of ScopedAllowIO in the only place where SQLite functions are used on UI thread.

Patch from Pavel Ivanov <paivanof@gmail.com>

BUG=75232,52909,137961
TEST=no test fails with message "Function marked as IO-only was called from a thread that disallows IO!"

Review URL: https://chromiumcodereview.appspot.com/10540155
Patch from Pavel Ivanov <paivanof@gmail.com>.

TBR=shess@chromium.org
Review URL: https://chromiumcodereview.appspot.com/10806025

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

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.
-  const FilePath dir = path_.DirName();
-  if (!file_util::PathExists(dir) && !file_util::CreateDirectory(dir))
-    return false;
+  // 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;
+  }
 
   db_.reset(new sql::Connection);
   if (!db_->Open(path_)) {

sql/connection.cc

@@ -76,14 +76,6 @@ 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;
   }
@@ -96,8 +88,7 @@ Connection::Connection()
       cache_size_(0),
       exclusive_locking_(false),
       transaction_nesting_(0),
-      needs_rollback_(false),
-      in_memory_(false) {
+      needs_rollback_(false) {
 }
 
 Connection::~Connection() {
@@ -113,7 +104,6 @@ bool Connection::Open(const FilePath& path) {
 }
 
 bool Connection::OpenInMemory() {
-  in_memory_ = true;
   return OpenInternal(":memory:");
 }
 
@@ -135,22 +125,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;
   }
 }
 
 void Connection::Preload() {
-  AssertIOAllowed();
-
   if (!db_) {
     DLOG(FATAL) << "Cannot preload null db";
     return;
@@ -175,8 +156,6 @@ 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;
@@ -313,7 +292,6 @@ bool Connection::CommitTransaction() {
 }
 
 int Connection::ExecuteAndReturnErrorCode(const char* sql) {
-  AssertIOAllowed();
   if (!db_)
     return false;
   return sqlite3_exec(db_, sql, NULL, NULL, NULL);
@@ -364,8 +342,6 @@ scoped_refptr<Connection::StatementRef> Connection::GetCachedStatement(
 
 scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
     const char* sql) {
-  AssertIOAllowed();
-
   if (!db_)
     return new StatementRef(this, NULL);  // Return inactive statement.
 
@@ -379,7 +355,6 @@ scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
 }
 
 bool Connection::IsSQLValid(const char* sql) {
-  AssertIOAllowed();
   sqlite3_stmt* stmt = NULL;
   if (sqlite3_prepare_v2(db_, sql, -1, &stmt, NULL) != SQLITE_OK)
     return false;
@@ -466,8 +441,6 @@ 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;

sql/connection.h

@@ -1,4 +1,4 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2011 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.
 
@@ -12,7 +12,6 @@
 #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"
 
@@ -321,14 +320,6 @@ 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;
 
@@ -364,10 +355,6 @@ 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>;
 
@@ -430,10 +417,6 @@ 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_;

sql/statement.cc

@@ -48,23 +48,20 @@ bool Statement::CheckValid() const {
 }
 
 bool Statement::Run() {
-  ref_->AssertIOAllowed();
   if (!CheckValid())
     return false;
 
   return CheckError(sqlite3_step(ref_->stmt())) == SQLITE_DONE;
 }
 
 bool Statement::Step() {
-  ref_->AssertIOAllowed();
   if (!CheckValid())
     return false;
 
   return CheckError(sqlite3_step(ref_->stmt())) == SQLITE_ROW;
 }
 
 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