Reviewed by Maciej
authorbeidson <beidson@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 9 Jul 2006 17:52:52 +0000 (17:52 +0000)
committerbeidson <beidson@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 9 Jul 2006 17:52:52 +0000 (17:52 +0000)
        Set the stage to remove the workaround for the SQLite BLOB corruption.  A few other small cleanups, and
        preparation for pruning unreferenced and unretained icons.

        * icon/IconDatabase.cpp:
        (WebCore::IconDatabase::recreateDatabase):  Added another trigger to assist in icon removal
        (WebCore::IconDatabase::deletePrivateTables):  Cleaned up logging messages
        (WebCore::IconDatabase::imageDataForIconID):  #ifdefed the blobbing hack for impending removal, use the real blob by default
        (WebCore::IconDatabase::imageDataForIconURL):  same
        (WebCore::IconDatabase::imageDataForPageURL):  same
        (WebCore::IconDatabase::pruneUnreferencedIcons):  Will delete any icons and their data that are not referenced
          by any PageURL
        * icon/IconDatabase.h:

        * icon/SQLDatabase.h:  Changed BlobAsVector to be unsigned char as thats the most reasonable type for
          a byte-buffer, and is what CFData expects
        * icon/SQLStatement.cpp:
        (WebCore::SQLStatement::getColumnBlobAsVector):

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

WebCore/ChangeLog
WebCore/icon/IconDatabase.cpp
WebCore/icon/IconDatabase.h
WebCore/icon/SQLDatabase.h
WebCore/icon/SQLStatement.cpp

index 0c68097e876f7d910ea25a30a951077b0cbf3366..930682a1929cb56ef0e748d970131e061b56adf2 100644 (file)
@@ -1,3 +1,25 @@
+2006-07-09  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Maciej
+
+        Set the stage to remove the workaround for the SQLite BLOB corruption.  A few other small cleanups, and
+        preparation for pruning unreferenced and unretained icons.
+
+        * icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::recreateDatabase):  Added another trigger to assist in icon removal
+        (WebCore::IconDatabase::deletePrivateTables):  Cleaned up logging messages
+        (WebCore::IconDatabase::imageDataForIconID):  #ifdefed the blobbing hack for impending removal, use the real blob by default
+        (WebCore::IconDatabase::imageDataForIconURL):  same
+        (WebCore::IconDatabase::imageDataForPageURL):  same
+        (WebCore::IconDatabase::pruneUnreferencedIcons):  Will delete any icons and their data that are not referenced
+          by any PageURL
+        * icon/IconDatabase.h:
+
+        * icon/SQLDatabase.h:  Changed BlobAsVector to be unsigned char as thats the most reasonable type for
+          a byte-buffer, and is what CFData expects
+        * icon/SQLStatement.cpp:
+        (WebCore::SQLStatement::getColumnBlobAsVector):
+
 2006-07-09  Darin Adler  <darin@apple.com>
 
         - move all but the last 12 files out of kwq directory
index c3ecca748457050b8dcf277f7fee639e7072b8ee..912c03694ac044c30fa5025e833dbd7f7cd3b7d8 100644 (file)
 #include <time.h>
 
 
-
 // FIXME - Make sure we put a private browsing consideration in that uses the temporary tables anytime private browsing would be an issue.
 
+// FIXME - One optimization to be made when this is no longer in flux is to make query construction smarter - that is queries that are created from
+// multiple strings and numbers should be handled differently than with String + String + String + etc.
+
 const char* DefaultIconDatabaseFilename = "/icon.db";
 
 namespace WebCore {
@@ -151,6 +153,12 @@ void IconDatabase::recreateDatabase()
         m_db.close();
         return;
     }
+    if (!m_db.executeCommand("CREATE TRIGGER delete_icon_resource AFTER DELETE ON Icon BEGIN DELETE FROM IconResource WHERE IconResource.iconID =  old.iconID; END;")) {
+        LOG_ERROR("Unable to create delete_icon_resource trigger in icon.db (%i) - %s", m_db.lastError(), m_db.lastErrorMsg());
+        m_db.close();
+        return;
+    }
+    
 }    
 
 void IconDatabase::createPrivateTables()
