WebCore: 1. Fix a bug in SQLiteTransaction: do not assume that COMMIT always

succeeds.
2. Jump straight to the transaction error callback when a
statement fails in a way that makes sqlite automatically rollback
the transaction.
3. Fix the code that handles the "quota reached" failure, as it is
one of the failures that lead to an automatic transaction
rollback.

Reviewed by Eric Seidel.

https://bugs.webkit.org/show_bug.cgi?id=34280

* platform/sql/SQLiteDatabase.cpp:
(WebCore::SQLiteDatabase::isAutoCommitOn):
* platform/sql/SQLiteDatabase.h:
* platform/sql/SQLiteTransaction.cpp:
(WebCore::SQLiteTransaction::begin):
(WebCore::SQLiteTransaction::commit):
(WebCore::SQLiteTransaction::rollback):
(WebCore::SQLiteTransaction::transactionWasRolledBackBySqlite):
* platform/sql/SQLiteTransaction.h:
* storage/SQLTransaction.cpp:
(WebCore::SQLTransaction::SQLTransaction):
(WebCore::SQLTransaction::runStatements):
(WebCore::SQLTransaction::runCurrentStatement):
(WebCore::SQLTransaction::handleCurrentStatementError):
(WebCore::SQLTransaction::deliverQuotaIncreaseCallback):

LayoutTests: 1. Enhance quota-tracking.html: if sqlite automatically rolls back
a transaction because of a statement failure, make sure the rest
of the statements in the transaction are not executed.
2. Fix the expectations for quota-tracking.html. Sqlite cannot
recover from reaching a DB's max size.

Reviewed by Eric Seidel.

* storage/quota-tracking-expected.txt:
* storage/quota-tracking.html:



git-svn-id: svn://svn.chromium.org/blink/trunk@54393 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/LayoutTests/ChangeLog

+2010-02-04  Dumitru Daniliuc  <dumi@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        1. Enhance quota-tracking.html: if sqlite automatically rolls back
+        a transaction because of a statement failure, make sure the rest
+        of the statements in the transaction are not executed.
+        2. Fix the expectations for quota-tracking.html. Sqlite cannot
+        recover from reaching a DB's max size.
+
+        * storage/quota-tracking-expected.txt:
+        * storage/quota-tracking.html:
+
 2010-02-04  Csaba Osztrogonác  <ossy@webkit.org>
 
         Unreviewed typo fix for r54379.

third_party/WebKit/LayoutTests/storage/quota-tracking-expected.txt

 UI DELEGATE DATABASE CALLBACK: exceededDatabaseQuotaForSecurityOrigin:{file, , 0} database:QuotaManagementDatabase2
 This test checks to make sure that quotas are enforced per-origin instead of per-database, as they were prior to http://trac.webkit.org/projects/webkit/changeset/29983.
-The test clears all databases, sets the quota for the origin to 32k, then tries to insert 17k of data into two databases. If things go as planned, the UI Delegate will be informed of the exceeded quota.
+The test clears all databases, sets the quota for the origin to 32k, then tries to insert 17k of data into two databases. If things go as planned, the UI Delegate will be informed of the exceeded quota and should increase the quota for this origin. Inserting 17k of data the third time should succeed again.
 Adding a table
 Inserting some data
 Done adding data
 Adding a table
 Inserting some data
+Expected quota exception - there was not enough remaining storage space, or the storage quota was reached and the user declined to allow more space
+Done adding data
+Adding a table
+Inserting some data
 Done adding data
 Test Complete
 

third_party/WebKit/LayoutTests/storage/quota-tracking.html

@@ -3,6 +3,7 @@
 <script>
 var database1;
 var database2;
+var database3;
 
 function log(message)
 {
@@ -16,19 +17,33 @@ function finishTest()
         layoutTestController.notifyDone();
 }
 
-function errorFunction(error)
+function statementErrorFunction(tx, error)
 {
-    log("Test failed - " + error.message);
+    log("Unexpected exception - " + error.message);
     finishTest();
 }
 
+function transactionErrorFunction(db, error)
+{
+    // We only expect an error message for database2
+    if (db == database2) {
+        log("Expected quota exception - " + error.message);
+        checkCompletion(db);
+    } else {
+        log("Unexpected exception - " + error.message);
+        finishTest();
+    }
+}
+
 function checkCompletion(db)
 {
     log("Done adding data");
 
     db.complete = true;
-    if (database1.complete && database2.complete)
+    if (database1.complete && database2.complete && database3.complete)
         finishTest();
+    else if (database2.complete)
+        testDatabase(database3);
     else
         testDatabase(database2);
 }
