Regression(r277571) Call to SQLiteDatabase::turnOnIncrementalAutoVacu…

…um() from ITP fails

https://bugs.webkit.org/show_bug.cgi?id=229886
<rdar://82581507>

Reviewed by Alex Christensen.

Before r277571, `lastError()` was getting called after the call to `statement->columnInt(0)`
and *before* the call to `statement.finalize()`. After r277571, the call to `statement.finalize()`
became implicit, and gets called when `statement` does out of scope. As a result, `lastError()`
was getting called *after* the statement finalization instead of after. This is an issue because
the code expects the last error to be for `statement->columnInt(0)` and thus expects SQLITE_ROW.
I moved the `lastError()` call so that it now after between `statement->columnInt(0)` and the
implicit statement finalization, like it used to.

* platform/sql/SQLiteDatabase.cpp:
(WebCore::SQLiteDatabase::turnOnIncrementalAutoVacuum):


Canonical link: https://commits.webkit.org/241334@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@282030 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,22 @@
+2021-09-03  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r277571) Call to SQLiteDatabase::turnOnIncrementalAutoVacuum() from ITP fails
+        https://bugs.webkit.org/show_bug.cgi?id=229886
+        <rdar://82581507>
+
+        Reviewed by Alex Christensen.
+
+        Before r277571, `lastError()` was getting called after the call to `statement->columnInt(0)`
+        and *before* the call to `statement.finalize()`. After r277571, the call to `statement.finalize()`
+        became implicit, and gets called when `statement` does out of scope. As a result, `lastError()`
+        was getting called *after* the statement finalization instead of after. This is an issue because
+        the code expects the last error to be for `statement->columnInt(0)` and thus expects SQLITE_ROW.
+        I moved the `lastError()` call so that it now after between `statement->columnInt(0)` and the
+        implicit statement finalization, like it used to.
+
+        * platform/sql/SQLiteDatabase.cpp:
+        (WebCore::SQLiteDatabase::turnOnIncrementalAutoVacuum):
+
 2021-09-03  Devin Rousso  <drousso@apple.com>
 
         [Web App Manifest] Always fetch the first manifest if provided

Source/WebCore/platform/sql/SQLiteDatabase.cpp

@@ -622,18 +622,19 @@ bool SQLiteDatabase::isAutoCommitOn() const
 
 bool SQLiteDatabase::turnOnIncrementalAutoVacuum()
 {
-    int autoVacuumMode = 0;
-    if (auto statement = prepareStatement("PRAGMA auto_vacuum"_s))
-        autoVacuumMode = statement->columnInt(0);
-    int error = lastError();
+    auto statement = prepareStatement("PRAGMA auto_vacuum"_s);
+    if (!statement)
+        return false;
+
+    int autoVacuumMode = statement->columnInt(0);
 
     // Check if we got an error while trying to get the value of the auto_vacuum flag.
     // If we got a SQLITE_BUSY error, then there's probably another transaction in
     // progress on this database. In this case, keep the current value of the
     // auto_vacuum flag and try to set it to INCREMENTAL the next time we open this
     // database. If the error is not SQLITE_BUSY, then we probably ran into a more
     // serious problem and should return false (to log an error message).
-    if (error != SQLITE_ROW)
+    if (lastError() != SQLITE_ROW)
         return false;
 
     switch (autoVacuumMode) {
@@ -646,8 +647,7 @@ bool SQLiteDatabase::turnOnIncrementalAutoVacuum()
         if (!executeCommand("PRAGMA auto_vacuum = 2"_s))
             return false;
         runVacuumCommand();
-        error = lastError();
-        return (error == SQLITE_OK);
+        return lastError() == SQLITE_OK;
     }
 }