[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.20-204-g221d8e8

dumi at chromium.org dumi at chromium.org
Wed Feb 10 22:15:26 UTC 2010


The following commit has been merged in the webkit-1.1 branch:
commit e957a7619e9e54db2605050c8dea6e5438f7d27b
Author: dumi at chromium.org <dumi at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Fri Feb 5 02:09:21 2010 +0000

    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: http://svn.webkit.org/repository/webkit/trunk@54393 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index fb6bfe0..fc6b529 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2010-02-04  Dumitru Daniliuc  <dumi at 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 at webkit.org>
 
         Unreviewed typo fix for r54379.
diff --git a/LayoutTests/storage/quota-tracking-expected.txt b/LayoutTests/storage/quota-tracking-expected.txt
index 1243889..62a2c6b 100644
--- a/LayoutTests/storage/quota-tracking-expected.txt
+++ b/LayoutTests/storage/quota-tracking-expected.txt
@@ -1,11 +1,15 @@
 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
 
diff --git a/LayoutTests/storage/quota-tracking.html b/LayoutTests/storage/quota-tracking.html
index 7b47e7b..fb2cc9e 100644
--- a/LayoutTests/storage/quota-tracking.html
+++ b/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>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 85d4a09..391635f 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,34 @@
+2010-02-04  Dumitru Daniliuc  <dumi at 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 at google.com>
 
         Not reviewed, rollback.
diff --git a/WebCore/platform/sql/SQLiteDatabase.cpp b/WebCore/platform/sql/SQLiteDatabase.cpp
index d170db5..faaf5de 100644
--- a/WebCore/platform/sql/SQLiteDatabase.cpp
+++ b/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
diff --git a/WebCore/platform/sql/SQLiteDatabase.h b/WebCore/platform/sql/SQLiteDatabase.h
index 9982254..a3e9852 100644
--- a/WebCore/platform/sql/SQLiteDatabase.h
+++ b/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*);
diff --git a/WebCore/platform/sql/SQLiteTransaction.cpp b/WebCore/platform/sql/SQLiteTransaction.cpp
index a34613f..6f90eac 100644
--- a/WebCore/platform/sql/SQLiteTransaction.cpp
+++ b/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
diff --git a/WebCore/platform/sql/SQLiteTransaction.h b/WebCore/platform/sql/SQLiteTransaction.h
index 557d81c..924241f 100644
--- a/WebCore/platform/sql/SQLiteTransaction.h
+++ b/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;
diff --git a/WebCore/storage/SQLTransaction.cpp b/WebCore/storage/SQLTransaction.cpp
index db25e1a..754cebc 100644
--- a/WebCore/storage/SQLTransaction.cpp
+++ b/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);

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list