[sql] Convert webkit tests to use ScopedErrorIgnorer.

Get rid of some error callbacks by converting to ScopedErrorIgnorer.
Also make the error suppression more specific.

BUG=none

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

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

4 files changed, 54 insertions, 39 deletions

diff --git a/webkit/browser/appcache/appcache_database_unittest.cc b/webkit/browser/appcache/appcache_database_unittest.cc
index 34a6f0c..e67a68d 100644
--- a/webkit/browser/appcache/appcache_database_unittest.cc
+++ b/webkit/browser/appcache/appcache_database_unittest.cc
@@ -9,8 +9,10 @@
 #include "sql/connection.h"
 #include "sql/meta_table.h"
 #include "sql/statement.h"
+#include "sql/test/scoped_error_ignorer.h"
 #include "sql/transaction.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/sqlite/sqlite3.h"
 #include "webkit/browser/appcache/appcache_database.h"
 #include "webkit/browser/appcache/appcache_entry.h"
 
@@ -18,12 +20,6 @@ namespace {
 
 const base::Time kZeroTime;
 
-// Set this error callback to "handle" errors by ignoring them.  This
-// causes the sql/ machinery to not throw fatal reports and allow
-// higher-level code to return failures.
-void IgnoreDatabaseErrors(int error, sql::Statement* stmt) {
-}
-
 }  // namespace
 
 namespace appcache {
@@ -112,7 +108,13 @@ TEST(AppCacheDatabaseTest, EntryRecords) {
   AppCacheDatabase db(kEmptyPath);
   EXPECT_TRUE(db.LazyOpen(true));
 
-  db.db_->set_error_callback(base::Bind(&IgnoreDatabaseErrors));
+  sql::ScopedErrorIgnorer ignore_errors;
+  // TODO(shess): Suppressing SQLITE_CONSTRAINT because the code
+  // expects that and handles the resulting error.  Consider revising
+  // the code to use INSERT OR IGNORE (which would not throw
+  // SQLITE_CONSTRAINT) and then check ChangeCount() to see if any
+  // changes were made.
+  ignore_errors.IgnoreError(SQLITE_CONSTRAINT);
 
   AppCacheDatabase::EntryRecord entry;
 
@@ -178,6 +180,8 @@ TEST(AppCacheDatabaseTest, EntryRecords) {
   EXPECT_TRUE(db.DeleteEntriesForCache(1));
   EXPECT_FALSE(db.AddEntryFlags(GURL("http://blah/1"), 1,
                                 AppCacheEntry::FOREIGN));
+
+  ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
 }
 
 TEST(AppCacheDatabaseTest, CacheRecords) {
@@ -185,7 +189,9 @@ TEST(AppCacheDatabaseTest, CacheRecords) {
   AppCacheDatabase db(kEmptyPath);
   EXPECT_TRUE(db.LazyOpen(true));
 
-  db.db_->set_error_callback(base::Bind(&IgnoreDatabaseErrors));
+  sql::ScopedErrorIgnorer ignore_errors;
+  // TODO(shess): See EntryRecords test.
+  ignore_errors.IgnoreError(SQLITE_CONSTRAINT);
 
   const AppCacheDatabase::CacheRecord kZeroRecord;
   AppCacheDatabase::CacheRecord record;
@@ -220,6 +226,8 @@ TEST(AppCacheDatabaseTest, CacheRecords) {
   EXPECT_FALSE(db.FindCacheForGroup(1, &record));
 
   EXPECT_TRUE(db.DeleteCache(1));
+
+  ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
 }
 
 TEST(AppCacheDatabaseTest, GroupRecords) {
@@ -227,7 +235,9 @@ TEST(AppCacheDatabaseTest, GroupRecords) {
   AppCacheDatabase db(kEmptyPath);
   EXPECT_TRUE(db.LazyOpen(true));
 
-  db.db_->set_error_callback(base::Bind(&IgnoreDatabaseErrors));
+  sql::ScopedErrorIgnorer ignore_errors;
+  // TODO(shess): See EntryRecords test.
+  ignore_errors.IgnoreError(SQLITE_CONSTRAINT);
 
   const GURL kManifestUrl("http://blah/manifest");
   const GURL kOrigin(kManifestUrl.GetOrigin());
@@ -347,6 +357,8 @@ TEST(AppCacheDatabaseTest, GroupRecords) {
   EXPECT_EQ(1, record.group_id);
   EXPECT_EQ(kManifest2, record.manifest_url);
   EXPECT_EQ(kOrigin2, record.origin);
+
+  ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
 }
 
 TEST(AppCacheDatabaseTest, NamespaceRecords) {
@@ -354,7 +366,9 @@ TEST(AppCacheDatabaseTest, NamespaceRecords) {
   AppCacheDatabase db(kEmptyPath);
   EXPECT_TRUE(db.LazyOpen(true));
 
-  db.db_->set_error_callback(base::Bind(&IgnoreDatabaseErrors));
+  sql::ScopedErrorIgnorer ignore_errors;
+  // TODO(shess): See EntryRecords test.
+  ignore_errors.IgnoreError(SQLITE_CONSTRAINT);
 
   const GURL kFooNameSpace1("http://foo/namespace1");
   const GURL kFooNameSpace2("http://foo/namespace2");
@@ -459,6 +473,8 @@ TEST(AppCacheDatabaseTest, NamespaceRecords) {
   EXPECT_EQ(2U, fallbacks.size());
   EXPECT_TRUE(fallbacks[0].namespace_.is_pattern);
   EXPECT_TRUE(fallbacks[1].namespace_.is_pattern);
+
+  ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
 }
 
 TEST(AppCacheDatabaseTest, OnlineWhiteListRecords) {
@@ -466,8 +482,6 @@ TEST(AppCacheDatabaseTest, OnlineWhiteListRecords) {
   AppCacheDatabase db(kEmptyPath);
   EXPECT_TRUE(db.LazyOpen(true));
 
-  db.db_->set_error_callback(base::Bind(&IgnoreDatabaseErrors));
-
   const GURL kFooNameSpace1("http://foo/namespace1");
   const GURL kFooNameSpace2("http://foo/namespace2");
   const GURL kBarNameSpace1("http://bar/namespace1");
@@ -515,7 +529,9 @@ TEST(AppCacheDatabaseTest, DeletableResponseIds) {
   AppCacheDatabase db(kEmptyPath);
   EXPECT_TRUE(db.LazyOpen(true));
 
-  db.db_->set_error_callback(base::Bind(&IgnoreDatabaseErrors));
+  sql::ScopedErrorIgnorer ignore_errors;
+  // TODO(shess): See EntryRecords test.
+  ignore_errors.IgnoreError(SQLITE_CONSTRAINT);
 
   std::vector<int64> ids;
 
@@ -578,6 +594,8 @@ TEST(AppCacheDatabaseTest, DeletableResponseIds) {
   EXPECT_EQ(5U, ids.size());
   for (int i = 0; i < static_cast<int>(ids.size()); ++i)
     EXPECT_EQ(i + 5, ids[i]);
+
+  ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
 }
 
 TEST(AppCacheDatabaseTest, OriginUsage) {
@@ -591,8 +609,6 @@ TEST(AppCacheDatabaseTest, OriginUsage) {
   AppCacheDatabase db(kEmptyPath);
   EXPECT_TRUE(db.LazyOpen(true));
 
-  db.db_->set_error_callback(base::Bind(&IgnoreDatabaseErrors));
-
   std::vector<AppCacheDatabase::CacheRecord> cache_records;
   EXPECT_EQ(0, db.GetOriginUsage(kOrigin));
   EXPECT_TRUE(db.FindCachesForOrigin(kOrigin, &cache_records));
diff --git a/webkit/browser/database/databases_table_unittest.cc b/webkit/browser/database/databases_table_unittest.cc
index 61b665b..09023f9 100644
--- a/webkit/browser/database/databases_table_unittest.cc
+++ b/webkit/browser/database/databases_table_unittest.cc
@@ -7,19 +7,11 @@
 #include "base/strings/utf_string_conversions.h"
 #include "sql/connection.h"
 #include "sql/statement.h"
+#include "sql/test/scoped_error_ignorer.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/sqlite/sqlite3.h"
 #include "webkit/browser/database/databases_table.h"
 
-namespace {
-
-// Set this error callback to "handle" errors by ignoring them.  This
-// causes the sql/ machinery to not throw fatal reports and allow
-// higher-level code to return failures.
-void IgnoreDatabaseErrors(int error, sql::Statement* stmt) {
-}
-
-}  // namespace
-
 namespace webkit_database {
 
 static void CheckDetailsAreEqual(const DatabaseDetails& d1,
@@ -40,7 +32,13 @@ TEST(DatabasesTableTest, TestIt) {
   // Initialize the 'Databases' table.
   sql::Connection db;
 
-  db.set_error_callback(base::Bind(&IgnoreDatabaseErrors));
+  sql::ScopedErrorIgnorer ignore_errors;
+  // TODO(shess): Suppressing SQLITE_CONSTRAINT because the code
+  // expects that and handles the resulting error.  Consider revising
+  // the code to use INSERT OR IGNORE (which would not throw
+  // SQLITE_CONSTRAINT) and then check ChangeCount() to see if any
+  // changes were made.
+  ignore_errors.IgnoreError(SQLITE_CONSTRAINT);
 
   // Initialize the temp dir and the 'Databases' table.
   EXPECT_TRUE(db.OpenInMemory());
@@ -143,6 +141,8 @@ TEST(DatabasesTableTest, TestIt) {
   // Check that trying to delete a record that doesn't exist fails.
   EXPECT_FALSE(databases_table.DeleteDatabaseDetails(
       "unknown_origin", ASCIIToUTF16("unknown_database")));
+
+  ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
 }
 
 }  // namespace webkit_database
diff --git a/webkit/browser/dom_storage/dom_storage_database.cc b/webkit/browser/dom_storage/dom_storage_database.cc
index 8a94fc8..f3199b6 100644
--- a/webkit/browser/dom_storage/dom_storage_database.cc
+++ b/webkit/browser/dom_storage/dom_storage_database.cc
@@ -15,18 +15,6 @@ namespace {
 
 const base::FilePath::CharType kJournal[] = FILE_PATH_LITERAL("-journal");
 
-void DatabaseErrorCallback(int error, sql::Statement* stmt) {
-  // Without a callback to ignore errors,
-  // DomStorageDatabaseTest.TestCanOpenFileThatIsNotADatabase fails with:
-  // ERROR:connection.cc(735)] sqlite error 522, errno 0: disk I/O error
-  // FATAL:connection.cc(750)] disk I/O error
-  // <backtrace>
-  // <crash>
-  //
-  // TODO(shess): If/when infrastructure lands which can allow tests
-  // to handle SQLite errors appropriately, remove this.
-}
-
 }  // anon namespace
 
 namespace dom_storage {
@@ -162,7 +150,6 @@ bool DomStorageDatabase::LazyOpen(bool create_if_needed) {
 
   db_.reset(new sql::Connection());
   db_->set_histogram_tag("DomStorageDatabase");
-  db_->set_error_callback(base::Bind(&DatabaseErrorCallback));
 
   if (file_path_.empty()) {
     // This code path should only be triggered by unit tests.
diff --git a/webkit/browser/dom_storage/dom_storage_database_unittest.cc b/webkit/browser/dom_storage/dom_storage_database_unittest.cc
index d3d4ae2..d1be10b 100644
--- a/webkit/browser/dom_storage/dom_storage_database_unittest.cc
+++ b/webkit/browser/dom_storage/dom_storage_database_unittest.cc
@@ -10,7 +10,9 @@
 #include "base/path_service.h"
 #include "base/strings/utf_string_conversions.h"
 #include "sql/statement.h"
+#include "sql/test/scoped_error_ignorer.h"
 #include "testing/gtest/include/gtest/gtest.h"
+#include "third_party/sqlite/sqlite3.h"
 
 namespace dom_storage {
 
@@ -345,6 +347,9 @@ TEST(DomStorageDatabaseTest, TestCanOpenFileThatIsNotADatabase) {
   file_util::WriteFile(file_name, kData, strlen(kData));
 
   {
+    sql::ScopedErrorIgnorer ignore_errors;
+    ignore_errors.IgnoreError(SQLITE_IOERR_SHORT_READ);
+
     // Try and open the file. As it's not a database, we should end up deleting
     // it and creating a new, valid file, so everything should actually
     // succeed.
@@ -356,9 +361,14 @@ TEST(DomStorageDatabaseTest, TestCanOpenFileThatIsNotADatabase) {
     EXPECT_TRUE(db.IsOpen());
 
     CheckValuesMatch(&db, values);
+
+    ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
   }
 
   {
+    sql::ScopedErrorIgnorer ignore_errors;
+    ignore_errors.IgnoreError(SQLITE_CANTOPEN);
+
     // Try to open a directory, we should fail gracefully and not attempt
     // to delete it.
     DomStorageDatabase db(temp_dir.path());
@@ -375,6 +385,8 @@ TEST(DomStorageDatabaseTest, TestCanOpenFileThatIsNotADatabase) {
     EXPECT_FALSE(db.IsOpen());
 
     EXPECT_TRUE(base::PathExists(temp_dir.path()));
+
+    ASSERT_TRUE(ignore_errors.CheckIgnoredErrors());
   }
 }