@@ -38,9 +53,20 @@ function addData(db)
     db.transaction(function(tx) {
         log("Inserting some data");
         tx.executeSql("INSERT INTO DataTest (randomData) VALUES (ZEROBLOB(17408))", [],
-                      function(tx, result) { },
-                      function(tx, error) { errorFunction(error); });
-    }, errorFunction, function() {
+                      function(tx, result) { }, statementErrorFunction);
+        if (db == database2) {
+            // Try to run this statement on 'database2' only.
+            // It should not be run, because the previous statement should've
+            // resulted in a failure that made sqlite roll back the entire transaction.
+            tx.executeSql("INSERT INTO DataTest (randomData) VALUES (ZEROBLOB(10))", [],
+                          function(tx, result) {
+                              log("This statement should not have been run.");
+                              finishTest();
+                          }, statementErrorFunction);
+        }
+    }, function(error) {
+        transactionErrorFunction(db, error);
+    }, function() {
         checkCompletion(db);
     });
 }
@@ -50,9 +76,10 @@ function testDatabase(db)
     db.transaction(function(tx) {
         log("Adding a table");
         tx.executeSql("CREATE TABLE DataTest (randomData)", [],
-                      function(tx, result) { },
-                      function(tx, error) { errorFunction(error); });
-    }, errorFunction, function() {
+                      function(tx, result) { }, statementErrorFunction);
+    }, function(error) {
+        transactionErrorFunction(db, error);
+    }, function() {
         addData(db);
     });
 }
@@ -69,8 +96,10 @@ function runTest()
 
     database1 = openDatabase("QuotaManagementDatabase1", "1.0", "Test for quota management <rdar://5628468>", 1);
     database2 = openDatabase("QuotaManagementDatabase2", "1.0", "Test for quota management <rdar://5628468>", 1);
+    database3 = openDatabase("QuotaManagementDatabase3", "1.0", "Test for quota management <rdar://5628468>", 1);
     database1.complete = false;
     database2.complete = false;
+    database3.complete = false;
 
     testDatabase(database1);
 }
@@ -80,7 +109,7 @@ function runTest()
 
 <body onload="runTest()">
 This test checks to make sure that quotas are enforced per-origin instead of per-database, as they were prior to http://trac.webkit.org/projects/webkit/changeset/29983.<br>
-The test clears all databases, sets the quota for the origin to 32k, then tries to insert 17k of data into two databases.  If things go as planned, the UI Delegate will be informed of the exceeded quota.
+The test clears all databases, sets the quota for the origin to 32k, then tries to insert 17k of data into two databases. If things go as planned, the UI Delegate will be informed of the exceeded quota and should increase the quota for this origin. Inserting 17k of data the third time should succeed again.
 <pre id="console">
 </pre>
 </body>

third_party/WebKit/WebCore/ChangeLog

+2010-02-04  Dumitru Daniliuc  <dumi@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        1. Fix a bug in SQLiteTransaction: do not assume that COMMIT always
+        succeeds.
+        2. Jump straight to the transaction error callback when a
+        statement fails in a way that makes sqlite automatically rollback
+        the transaction.
+        3. Fix the code that handles the "quota reached" failure, as it is
+        one of the failures that lead to an automatic transaction
+        rollback.
+
+        https://bugs.webkit.org/show_bug.cgi?id=34280
+
+        * platform/sql/SQLiteDatabase.cpp:
+        (WebCore::SQLiteDatabase::isAutoCommitOn):
+        * platform/sql/SQLiteDatabase.h:
+        * platform/sql/SQLiteTransaction.cpp:
+        (WebCore::SQLiteTransaction::begin):
+        (WebCore::SQLiteTransaction::commit):
+        (WebCore::SQLiteTransaction::rollback):
+        (WebCore::SQLiteTransaction::transactionWasRolledBackBySqlite):
+        * platform/sql/SQLiteTransaction.h:
+        * storage/SQLTransaction.cpp:
+        (WebCore::SQLTransaction::SQLTransaction):
+        (WebCore::SQLTransaction::runStatements):
+        (WebCore::SQLTransaction::runCurrentStatement):
+        (WebCore::SQLTransaction::handleCurrentStatementError):
+        (WebCore::SQLTransaction::deliverQuotaIncreaseCallback):
+
 2010-02-04  Peter Kasting  <pkasting@google.com>
 
         Not reviewed, rollback.

third_party/WebKit/WebCore/platform/sql/SQLiteDatabase.cpp

@@ -361,4 +361,9 @@ void SQLiteDatabase::unlock()
     m_lockingMutex.unlock();
 }
 
+bool SQLiteDatabase::isAutoCommitOn() const
+{
+    return sqlite3_get_autocommit(m_db);
+}
+
 } // namespace WebCore