@@ -170,12 +178,9 @@ void IconDatabase::createPrivateTables()
 
 void IconDatabase::deletePrivateTables()
 {
-    if (!m_db.executeCommand("DROP TABLE TempPageURL;"))
-        LOG_ERROR("Could not drop TempPageURL table - %s", m_db.lastErrorMsg());
-    if (!m_db.executeCommand("DROP TABLE TempIcon;"))
-        LOG_ERROR("Could not drop TempIcon table - %s", m_db.lastErrorMsg());
-    if (!m_db.executeCommand("DROP TABLE TempIconResource;"))
-        LOG_ERROR("Could not drop TempIconResource table - %s", m_db.lastErrorMsg());
+    m_db.executeCommand("DROP TABLE TempPageURL;");
+    m_db.executeCommand("DROP TABLE TempIcon;");
+    m_db.executeCommand("DROP TABLE TempIconResource;");
 }
 
 // FIXME - This is a DIRTY, dirty workaround for a problem that we're seeing where certain blobs are having a corrupt buffer
@@ -183,6 +188,7 @@ void IconDatabase::deletePrivateTables()
 // does an in place hex-to-character from the textual representation of the icon data.  After I manage to follow up with Adam Swift, the OSX sqlite maintainer,
 // who is too busy to help me until after 7-4-06, this NEEEEEEEEEEEEEEDS to be changed. 
 // *SIGH*
+#ifdef BLOB_WORKAROUND
 unsigned char hexToUnsignedChar(UChar h, UChar l)
 {
     unsigned char c;
@@ -232,13 +238,18 @@ Vector<unsigned char> hexStringToVector(const String& s)
     LOG(IconDatabase, "Finished atoi() - %i iterations, result size %i", count, result.size());
     return result;
 }
+#endif
 
 Vector<unsigned char> IconDatabase::imageDataForIconID(int id)
 {
-    String blob = SQLStatement(m_db, String::sprintf("SELECT data FROM IconResource WHERE iconid = %i", id)).getColumnText(0);
+#ifdef BLOB_WORKAROUND
+    String blob = SQLStatement(m_db, String::sprintf("SELECT quote(data) FROM IconResource WHERE iconid = %i", id)).getColumnText(0);
     if (blob.isEmpty())
         return Vector<unsigned char>();
     return hexStringToVector(blob);
+#else
+    return SQLStatement(m_db, String::sprintf("SELECT data FROM IconResource WHERE iconid = %i", id)).getColumnBlobAsVector(0);
+#endif
 }
 
 Vector<unsigned char> IconDatabase::imageDataForIconURL(const String& _iconURL)
@@ -246,8 +257,11 @@ Vector<unsigned char> IconDatabase::imageDataForIconURL(const String& _iconURL)
     //Escape single quotes for SQL 
     String iconURL = _iconURL;
     iconURL.replace('\'', "''");
-    String blob;
     
