sql: Disable trigger execution for sql::Database users.

The use of SQLite triggers is discouraged in Chrome feature code.

This CL disables trigger execution from databases that use sql::Database
infrastructure -- this applies to all Chrome feature code, but not to
WebSQL, which uses its own SQLite abstraction. CREATE TRIGGER and DROP
TRIGGER will still work, but the triggers will not be executed.

Disabling trigger execution in Chrome features is the best we can do for
now, because WebSQL still needs SQLite support for triggers. After we
remove WebSQL, we can remove all trigger support from SQLite.

Bug: 910955
Change-Id: I1f994c67341e44253f1f78188a1eaa7c4d966f88
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3020167
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Auto-Submit: Victor Costan <pwnall@chromium.org>
Reviewed-by: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#900805}

sql/README.md

@@ -393,6 +393,10 @@ to disable SQLite's CHECK constraint support using
 Triggers significantly increase the difficulty of reviewing and maintaining
 Chrome features that use them.
 
+Triggers are not executed on SQLite databases opened with Chrome's
+`sql::Database` infrastructure. This is intended to steer feature developers
+away from the discouraged feature.
+
 After [WebSQL](https://www.w3.org/TR/webdatabase/) is removed from Chrome, we
 plan to disable SQLite's trigger support using
 [SQLITE_OMIT_TRIGGER](https://sqlite.org/compile.html#omit_trigger).

sql/database.cc

@@ -1497,6 +1497,12 @@ bool Database::OpenInternal(const std::string& file_name,
       return false;
   }
 
+  // The use of triggers is discouraged for Chrome code. Thanks to this
+  // configuration change, triggers are not executed. CREATE TRIGGER and DROP
+  // TRIGGER still succeed.
+  err = sqlite3_db_config(db_, SQLITE_DBCONFIG_ENABLE_TRIGGER, 0, nullptr);
+  DCHECK_EQ(err, SQLITE_OK) << "sqlite3_db_config() should not fail";
+
   // 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

sql/database_unittest.cc

@@ -1387,6 +1387,27 @@ TEST_P(SQLDatabaseTest, GetMemoryUsage) {
       << "Page cache usage should go down after calling TrimMemory()";
 }
 
+TEST_P(SQLDatabaseTest, TriggersDisabledByDefault) {
+  ASSERT_TRUE(db_->Execute("CREATE TABLE data(id INTEGER)"));
+
+  // sqlite3_db_config() currently only disables running triggers. Schema
+  // operations on triggers are still allowed.
+  EXPECT_TRUE(
+      db_->Execute("CREATE TRIGGER trigger AFTER INSERT ON data "
+                   "BEGIN DELETE FROM data; END"));
+
+  ASSERT_TRUE(db_->Execute("INSERT INTO data(id) VALUES(42)"));
+
+  Statement select(db_->GetUniqueStatement("SELECT id FROM data"));
+  EXPECT_TRUE(select.Step())
+      << "If the trigger did not run, the table should not be empty.";
+  EXPECT_EQ(42, select.ColumnInt64(0));
+
+  // sqlite3_db_config() currently only disables running triggers. Schema
+  // operations on triggers are still allowed.
+  EXPECT_TRUE(db_->Execute("DROP TRIGGER IF EXISTS trigger"));
+}
+
 class SQLDatabaseTestExclusiveMode : public testing::Test,
                                      public testing::WithParamInterface<bool> {
  public:

sql/recovery_unittest.cc

@@ -780,16 +780,10 @@ TEST_F(SQLRecoveryTest, RecoverDatabase) {
   ASSERT_TRUE(
       db_.Execute("CREATE VIEW v AS SELECT x.v FROM x, y WHERE x.v = y.v"));
 
-  // When an element is deleted from [x], trigger a delete on [y].  Between the
-  // BEGIN and END, [old] stands for the deleted rows from [x].
-  ASSERT_TRUE(
-      db_.Execute("CREATE TRIGGER t AFTER DELETE ON x "
-                  "BEGIN DELETE FROM y WHERE y.v = old.v; END"));
-
   // Save aside a copy of the original schema, verifying that it has the created
   // items plus the sqlite_sequence table.
   const std::string orig_schema(GetSchema(&db_));
-  ASSERT_EQ(6, std::count(orig_schema.begin(), orig_schema.end(), '\n'));
+  ASSERT_EQ(5, std::count(orig_schema.begin(), orig_schema.end(), '\n'));
 
   static const char kXSql[] = "SELECT * FROM x ORDER BY 1";
   static const char kYSql[] = "SELECT * FROM y ORDER BY 1";
@@ -815,13 +809,6 @@ TEST_F(SQLRecoveryTest, RecoverDatabase) {
   EXPECT_EQ("bob|truck\ndean|trailer\njim|telephone",
             ExecuteWithResults(&db_, kYSql, "|", "\n"));
   EXPECT_EQ("trailer\ntruck", ExecuteWithResults(&db_, kVSql, "|", "\n"));
-
-  // Test that the trigger works.
-  ASSERT_TRUE(db_.Execute("DELETE FROM x WHERE v = 'truck'"));
-  EXPECT_EQ("1|turtle\n3|trailer", ExecuteWithResults(&db_, kXSql, "|", "\n"));
-  EXPECT_EQ("dean|trailer\njim|telephone",
-            ExecuteWithResults(&db_, kYSql, "|", "\n"));
-  EXPECT_EQ("trailer", ExecuteWithResults(&db_, kVSql, "|", "\n"));
 }
 
 // When RecoverDatabase() encounters SQLITE_NOTADB, the database is deleted.