Reviewed by Maciej
authorbeidson <beidson@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Aug 2006 06:01:09 +0000 (06:01 +0000)
committerbeidson <beidson@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 23 Aug 2006 06:01:09 +0000 (06:01 +0000)
        The role of the SiteIcon is now the original intention - to be a cache of data relating to an Icon
        As such, I'm renaming it to IconDataCache.
        Also, the IconDatabase has to manually set the image data on the IconDataCache and also sets the
        TimeStamp when an icon is created or the data is changed.
        IconDataCache now has a method to write itself *to* a given database, instead of read itself from one.
        IconDatabase schema changes to have the timestamp set manually instead of via a trigger.
        The overall purpose of this change is to cache the timestamp, killing off a very common SQL query.

        * WebCore.xcodeproj/project.pbxproj: Renamed a file
        * loader/icon/IconDataCache.cpp: Added.
        (IconDataCache::IconDataCache):
        (IconDataCache::getImage): Now either returns the stored image, or 0 - no attempt to grab data
        (IconDataCache::manuallySetImageData): Delete the old image and create the new one
        (IconDataCache::writeToDatabase): Write the current iconURL, data, and timestamp to the given DB
        (IconDataCache::imageDataStatus): Determine if an IconDataCache is new without data versus actually having null data
        * loader/icon/IconDatabase.cpp:
        (WebCore::IconDatabase::createDatabaseTables): Changed DB schema to version 5 (hopefully the final version)
        (WebCore::IconDatabase::iconForPageURL):
        (WebCore::IconDatabase::isIconExpiredForIconURL): Uses the timestamp in the IconDataCache object instead of always querying
        (WebCore::IconDatabase::getOrCreateIconDataCache): Added, to handle creation of new IconDataCache when appropriate
        (WebCore::IconDatabase::setIconDataForIconURL): Puts data in SiteIcon then marks it for a future write
        (WebCore::IconDatabase::syncDatabase): Now syncs SiteIconsPendingUpdate
        * loader/icon/IconDatabase.h:
        (WebCore::IconDataCache::getTimestamp):
        (WebCore::IconDataCache::setTimestamp):
        * loader/icon/SiteIcon.cpp: Removed.

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

WebCore/ChangeLog
WebCore/WebCore.xcodeproj/project.pbxproj
WebCore/loader/icon/IconDataCache.cpp [new file with mode: 0644]
WebCore/loader/icon/IconDatabase.cpp
WebCore/loader/icon/IconDatabase.h
WebCore/loader/icon/SiteIcon.cpp [deleted file]

index d5c8659e91c07916300d29b05187f9527e007041..6bba1d3c93856e313be4219ae4ece69972363907 100644 (file)
@@ -1,3 +1,35 @@
+2006-08-22  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Maciej
+
+        The role of the SiteIcon is now the original intention - to be a cache of data relating to an Icon
+        As such, I'm renaming it to IconDataCache.
+        Also, the IconDatabase has to manually set the image data on the IconDataCache and also sets the 
+        TimeStamp when an icon is created or the data is changed.
+        IconDataCache now has a method to write itself *to* a given database, instead of read itself from one.
+        IconDatabase schema changes to have the timestamp set manually instead of via a trigger.
+        The overall purpose of this change is to cache the timestamp, killing off a very common SQL query.
+
+        * WebCore.xcodeproj/project.pbxproj: Renamed a file
+        * loader/icon/IconDataCache.cpp: Added.
+        (IconDataCache::IconDataCache):
+        (IconDataCache::getImage): Now either returns the stored image, or 0 - no attempt to grab data
+        (IconDataCache::manuallySetImageData): Delete the old image and create the new one
+        (IconDataCache::writeToDatabase): Write the current iconURL, data, and timestamp to the given DB
+        (IconDataCache::imageDataStatus): Determine if an IconDataCache is new without data versus actually having null data
+        * loader/icon/IconDatabase.cpp:
+        (WebCore::IconDatabase::createDatabaseTables): Changed DB schema to version 5 (hopefully the final version)
+        (WebCore::IconDatabase::iconForPageURL): 
+        (WebCore::IconDatabase::isIconExpiredForIconURL): Uses the timestamp in the IconDataCache object instead of always querying
+        (WebCore::IconDatabase::getOrCreateIconDataCache): Added, to handle creation of new IconDataCache when appropriate
+        (WebCore::IconDatabase::setIconDataForIconURL): Puts data in SiteIcon then marks it for a future write
+        (WebCore::IconDatabase::syncDatabase): Now syncs SiteIconsPendingUpdate
+        * loader/icon/IconDatabase.h:
+        (WebCore::IconDataCache::getTimestamp):
+        (WebCore::IconDataCache::setTimestamp):
+        * loader/icon/SiteIcon.cpp: Removed.
+
+
 2006-08-22  Justin Garcia  <justin.garcia@apple.com>
 
         Reviewed by harrison