+#ifdef BLOB_WORKAROUND
+    LOG(IconDatabase, "imageDataForIconURL called with BLOB_WORKAROUND set");
+    String blob;
+     
     // If private browsing is enabled, we'll check there first as the most up-to-date data for an icon will be there
     if (m_privateBrowsingEnabled) {
         blob = SQLStatement(m_db, "SELECT quote(TempIconResource.data) FROM TempIconResource, TempIcon WHERE TempIcon.url = '" + iconURL + "' AND TempIconResource.iconID = TempIcon.iconID;").getColumnText(0);
@@ -263,6 +277,20 @@ Vector<unsigned char> IconDatabase::imageDataForIconURL(const String& _iconURL)
         return Vector<unsigned char>();
     
     return hexStringToVector(blob);
+#else    
+    LOG(IconDatabase, "imageDataForIconURL called using preferred method");
+    // If private browsing is enabled, we'll check there first as the most up-to-date data for an icon will be there
+    if (m_privateBrowsingEnabled) {    
+        Vector<unsigned char> blob = SQLStatement(m_db, "SELECT TempIconResource.data FROM TempIconResource, TempIcon WHERE TempIcon.url = '" + iconURL + "' AND TempIconResource.iconID = TempIcon.iconID;").getColumnBlobAsVector(0);
+        if (!blob.isEmpty()) {
+            LOG(IconDatabase, "Icon data pulled from temp tables");
+            return blob;
+        }
+    } 
+    
+    // It wasn't found there, so lets check the main tables
+    return SQLStatement(m_db, "SELECT IconResource.data FROM IconResource, Icon WHERE Icon.url = '" + iconURL + "' AND IconResource.iconID = Icon.iconID;").getColumnBlobAsVector(0);
+#endif
 }
 
 Vector<unsigned char> IconDatabase::imageDataForPageURL(const String& _pageURL)
@@ -270,11 +298,12 @@ Vector<unsigned char> IconDatabase::imageDataForPageURL(const String& _pageURL)
     //Escape single quotes for SQL 
     String pageURL = _pageURL;
     pageURL.replace('\'', "''");
-    String blob;
     
