WebCore:

        Reviewed by Adam

        In very specific circumstances, prepared SQLStatements can become invalid without
        any warning.  This checks for that state and re-prepares the statement and also
        adds more aggressive error-checking everywhere these statements are used.

        * loader/icon/IconDatabase.cpp:
        (WebCore::readySQLStatement): Check if the statement is expired - reprepare it
        (WebCore::IconDatabase::imageDataForIconURLQuery): More thoroughly catch error cases
        (WebCore::IconDatabase::timeStampForIconURLQuery): Ditto
        (WebCore::IconDatabase::iconURLForPageURLQuery): Ditto
        (WebCore::IconDatabase::forgetPageURLQuery): Ditto
        (WebCore::IconDatabase::setIconIDForPageURLQuery): Ditto
        (WebCore::IconDatabase::getIconIDForIconURLQuery): Ditto
        (WebCore::IconDatabase::addIconForIconURLQuery): Ditto
        (WebCore::IconDatabase::hasIconForIconURLQuery): Ditto
        * loader/icon/SQLStatement.cpp:
        (WebCore::SQLStatement::isExpired): Added (accessor to sqlite3_expired())
        * loader/icon/SQLStatement.h:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@16595 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index e8ae5f4e2a7966fbbb2392a9e64409d07b66722e..e0f728f0631b20cfc5f31a178a7ae1141b2a8204 100644 (file)
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2006-09-27  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Adam
+
+        In very specific circumstances, prepared SQLStatements can become invalid without
+        any warning.  This checks for that state and re-prepares the statement and also
+        adds more aggressive error-checking everywhere these statements are used.
+
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::readySQLStatement): Check if the statement is expired - reprepare it
+        (WebCore::IconDatabase::imageDataForIconURLQuery): More thoroughly catch error cases
+        (WebCore::IconDatabase::timeStampForIconURLQuery): Ditto
+        (WebCore::IconDatabase::iconURLForPageURLQuery): Ditto
+        (WebCore::IconDatabase::forgetPageURLQuery): Ditto
+        (WebCore::IconDatabase::setIconIDForPageURLQuery): Ditto
+        (WebCore::IconDatabase::getIconIDForIconURLQuery): Ditto
+        (WebCore::IconDatabase::addIconForIconURLQuery): Ditto
+        (WebCore::IconDatabase::hasIconForIconURLQuery): Ditto
+        * loader/icon/SQLStatement.cpp:
+        (WebCore::SQLStatement::isExpired): Added (accessor to sqlite3_expired())
+        * loader/icon/SQLStatement.h:
+
 2006-09-26  Darin Adler  <darin@apple.com>
 
         Reviewed by Steve.

diff --git a/WebCore/loader/icon/IconDatabase.cpp b/WebCore/loader/icon/IconDatabase.cpp
index 8b50f5af1627db079ae051256bf049a3e801b359..e3971c170d0aa70f09f6d403097e29c11f821cef 100644 (file)
--- a/WebCore/loader/icon/IconDatabase.cpp
+++ b/WebCore/loader/icon/IconDatabase.cpp
@@ -928,13 +928,16 @@ IconDatabase::~IconDatabase()
 // 2 - Lazy construction of the Statement in the first place, in case we've never made this query before
 inline void readySQLStatement(SQLStatement*& statement, SQLDatabase& db, const String& str)
 {
-    if (statement && statement->database() != &db) {
+    if (statement && (statement->database() != &db || statement->isExpired())) {
+        if (statement->isExpired())
+            LOG(IconDatabase, "SQLStatement associated with %s is expired", str.ascii().data());
         delete statement;
         statement = 0;
     }
     if (!statement) {
         statement = new SQLStatement(db, str);
-        statement->prepare();
+        int result = statement->prepare();
+        ASSERT(result == SQLResultOk);
     }
 }
 
@@ -951,12 +954,18 @@ bool IconDatabase::pageURLTableIsEmptyQuery(SQLDatabase& db)
     return !SQLStatement(db, "SELECT iconID FROM PageURL LIMIT 1;").returnsAtLeastOneResult();
 }
 