third_party/WebKit/WebCore/platform/sql/SQLiteDatabase.h

@@ -106,6 +106,7 @@ public:
     // (un)locks the database like a mutex
     void lock();
     void unlock();
+    bool isAutoCommitOn() const;
 
 private:
     static int authorizerFunction(void*, int, const char*, const char*, const char*, const char*);

third_party/WebKit/WebCore/platform/sql/SQLiteTransaction.cpp

@@ -55,33 +55,31 @@ void SQLiteTransaction::begin()
         // http://www.sqlite.org/lang_transaction.html
         // http://www.sqlite.org/lockingv3.html#locking
         if (m_readOnly)
-            m_inProgress = m_db.executeCommand("BEGIN;");
+            m_inProgress = m_db.executeCommand("BEGIN");
         else
-            m_inProgress = m_db.executeCommand("BEGIN IMMEDIATE;");
+            m_inProgress = m_db.executeCommand("BEGIN IMMEDIATE");
         m_db.m_transactionInProgress = m_inProgress;
     }
 }
 
 void SQLiteTransaction::commit()
 {
-    // FIXME: this code is buggy; it assumes that COMMIT always succeeds which is not the case:
-    // the transaction could've been silently rolled back before getting to the COMMIT statement
-    // (https://bugs.webkit.org/show_bug.cgi?id=34280). However, the rest of the code does not
-    // know how to deal with a premature rollback and a failed COMMIT at this moment, so until
-    // we figure out what to do with bug 34280, it's better to leave this code as it is.
     if (m_inProgress) {
         ASSERT(m_db.m_transactionInProgress);
-        m_db.executeCommand("COMMIT;");
-        m_inProgress = false;
-        m_db.m_transactionInProgress = false;
+        m_inProgress = !m_db.executeCommand("COMMIT");
+        m_db.m_transactionInProgress = m_inProgress;
     }
 }
 
 void SQLiteTransaction::rollback()
 {
+    // We do not use the 'm_inProgress = m_db.executeCommand("ROLLBACK")' construct here,
+    // because m_inProgress should always be set to false after a ROLLBACK, and
+    // m_db.executeCommand("ROLLBACK") can sometimes harmlessly fail, thus returning
+    // a non-zero/true result (http://www.sqlite.org/lang_transaction.html).
     if (m_inProgress) {
         ASSERT(m_db.m_transactionInProgress);
-        m_db.executeCommand("ROLLBACK;");
+        m_db.executeCommand("ROLLBACK");
         m_inProgress = false;
         m_db.m_transactionInProgress = false;
     }
@@ -95,4 +93,11 @@ void SQLiteTransaction::stop()
     }
 }
 
+bool SQLiteTransaction::wasRolledBackBySqlite() const
+{
+    // According to http://www.sqlite.org/c3ref/get_autocommit.html,
+    // the auto-commit flag should be off in the middle of a transaction
+    return m_inProgress && m_db.isAutoCommitOn();
+}
+
 } // namespace WebCore

third_party/WebKit/WebCore/platform/sql/SQLiteTransaction.h

@@ -44,6 +44,7 @@ public:
     void stop();
     
     bool inProgress() const { return m_inProgress; }
+    bool wasRolledBackBySqlite() const;
 private:
     SQLiteDatabase& m_db;
     bool m_inProgress;

third_party/WebKit/WebCore/storage/SQLTransaction.cpp

@@ -313,7 +313,7 @@ void SQLTransaction::runStatements()
     // If there is a series of statements queued up that are all successful and have no associated
     // SQLStatementCallback objects, then we can burn through the queue
     do {
-        if (m_shouldRetryCurrentStatement) {
+        if (m_shouldRetryCurrentStatement && !m_sqliteTransaction->wasRolledBackBySqlite()) {
             m_shouldRetryCurrentStatement = false;
             // FIXME - Another place that needs fixing up after <rdar://problem/5628468> is addressed.
             // See ::openTransactionAndPreflight() for discussion
@@ -393,8 +393,8 @@ bool SQLTransaction::runCurrentStatement()
 void SQLTransaction::handleCurrentStatementError()
 {
     // Transaction Steps 6.error - Call the statement's error callback, but if there was no error callback,
-    // jump to the transaction error callback
-    if (m_currentStatement->hasStatementErrorCallback()) {
+    // or the transaction was rolled back, jump to the transaction error callback
+    if (m_currentStatement->hasStatementErrorCallback() && !m_sqliteTransaction->wasRolledBackBySqlite()) {
         m_nextStep = &SQLTransaction::deliverStatementCallback;
         LOG(StorageAPI, "Scheduling deliverStatementCallback for transaction %p\n", this);
         m_database->scheduleTransactionCallback(this);