+#ifdef BLOB_WORKAROUND
+    String blob;
     // If private browsing is enabled, we'll check there first as the most up-to-date data for an icon will be there
     if (m_privateBrowsingEnabled) {
-        blob = SQLStatement(m_db, "SELECT TempIconResource.data FROM TempIconResource, TempPageURL WHERE TempPageURL.url = '" + pageURL + "' AND TempIconResource.iconID = TempPageURL.iconID;").getColumnText(0);
+        blob = SQLStatement(m_db, "SELECT quote(TempIconResource.data) FROM TempIconResource, TempPageURL WHERE TempPageURL.url = '" + pageURL + "' AND TempIconResource.iconID = TempPageURL.iconID;").getColumnText(0);
         if (!blob.isEmpty()) {
             LOG(IconDatabase, "Icon data pulled from temp tables");
             return hexStringToVector(blob);
@@ -287,6 +316,20 @@ Vector<unsigned char> IconDatabase::imageDataForPageURL(const String& _pageURL)
         return Vector<unsigned char>();
     
     return hexStringToVector(blob);
+#else
+    LOG(IconDatabase, "imageDataForIconURL called using preferred method");
+    // If private browsing is enabled, we'll check there first as the most up-to-date data for an icon will be there
+    if (m_privateBrowsingEnabled) {    
+        Vector<unsigned char> blob = SQLStatement(m_db, "SELECT TempIconResource.data FROM TempIconResource, TempPageURL WHERE TempPageURL.url = '" + pageURL + "' AND TempIconResource.iconID = TempPageURL.iconID;").getColumnBlobAsVector(0);
+        if (!blob.isEmpty()) {
+            LOG(IconDatabase, "Icon data pulled from temp tables");
+            return blob;
+        }
+    } 
+    
+    // It wasn't found there, so lets check the main tables
+    return SQLStatement(m_db, "SELECT IconResource.data FROM IconResource, PageURL WHERE PageURL.url = '" + pageURL + "' AND IconResource.iconID = PageURL.iconID;").getColumnBlobAsVector(0);
+#endif
 }
 
 void IconDatabase::setPrivateBrowsingEnabled(bool flag)
@@ -491,6 +534,20 @@ void IconDatabase::performSetIconURLForPageURL(int64_t iconID, const String& pag
             LOG_ERROR("Failed to insert record into %s - %s", pageTable.ascii().data(), m_db.lastErrorMsg());
 }
 
+void IconDatabase::pruneUnreferencedIcons(int numberToPrune)
+{
+    if (!numberToPrune)
+        return;
+        
+    if (numberToPrune > 0)
+        if (!m_db.executeCommand(String::sprintf("DELETE FROM Icon WHERE iconID IN (SELECT Icon.iconID FROM Icon WHERE Icon.iconID NOT IN(SELECT PageURL.iconID FROM PageURL) LIMIT %i;", numberToPrune)))
+            LOG_ERROR("Failed to prune %i unreferenced icons from the DB", numberToPrune);
+    else
+        if (!m_db.executeCommand(String("DELETE FROM Icon WHERE iconID IN (SELECT Icon.iconID FROM Icon WHERE Icon.iconID NOT IN(SELECT PageURL.iconID FROM PageURL);")))
+            LOG_ERROR("Failed to prune all unreferenced icons from the DB");
+}
+
+
 bool IconDatabase::hasIconForIconURL(const String& _url)
 {
     // First check the in memory mapped icons...
index a441b4dd044e6fb6410f29859ec7a9f3117a87dd..f56d9fb1a19aa46aa20b145044eae5fa4d0f75b6 100644 (file)
@@ -90,7 +90,7 @@ public:
     bool open(const String& path);
     bool isOpen() { return m_db.isOpen(); }
     void close();
-
     Image* iconForPageURL(const String&, const IntSize&, bool cache = true);
     Image* iconForIconURL(const String&, const IntSize&, bool cache = true);
     String iconURLForPageURL(const String&);
@@ -111,6 +111,10 @@ public:
     void setHaveNoIconForIconURL(const String&);
     void setIconURLForPageURL(const String& iconURL, const String& pageURL);
 
+    // Removes icons from the db that no longer have any PageURLs pointing to them
+    // If numberToPrune is negative, ALL dangling icons will be pruned
+    void pruneUnreferencedIcons(int numberToPrune);
+
     static const int currentDatabaseVersion;    
 private:
     IconDatabase();
index cea0fb54b2fc059206bd297b87b1179e87c431a0..6f8c3b39fceb85ce8ae61babae7354792e1120cf 100644 (file)
@@ -136,7 +136,7 @@ public:
     int getColumnInt(int col);
     int64_t getColumnInt64(int col);
     const void* getColumnBlob(int col, int& size);
-    Vector<char> getColumnBlobAsVector(int col);
+    Vector<unsigned char> getColumnBlobAsVector(int col);
 
     bool returnTextResults(int col, Vector<String>& v);
     bool returnTextResults16(int col, Vector<String>& v);
index b43ffdb241d025c1197d7560483b256894965a77..974a79666c1039de3d5e5af58bb1ade840358f51 100644 (file)
@@ -76,7 +76,7 @@ int SQLStatement::finalize()
     return lastError();
 }
 
-int SQLStatement::reset()
+int SQLStatement::reset() 
 {
     if (m_statement) {
         return sqlite3_reset(m_statement);
@@ -215,23 +215,23 @@ int64_t SQLStatement::getColumnInt64(int col)
     return sqlite3_column_int64(m_statement, col);
 }
     
-Vector<char> SQLStatement::getColumnBlobAsVector(int col)
+Vector<unsigned char> SQLStatement::getColumnBlobAsVector(int col)
 {
     if (!m_statement)
         if (prepareAndStep() != SQLITE_ROW)
-            return Vector<char>();
+            return Vector<unsigned char>();
     if (columnCount() <= col)
-        return Vector<char>();
+        return Vector<unsigned char>();
  
     const void* blob = sqlite3_column_blob(m_statement, col);
-    if (blob) {
-        int size = sqlite3_column_bytes(m_statement, col);
-        Vector<char> result((size_t)size);
-        for (int i = 0; i < size; ++i)
-            result[i] = ((const char*)blob)[i];
-        return result;
-    } 
-    return Vector<char>();
+    if (!blob) 
+        return Vector<unsigned char>();
+        
+    int size = sqlite3_column_bytes(m_statement, col);
+    Vector<unsigned char> result((size_t)size);
+    for (int i = 0; i < size; ++i)
+        result[i] = ((const unsigned char*)blob)[i];
+    return result;
 }
 
 const void* SQLStatement::getColumnBlob(int col, int& size)