[sql] Differentiate compile errors from regular errors.

GetUniqueStatement() and GetUntrackedStatement() attempt to be
pedantic about errors preparing statements, because often those errors
indicate either a syntax error in the SQL being prepared, or an error
with the database schema it is being prepared against.  That makes
testing code against various other problems like corruption
challenging.  This change attempts to restrict the flagged errors to
include fewer errors unrelated to the form of the SQL statement (or
schema).

Additionally, slightly improve the debug output for OnSqliteError()
for databases which haven't been tagged.

BUG=537742

Review URL: https://codereview.chromium.org/1434173003

Cr-Commit-Position: refs/heads/master@{#359452}

2 files changed, 30 insertions, 3 deletions

diff --git a/sql/connection.cc b/sql/connection.cc
index 44cb597..5385f49f 100644
--- a/sql/connection.cc
+++ b/sql/connection.cc
@@ -232,6 +232,23 @@ bool Connection::ShouldIgnoreSqliteError(int error) {
   return current_ignorer_cb_->Run(error);
 }
 
+// static
+bool Connection::ShouldIgnoreSqliteCompileError(int error) {
+  // Put this first in case tests need to see that the check happened.
+  if (ShouldIgnoreSqliteError(error))
+    return true;
+
+  // Trim extended error codes.
+  int basic_error = error & 0xff;
+
+  // These errors relate more to the runtime context of the system than to
+  // errors with a SQL statement or with the schema, so they aren't generally
+  // interesting to flag.  This list is not comprehensive.
+  return basic_error == SQLITE_BUSY ||
+      basic_error == SQLITE_NOTADB ||
+      basic_error == SQLITE_CORRUPT;
+}
+
 bool Connection::OnMemoryDump(const base::trace_event::MemoryDumpArgs& args,
                               base::trace_event::ProcessMemoryDump* pmd) {
   if (args.level_of_detail ==
@@ -1339,7 +1356,7 @@ scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
   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.
-    if (!ShouldIgnoreSqliteError(rc))
+    if (!ShouldIgnoreSqliteCompileError(rc))
       DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
 
     // It could also be database corruption.
@@ -1349,6 +1366,8 @@ scoped_refptr<Connection::StatementRef> Connection::GetUniqueStatement(
   return new StatementRef(this, stmt, true);
 }
 
+// TODO(shess): Unify this with GetUniqueStatement().  The only difference that
+// seems legitimate is not passing |this| to StatementRef.
 scoped_refptr<Connection::StatementRef> Connection::GetUntrackedStatement(
     const char* sql) const {
   // Return inactive statement.
@@ -1359,7 +1378,7 @@ scoped_refptr<Connection::StatementRef> Connection::GetUntrackedStatement(
   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.
-    if (!ShouldIgnoreSqliteError(rc))
+    if (!ShouldIgnoreSqliteCompileError(rc))
       DLOG(FATAL) << "SQL compile error " << GetErrorMessage();
     return new StatementRef(NULL, NULL, false);
   }
@@ -1766,7 +1785,11 @@ int Connection::OnSqliteError(int err, sql::Statement *stmt, const char* sql) {
     sql = stmt->GetSQLStatement();
   if (!sql)
     sql = "-- unknown";
-  LOG(ERROR) << histogram_tag_ << " sqlite error " << err
+
+  std::string id = histogram_tag_;
+  if (id.empty())
+    id = DbPath().BaseName().AsUTF8Unsafe();
+  LOG(ERROR) << id << " sqlite error " << err
              << ", errno " << GetLastErrno()
              << ": " << GetErrorMessage()
              << ", sql: " << sql;
diff --git a/sql/connection.h b/sql/connection.h
index 4e3ce26..5b219db 100644
--- a/sql/connection.h
+++ b/sql/connection.h
@@ -466,6 +466,10 @@ class SQL_EXPORT Connection : public base::trace_event::MemoryDumpProvider {
   // tests.
   static bool ShouldIgnoreSqliteError(int error);
 
+  // Additionally ignores errors which are unlikely to be caused by problems
+  // with the syntax of a SQL statement, or problems with the database schema.
+  static bool ShouldIgnoreSqliteCompileError(int error);
+
   // base::trace_event::MemoryDumpProvider implementation.
   bool OnMemoryDump(
       const base::trace_event::MemoryDumpArgs& args,