-void IconDatabase::imageDataForIconURLQuery(SQLDatabase& db, const String& iconURL, Vector<unsigned char>& result)
+void IconDatabase::imageDataForIconURLQuery(SQLDatabase& db, const String& iconURL, Vector<unsigned char>& imageData)
 {
     readySQLStatement(m_imageDataForIconURLStatement, db, "SELECT Icon.data FROM Icon WHERE Icon.url = (?);");
     m_imageDataForIconURLStatement->bindText16(1, iconURL, false);
-    m_imageDataForIconURLStatement->step();
-    m_imageDataForIconURLStatement->getColumnBlobAsVector(0, result);
+    
+    int result = m_imageDataForIconURLStatement->step();
+    imageData.clear();
+    if (result == SQLResultRow)
+        m_imageDataForIconURLStatement->getColumnBlobAsVector(0, imageData);
+    else if (result != SQLResultDone)
+        LOG_ERROR("imageDataForIconURLQuery failed");
+
     m_imageDataForIconURLStatement->reset();
 }
 
@@ -964,8 +973,16 @@ int IconDatabase::timeStampForIconURLQuery(SQLDatabase& db, const String& iconUR
 {
     readySQLStatement(m_timeStampForIconURLStatement, db, "SELECT Icon.stamp FROM Icon WHERE Icon.url = (?);");
     m_timeStampForIconURLStatement->bindText16(1, iconURL, false);
-    m_timeStampForIconURLStatement->step();
-    int result = m_timeStampForIconURLStatement->getColumnInt(0);
+
+    int result = m_timeStampForIconURLStatement->step();
+    if (result == SQLResultRow)
+        result = m_timeStampForIconURLStatement->getColumnInt(0);
+    else {
+        if (result != SQLResultDone)
+            LOG_ERROR("timeStampForIconURLQuery failed");
+        result = 0;
+    }
+
     m_timeStampForIconURLStatement->reset();
     return result;
 }
@@ -974,17 +991,26 @@ String IconDatabase::iconURLForPageURLQuery(SQLDatabase& db, const String& pageU
 {
     readySQLStatement(m_iconURLForPageURLStatement, db, "SELECT Icon.url FROM Icon, PageURL WHERE PageURL.url = (?) AND Icon.iconID = PageURL.iconID;");
     m_iconURLForPageURLStatement->bindText16(1, pageURL, false);
-    m_iconURLForPageURLStatement->step();
-    String result = m_iconURLForPageURLStatement->getColumnText16(0);
+    
+    int result = m_iconURLForPageURLStatement->step();
+    String iconURL;
+    if (result == SQLResultRow)
+        iconURL = m_iconURLForPageURLStatement->getColumnText16(0);
+    else if (result != SQLResultDone)
+        LOG_ERROR("iconURLForPageURLQuery failed");
+    
     m_iconURLForPageURLStatement->reset();
-    return result;
+    return iconURL;
 }
 
 void IconDatabase::forgetPageURLQuery(SQLDatabase& db, const String& pageURL)
 {
     readySQLStatement(m_forgetPageURLStatement, db, "DELETE FROM PageURL WHERE url = (?);");
     m_forgetPageURLStatement->bindText16(1, pageURL, false);
-    m_forgetPageURLStatement->step();
+
+    if (m_forgetPageURLStatement->step() != SQLResultOk)
+        LOG_ERROR("forgetPageURLQuery failed");
+    
     m_forgetPageURLStatement->reset();
 }
 
@@ -993,7 +1019,10 @@ void IconDatabase::setIconIDForPageURLQuery(SQLDatabase& db, int64_t iconID, con
     readySQLStatement(m_setIconIDForPageURLStatement, db, "INSERT INTO PageURL (url, iconID) VALUES ((?), ?);");
     m_setIconIDForPageURLStatement->bindText16(1, pageURL, false);
     m_setIconIDForPageURLStatement->bindInt64(2, iconID);
-    m_setIconIDForPageURLStatement->step();
+
+    if (m_setIconIDForPageURLStatement->step() != SQLResultOk)
+        LOG_ERROR("setIconIDForPageURLQuery failed");
+
     m_setIconIDForPageURLStatement->reset();
 }
 
@@ -1001,8 +1030,16 @@ int64_t IconDatabase::getIconIDForIconURLQuery(SQLDatabase& db, const String& ic
 {
     readySQLStatement(m_getIconIDForIconURLStatement, db, "SELECT Icon.iconID FROM Icon WHERE Icon.url = (?);");
     m_getIconIDForIconURLStatement->bindText16(1, iconURL, false);
-    m_getIconIDForIconURLStatement->step();
-    int64_t result = m_getIconIDForIconURLStatement->getColumnInt64(0);
+    
+    int64_t result = m_getIconIDForIconURLStatement->step();
+    if (result == SQLResultRow)
+        result = m_getIconIDForIconURLStatement->getColumnInt64(0);
+    else {
+        if (result != SQLResultDone)
+            LOG_ERROR("getIconIDForIconURLQuery failed");
+        result = 0;
+    }
+
     m_getIconIDForIconURLStatement->reset();
     return result;
 }
@@ -1011,9 +1048,15 @@ int64_t IconDatabase::addIconForIconURLQuery(SQLDatabase& db, const String& icon
 {
     readySQLStatement(m_addIconForIconURLStatement, db, "INSERT INTO Icon (url) VALUES ((?));");
     m_addIconForIconURLStatement->bindText16(1, iconURL, false);
-    int64_t result = 0;
-    if (m_addIconForIconURLStatement->step() == SQLResultOk)
+    
+    int64_t result = m_addIconForIconURLStatement->step();
+    if (result== SQLResultOk)
         result = db.lastInsertRowID();
+    else {
+        LOG_ERROR("addIconForIconURLQuery failed");
+        result = 0;
+    }
+
     m_addIconForIconURLStatement->reset();
     return result;
 }
@@ -1022,9 +1065,14 @@ bool IconDatabase::hasIconForIconURLQuery(SQLDatabase& db, const String& iconURL
 {
     readySQLStatement(m_hasIconForIconURLStatement, db, "SELECT Icon.iconID FROM Icon WHERE Icon.url = (?);");
     m_hasIconForIconURLStatement->bindText16(1, iconURL, false);
-    bool result = m_hasIconForIconURLStatement->step() == SQLResultRow;
+
+    int result = m_hasIconForIconURLStatement->step();
+
+    if (result != SQLResultRow && result != SQLResultDone)
+        LOG_ERROR("hasIconForIconURLQuery failed");
+
     m_hasIconForIconURLStatement->reset();
-    return result;
+    return result == SQLResultRow;
 }
 
 } //namespace WebCore

diff --git a/WebCore/loader/icon/SQLStatement.cpp b/WebCore/loader/icon/SQLStatement.cpp
index 8a545362800a14274a61038224be761d0498edc9..1406acbfe2e366f9c3855ddddd657e2dcc381b89 100644 (file)
--- a/WebCore/loader/icon/SQLStatement.cpp
+++ b/WebCore/loader/icon/SQLStatement.cpp
@@ -373,4 +373,9 @@ bool SQLStatement::returnDoubleResults(int col, Vector<double>& v)
     return result;
 }
 
+bool SQLStatement::isExpired()\r
+{\r
+    return m_statement ? sqlite3_expired(m_statement) : true;\r
+}\r
+
 } // namespace WebCore

diff --git a/WebCore/loader/icon/SQLStatement.h b/WebCore/loader/icon/SQLStatement.h
index 7f74c3672b51e60174e2cbbb57f0d30d1584f591..a199847c82b7e4e1ab1d41765d82ee43a9029872 100644 (file)
--- a/WebCore/loader/icon/SQLStatement.h
+++ b/WebCore/loader/icon/SQLStatement.h
@@ -64,7 +64,9 @@ public:
     // returns true is step() returns SQLITE_ROW
     // returns false otherwise
     bool returnsAtLeastOneResult();
-    
+
+    bool isExpired();
+
     // Returns -1 on last-step failing.  Otherwise, returns number of rows
     // returned in the last step()
     int columnCount();