index 30bb0d2a2b7030255a3f613fb4b402b8fc3fdf78..a66ac1268acdd4c14cded684b45f71aceeb7d6e9 100644 (file)
                5126E6BC0A2E3B12005C29FA /* IconDatabase.h in Headers */ = {isa = PBXBuildFile; fileRef = 5126E6BA0A2E3B12005C29FA /* IconDatabase.h */; settings = {ATTRIBUTES = (Private, ); }; };
                5126E6BF0A2E3B29005C29FA /* WebCoreIconDatabaseBridge.h in Headers */ = {isa = PBXBuildFile; fileRef = 5126E6BD0A2E3B29005C29FA /* WebCoreIconDatabaseBridge.h */; settings = {ATTRIBUTES = (Private, ); }; };
                5126E6C00A2E3B29005C29FA /* WebCoreIconDatabaseBridge.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5126E6BE0A2E3B29005C29FA /* WebCoreIconDatabaseBridge.mm */; };
-               516149ED0A525E3A003DFC7A /* SiteIcon.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 516149EC0A525E3A003DFC7A /* SiteIcon.cpp */; };
+               5186C0560A9C21470034FE94 /* IconDataCache.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 5186C0550A9C21470034FE94 /* IconDataCache.cpp */; };
                51D3EA160A3D24D300BADA35 /* SQLDatabase.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51D3EA130A3D24D300BADA35 /* SQLDatabase.cpp */; };
                51D3EA170A3D24D300BADA35 /* SQLDatabase.h in Headers */ = {isa = PBXBuildFile; fileRef = 51D3EA140A3D24D300BADA35 /* SQLDatabase.h */; };
                51D3EA180A3D24D300BADA35 /* SQLStatement.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 51D3EA150A3D24D300BADA35 /* SQLStatement.cpp */; };
                5126E6BE0A2E3B29005C29FA /* WebCoreIconDatabaseBridge.mm */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.objcpp; path = WebCoreIconDatabaseBridge.mm; sourceTree = "<group>"; };
                5150C2A10702629000AF642C /* WebDashboardRegion.h */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.c.h; path = WebDashboardRegion.h; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
                5150C2A50702629800AF642C /* WebDashboardRegion.m */ = {isa = PBXFileReference; fileEncoding = 30; indentWidth = 4; lastKnownFileType = sourcecode.c.objc; path = WebDashboardRegion.m; sourceTree = "<group>"; tabWidth = 8; usesTabs = 0; };
-               516149EC0A525E3A003DFC7A /* SiteIcon.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = SiteIcon.cpp; sourceTree = "<group>"; };
+               5186C0550A9C21470034FE94 /* IconDataCache.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = IconDataCache.cpp; sourceTree = "<group>"; };
                51D3EA130A3D24D300BADA35 /* SQLDatabase.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = SQLDatabase.cpp; sourceTree = "<group>"; };
                51D3EA140A3D24D300BADA35 /* SQLDatabase.h */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.c.h; path = SQLDatabase.h; sourceTree = "<group>"; };
                51D3EA150A3D24D300BADA35 /* SQLStatement.cpp */ = {isa = PBXFileReference; fileEncoding = 30; lastKnownFileType = sourcecode.cpp.cpp; path = SQLStatement.cpp; sourceTree = "<group>"; };
                5126E6B60A2E3AEF005C29FA /* icon */ = {
                        isa = PBXGroup;
                        children = (
-                               516149EC0A525E3A003DFC7A /* SiteIcon.cpp */,
                                5126E6B90A2E3B12005C29FA /* IconDatabase.cpp */,
                                5126E6BA0A2E3B12005C29FA /* IconDatabase.h */,
+                               5186C0550A9C21470034FE94 /* IconDataCache.cpp */,
                                51D3EA130A3D24D300BADA35 /* SQLDatabase.cpp */,
                                51D3EA140A3D24D300BADA35 /* SQLDatabase.h */,
                                51D3EA150A3D24D300BADA35 /* SQLStatement.cpp */,
                                448A29C00A46D9CB0030759F /* JSHTMLOptionsCollection.cpp in Sources */,
                                448AD27C0A48137A0023D179 /* JSHTMLOptionsCollectionCustom.cpp in Sources */,
                                E1052C320A4D70010072D99B /* DOMEventsNonstandard.mm in Sources */,
-                               516149ED0A525E3A003DFC7A /* SiteIcon.cpp in Sources */,
                                1A1D13810A5325520064BF5F /* DOMXPath.mm in Sources */,
                                93A1EAA00A5634C9006960A0 /* ImageDocumentMac.mm in Sources */,
                                1A0D57360A5C77FE007EDD4C /* OverflowEvent.cpp in Sources */,
                                1A750D8D0A90E521000FF215 /* JSNodeIterator.cpp in Sources */,
                                1A750DD40A90E729000FF215 /* JSNodeIteratorCustom.cpp in Sources */,
                                1A750E340A90F89F000FF215 /* JSTreeWalkerCustom.cpp in Sources */,
+                               5186C0560A9C21470034FE94 /* IconDataCache.cpp in Sources */,
                        );
                        runOnlyForDeploymentPostprocessing = 0;
                };
diff --git a/WebCore/loader/icon/IconDataCache.cpp b/WebCore/loader/icon/IconDataCache.cpp
new file mode 100644 (file)
index 0000000..d1c27cb
--- /dev/null
@@ -0,0 +1,121 @@
+/*
+ * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
+ * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
+ * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
+ * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
+ * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
+ */
+#include "IconDatabase.h"
+
+#include "Logging.h"
+#include "Image.h"
+#include <limits.h>
+
+using namespace WebCore;
+
+IconDataCache::IconDataCache(const String& url)
+    : m_iconURL(url)
+    , m_stamp(0)
+    , m_image(0)
+    , m_dataSet(false)
+{
+
+}
+
+IconDataCache::~IconDataCache()
+{
+    // Upon destruction of a IconDataCache, its image should no longer be in use anywhere
+    delete m_image;
+}
+
+Image* IconDataCache::getImage(const IntSize& size)
+{
+    // FIXME rdar://4680377 - For size right now, we are returning our one and only image and the Bridge
+    // is resizing it in place.  We need to actually store all the original representations here and return a native
+    // one, or resize the best one to the requested size and cache that result.
+    if (m_image)
+        return m_image;
+    
+    return 0;
+}
+
+void IconDataCache::setImageData(unsigned char* data, int size)
+{
+    if (!data)
+        ASSERT(!size);
+
+    // It's okay to delete the raw image data here. Any existing clients using this icon will be
+    // managing an image that was created with a copy of this raw image data.
+    if (m_image)
+        delete m_image;
+    m_image = new Image();
+
+    // Copy the provided data into the buffer of the new Image object
+    Vector<char>& dataBuffer = m_image->dataBuffer();
+    dataBuffer.resize(size);
+    memcpy(dataBuffer.data(), data, size);
+    
+    // Tell the Image object that all of its data has been set
+    if (!m_image->setData(true)) {
+        LOG(IconDatabase, "Manual image data for iconURL '%s' FAILED - it was probably invalid image data", m_iconURL.ascii().data());
+        delete m_image;
+        m_image = 0;
+    }
+    
+    m_dataSet = true;
+}
+
+void IconDataCache::writeToDatabase(SQLDatabase& db)
+{
+    if (m_iconURL.isEmpty())
+        return;
+    
+    // First we create and prepare the SQLStatement    
+    String escapedIconURL = m_iconURL;
+    escapedIconURL.replace('\'', "''");
+    // The following statement works no matter what in version 5 of the DB schema because we have the table
+    // replace any url entry with the new data if it already exists
+    SQLStatement sql(db, "INSERT INTO Icon (url,stamp,data) VALUES ('" + escapedIconURL + "', ?, ?);");
+    sql.prepare();
+        
+    // Then we bind the timestamp
+    sql.bindInt64(1, m_stamp);
+        
+    // Then, if we *have* data, we bind it.  Otherwise the DB will get "null" for the blob data, 
+    // signifying that this icon doesn't have any data
+    if (m_image && !m_image->dataBuffer().isEmpty())
+        sql.bindBlob(2, m_image->dataBuffer().data(), m_image->dataBuffer().size());
+    
+    // Finally we step and make sure the step was successful
+    if (sql.step() != SQLITE_DONE)
+        LOG_ERROR("Unable to set icon data for IconURL %s", m_iconURL.ascii().data());
+}
+
+ImageDataStatus IconDataCache::imageDataStatus()
+{
+    if (!m_dataSet)
+        return ImageDataStatusUnknown;
+    if (!m_image)
+        return ImageDataStatusMissing;
+    return ImageDataStatusPresent;
+}
+
+    
+
index ff890d5f7ffbcb2376d19a58c8560891a8861b8d..c1214cf51a31a62615cbb055c3fbdc33c22b8db0 100644 (file)
@@ -47,7 +47,7 @@ IconDatabase* IconDatabase::m_sharedInstance = 0;
 // Currently, an out-of-date schema causes the DB to be wiped and reset.  This isn't 
 // so bad during development but in the future, we would need to write a conversion
 // function to advance older released schemas to "current"
-const int IconDatabase::currentDatabaseVersion = 4;
+const int IconDatabase::currentDatabaseVersion = 5;
 
 // Icons expire once a day
 const int IconDatabase::iconExpirationTime = 60*60*24; 
@@ -203,16 +203,11 @@ void IconDatabase::createDatabaseTables(SQLDatabase& db)
         db.close();
         return;
     }
-    if (!db.executeCommand("CREATE TABLE Icon (iconID INTEGER PRIMARY KEY AUTOINCREMENT, url TEXT NOT NULL UNIQUE ON CONFLICT FAIL, stamp INTEGER, data BLOB);")) {
+    if (!db.executeCommand("CREATE TABLE Icon (iconID INTEGER PRIMARY KEY AUTOINCREMENT UNIQUE ON CONFLICT REPLACE, url TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE, stamp INTEGER, data BLOB);")) {
         LOG_ERROR("Could not create Icon table in database (%i) - %s", db.lastError(), db.lastErrorMsg());
         db.close();
         return;
     }
-    if (!db.executeCommand("CREATE TRIGGER update_icon_timestamp AFTER UPDATE ON Icon BEGIN UPDATE Icon SET stamp = strftime('%s','now') WHERE iconID = new.iconID; END;")) {
-        LOG_ERROR("Could not create timestamp updater in database (%i) - %s", db.lastError(), db.lastErrorMsg());
-        db.close();
-        return;
-    }
     if (!db.executeCommand("CREATE TABLE IconDatabaseInfo (key TEXT NOT NULL ON CONFLICT FAIL UNIQUE ON CONFLICT REPLACE,value TEXT NOT NULL ON CONFLICT FAIL);")) {
         LOG_ERROR("Could not create IconDatabaseInfo table in database (%i) - %s", db.lastError(), db.lastErrorMsg());
         db.close();
@@ -264,15 +259,16 @@ Image* IconDatabase::iconForPageURL(const String& pageURL, const IntSize& size,
     if (iconURL.isEmpty())
         return 0;
     
-    // If we do, maybe we have a SiteIcon for this IconURL
-    if (m_iconURLToSiteIcons.contains(iconURL)) {
-        SiteIcon* icon = m_iconURLToSiteIcons.get(iconURL);
-        return icon->getImage(size);
+    // If we do, maybe we have a IconDataCache for this IconURL
+    IconDataCache* icon = getOrCreateIconDataCache(iconURL);
+    
+    // If it's a new IconDataCache object that doesn't have its imageData set yet,
+    // we'll read in that image data now
+    if (icon->imageDataStatus() == ImageDataStatusUnknown) {
+        Vector<unsigned char> data = imageDataForIconURL(iconURL);
+        icon->setImageData(data.data(), data.size());
     }
         
-    // If we don't have either, we have to create the SiteIcon
-    SiteIcon* icon = new SiteIcon(iconURL);
-    m_iconURLToSiteIcons.set(iconURL, icon);
     return icon->getImage(size);
 }
 
@@ -283,6 +279,12 @@ bool IconDatabase::isIconExpiredForIconURL(const String& iconURL)
     if (iconURL.isEmpty()) 
         return true;
     
+    // If we have a IconDataCache, then it definitely has the Timestamp in it
+    IconDataCache* icon = m_iconURLToIconDataCacheMap.get(iconURL);
+    if (icon) 
+        return time(NULL) - icon->getTimestamp() > iconExpirationTime;
+            
+    // Otherwise, we'll get the timestamp from the DB and use it
     int stamp;
     if (m_privateBrowsingEnabled) {
         stamp = timeStampForIconURLQuery(m_privateBrowsingDB, iconURL);
@@ -293,6 +295,7 @@ bool IconDatabase::isIconExpiredForIconURL(const String& iconURL)
     stamp = timeStampForIconURLQuery(m_mainDB, iconURL);
     if (stamp)
         return (time(NULL) - stamp) > iconExpirationTime;
+    
     return false;
 }
     
@@ -391,9 +394,9 @@ void IconDatabase::retainIconURL(const String& iconURL)
 {
     ASSERT(!iconURL.isEmpty());
     
-    if (m_iconURLToRetainCount.contains(iconURL)) {
-        ASSERT(m_iconURLToRetainCount.get(iconURL) > 0);
-        m_iconURLToRetainCount.set(iconURL, m_iconURLToRetainCount.get(iconURL) + 1);
+    if (int retainCount = m_iconURLToRetainCount.get(iconURL)) {
+        ASSERT(retainCount > 0);
+        m_iconURLToRetainCount.set(iconURL, retainCount + 1);
     } else {
         m_iconURLToRetainCount.set(iconURL, 1);
         if (m_iconURLsPendingDeletion.contains(iconURL))
@@ -493,6 +496,32 @@ void IconDatabase::forgetIconForIconURLFromDatabase(const String& iconURL)
         LOG_ERROR("Unable to drop all PageURL for IconURL", iconURL.ascii().data()); 
 }
 
+IconDataCache* IconDatabase::getOrCreateIconDataCache(const String& iconURL)
+{
+    IconDataCache* icon;
+    if ((icon = m_iconURLToIconDataCacheMap.get(iconURL)))
+        return icon;
+        
+    icon = new IconDataCache(iconURL);
+    m_iconURLToIconDataCacheMap.set(iconURL, icon);
+    
+    // Get the most current time stamp for this IconURL
+    int timestamp = 0;
+    if (m_privateBrowsingEnabled)
+        timestamp = timeStampForIconURLQuery(m_privateBrowsingDB, iconURL);
+    if (!timestamp)
+        timestamp = timeStampForIconURLQuery(m_mainDB, iconURL);
+        
+    // If we can't get a timestamp for this URL, then it is a new icon and we initialize its timestamp now
+    if (!timestamp) {
+        icon->setTimestamp(time(NULL));
+        m_iconDataCachesPendingUpdate.add(icon);
+    } else 
+        icon->setTimestamp(timestamp);
+        
+    return icon;
+}
+
 void IconDatabase::setIconDataForIconURL(const void* data, int size, const String& iconURL)
 {
     ASSERT(size > -1);
@@ -504,30 +533,17 @@ void IconDatabase::setIconDataForIconURL(const void* data, int size, const Strin
     if (iconURL.isEmpty())
         return;
     
-    // First, if we already have a SiteIcon in memory, let's update its image data
-    if (m_iconURLToSiteIcons.contains(iconURL))
-        m_iconURLToSiteIcons.get(iconURL)->manuallySetImageData((unsigned char*)data, size);
-
-    // Next, we actually commit the image data to the database
-    // Start by making sure there's an entry for this IconURL in the current database
-    int64_t iconID = establishIconIDForIconURL(*m_currentDB, iconURL, true);
-    ASSERT(iconID);
+    // Get the IconDataCache for this IconURL (note, IconDataCacheForIconURL will create it if necessary)
+    IconDataCache* icon = getOrCreateIconDataCache(iconURL);
     
-    // First we create and prepare the SQLStatement
-    // The following statement also works to set the icon data to NULL because sqlite defaults unbound ? parameters to NULL
-    SQLStatement sql(*m_currentDB, "UPDATE Icon SET data = ? WHERE iconID = ?;");
-    sql.prepare();
-        
-    // Then we bind the icondata and iconID to the SQLStatement
-    if (data)
-        sql.bindBlob(1, data, size);
-    sql.bindInt64(2, iconID);
-    
-    // Finally we step and make sure the step was successful
-    if (sql.step() != SQLITE_DONE)
-        LOG_ERROR("Unable to set icon data for iconURL %s", iconURL.ascii().data());
-        
-    return;
+    // Set the data in the IconDataCache
+    icon->setImageData((unsigned char*)data, size);
+    
+    // Update the timestamp in the IconDataCache to NOW
+    icon->setTimestamp(time(NULL));
+
+    // Mark the IconDataCache as requiring an update to the database
+    m_iconDataCachesPendingUpdate.add(icon);
 }
 
 void IconDatabase::setHaveNoIconForIconURL(const String& iconURL)
@@ -567,7 +583,7 @@ void IconDatabase::setIconURLForPageURL(const String& iconURL, const String& pag
     m_pageURLToIconURLMap.set(pageURL, iconURL);
     
     // And mark this mapping to be added to the database
-    m_pageURLsPendingAddition.set(pageURL, iconURL);
+    m_pageURLsPendingAddition.add(pageURL);
     
     // Then start the timer to commit this change - or further delay the timer if it
     // was already started
@@ -646,28 +662,35 @@ void IconDatabase::syncDatabase()
 #endif
 
     // First we'll do the pending additions
-    for (HashMap<String, String>::iterator i = m_pageURLsPendingAddition.begin(); i != m_pageURLsPendingAddition.end(); ++i) {
-        // The HashMap maps PageURL->IconURL, but the method takes IconURL then PageURL - so this ordering is correct
-        setIconURLForPageURLInDatabase(i->second, i->first);
-        LOG(IconDatabase, "Commited IconURL %s for PageURL %s to database", (i->second).ascii().data(), (i->first).ascii().data());
+    // Starting with the IconDataCaches that need updating/insertion
+    for (HashSet<IconDataCache*>::iterator i = m_iconDataCachesPendingUpdate.begin(), end = m_iconDataCachesPendingUpdate.end(); i != end; ++i) {
+        (*i)->writeToDatabase(*m_currentDB);
+        LOG(IconDatabase, "Wrote IconDataCache for IconURL %s with timestamp of %lli to the DB", (*i)->getIconURL().ascii().data(), (*i)->getTimestamp());
+    }
+    m_iconDataCachesPendingUpdate.clear();
+    
+    HashSet<String>::iterator i = m_pageURLsPendingAddition.begin(), end = m_pageURLsPendingAddition.end();
+    for (; i != end; ++i) {
+        setIconURLForPageURLInDatabase(m_pageURLToIconURLMap.get(*i), *i);
+        LOG(IconDatabase, "Commited IconURL for PageURL %s to database", (*i).ascii().data());
     }
     m_pageURLsPendingAddition.clear();
     
     // Then we'll do the pending deletions
     // First lets wipe all the pageURLs
-    HashSet<String>::iterator i = m_pageURLsPendingDeletion.begin();
-    for (; i != m_pageURLsPendingDeletion.end(); ++i) {    
+    for (i = m_pageURLsPendingDeletion.begin(), end = m_pageURLsPendingDeletion.end(); i != end; ++i) {    
         forgetPageURL(*i);
         LOG(IconDatabase, "Deleted PageURL %s", (*i).ascii().data());
     }
     m_pageURLsPendingDeletion.clear();
 
     // Then get rid of all traces of the icons and IconURLs
-    SiteIcon* icon;    
-    for (i = m_iconURLsPendingDeletion.begin(); i != m_iconURLsPendingDeletion.end(); ++i) {
-        // Forget the SiteIcon
-        icon = m_iconURLToSiteIcons.get(*i);
-        m_iconURLToSiteIcons.remove(*i);
+    IconDataCache* icon;    
+    for (i = m_iconURLsPendingDeletion.begin(), end = m_iconURLsPendingDeletion.end(); i != end; ++i) {
+        // Forget the IconDataCache
+        icon = m_iconURLToIconDataCacheMap.get(*i);
+        if (icon)
+            m_iconURLToIconDataCacheMap.remove(*i);
         delete icon;
         
         // Forget the IconURL from the database
@@ -693,7 +716,7 @@ bool IconDatabase::hasEntryForIconURL(const String& iconURL)
         return false;
         
     // First check the in memory mapped icons...
-    if (m_iconURLToSiteIcons.contains(iconURL))
+    if (m_iconURLToIconDataCacheMap.contains(iconURL))
         return true;
 
     // Then we'll check the main database
index 1469c418bc62dcc57334ce4571697bbee4f42e95..aca575c5501704c253b239165a0dff093f507244 100644 (file)
@@ -50,28 +50,38 @@ namespace WTF {
 namespace WebCore { 
 
 class Image;
+
+typedef enum {
+    ImageDataStatusPresent, ImageDataStatusMissing, ImageDataStatusUnknown
+} ImageDataStatus;
     
-class SiteIcon {
+class IconDataCache {
 public:
-    SiteIcon(const String& url); 
-    ~SiteIcon();
+    IconDataCache(const String& url); 
+    ~IconDataCache();
     
-    void resetExpiration(time_t newExpiration = 0);
-    time_t getExpiration();
+    time_t getTimestamp() { return m_stamp; }
+    void setTimestamp(time_t stamp) { m_stamp = stamp; }
         
     Image* getImage(const IntSize&);    
     String getIconURL() { return m_iconURL; }
 
-    void manuallySetImageData(unsigned char* data, int size);
+    void setImageData(unsigned char* data, int size);
+    
+    void writeToDatabase(SQLDatabase& db);
+    
+    ImageDataStatus imageDataStatus();
+    
 private:
     String m_iconURL;
-    time_t m_expire;
+    time_t m_stamp;
     Image* m_image;
     
-    // This allows us to cache whether or not a SiteIcon has queried the database for its data yet
-    // We assume we only have to do that once per object and any subsequent updating of the image
-    // data will be via manuallySetImageData()
-    bool m_dataQueried;
+    // This allows us to cache whether or not a SiteIcon has had its data set yet
+    // This helps the IconDatabase know if it has to set the data on a new object or not,
+    // and also to determine if the icon is missing data or if it just hasn't been brought
+    // in from the DB yet
+    bool m_dataSet;
     
     // FIXME - Right now WebCore::Image doesn't have a very good API for accessing multiple representations
     // Even the NSImage way of doing things that we do in WebKit isn't very clean...  once we come up with a 
@@ -129,6 +139,9 @@ private:
     // This tries to get the iconID for the IconURL and, if it doesn't exist and createIfNecessary is true,
     // it will create the entry and return the new iconID
     int64_t establishIconIDForIconURL(SQLDatabase&, const String&, bool createIfNecessary = true);
+    
+    // This method returns the SiteIcon for the given IconURL and, if it doesn't exist it creates it first
+    IconDataCache* getOrCreateIconDataCache(const String& iconURL);
 
     // Remove traces of the given pageURL
     void forgetPageURL(const String& pageURL);
@@ -142,7 +155,7 @@ private:
     void removeAllIcons();
     
     // Called by the startup timer, this method removes all icons that are unretained
-    // after initial retains are complete
+    // after initial retains are complete, and pageURLs that are dangling
     void pruneUnretainedIconsOnStartup(Timer<IconDatabase>*);
     
     // Called by the prune timer, this method periodically removes all the icons in the pending-deletion
@@ -192,11 +205,11 @@ private:
     
     bool m_initialPruningComplete;
     
-    typedef HashMap<String, SiteIcon*> SiteIconMap;
-    SiteIconMap m_iconURLToSiteIcons;
+    HashMap<String, IconDataCache*> m_iconURLToIconDataCacheMap;
+    HashSet<IconDataCache*> m_iconDataCachesPendingUpdate;
     
     HashMap<String, String> m_pageURLToIconURLMap;
-    HashMap<String, String> m_pageURLsPendingAddition;
+    HashSet<String> m_pageURLsPendingAddition;
 
     // This will keep track of the retaincount for each pageURL
     HashMap<String, int> m_pageURLToRetainCount;
diff --git a/WebCore/loader/icon/SiteIcon.cpp b/WebCore/loader/icon/SiteIcon.cpp
deleted file mode 100644 (file)
index b542597..0000000
+++ /dev/null
@@ -1,107 +0,0 @@
-/*
- * Copyright (C) 2006 Apple Computer, Inc.  All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY APPLE COMPUTER, INC. ``AS IS'' AND ANY
- * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
- * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL APPLE COMPUTER, INC. OR
- * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL,
- * EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
- * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR
- * PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY
- * OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. 
- */
-#include "IconDatabase.h"
-
-#include "Logging.h"
-#include "Image.h"
-#include <limits.h>
-
-using namespace WebCore;
-
-SiteIcon::SiteIcon(const String& url)
-    : m_iconURL(url)
-    , m_image(0)
-    , m_dataQueried(false)
-{
-
-}
-
-SiteIcon::~SiteIcon()
-{
-    // Upon destruction of a SiteIcon, its image should no longer be in use anywhere
-    delete m_image;
-}
-
-Image* SiteIcon::getImage(const IntSize& size)
-{
-    // FIXME - For size right now, we are resizing our one-and-only shared Image
-    // What we really need to do is keep a hashmap of Images based on size and make a copy when/if we create a new one
-    if (m_image)
-        return m_image;
-    if (m_dataQueried)
-        return 0;
-    
-    if (IconDatabase::m_sharedInstance) {
-        Vector<unsigned char> imageData = IconDatabase::m_sharedInstance->imageDataForIconURL(m_iconURL);
-        LOG(IconDatabase, "For URL %s, we got a buffer of size %i", m_iconURL.ascii().data(), imageData.size());
-        
-        m_dataQueried = true;
-        if (!imageData.size())
-            return 0;
-        manuallySetImageData(imageData.data(), imageData.size());
-        
-        return m_image;
-    }
-    return 0;
-}
-
-void SiteIcon::manuallySetImageData(unsigned char* data, int size)
-{
-    if (!data)
-        ASSERT(!size);
-        
-    NativeBytePtr nativeData = 0;
-    // FIXME - Any other platform will need their own method to create NativeBytePtr from the void*
-#ifdef __APPLE__
-    nativeData = CFDataCreate(NULL, data, size);
-#endif
-
-    // It's okay to delete the raw image data here. Any existing clients using this icon will be
-    // managing an image that was created with a copy of this raw image data.
-    // FIXME: The IconDatabase interface would be more robust if it were cleaned up to never
-    // expose this raw image data.
-    if (m_image)
-        delete m_image;
-    m_image = new Image();
-
-    if (!m_image->setNativeData(nativeData, true)) {
-        LOG(IconDatabase, "Manual image data for iconURL '%s' FAILED", m_iconURL.ascii().data());
-        delete m_image;
-        m_image = 0;
-    }
-}
-
-void SiteIcon::resetExpiration(time_t newExpiration)
-{
-    // FIXME - Write expiration time to SQL
-}
-
-time_t SiteIcon::getExpiration()
-{
-    // FIXME - Return expiration time from SQL
-    return INT_MAX;
-}
-    
-