Fix a bug in QPixmapCache when the cache is trimmed by QCache.

There was a bug in QPixmapCache when QCache trims the content, some keys
were not invalidated. The ifdef for WinCE (that i removed) was a wrong
fix, it let the auto-test pass but it doesn't fix the bug. The approach
here is to add a QPixmapCacheEntry that release the key it owns when
QCache deletes it : we are now sure that nothing happen in our back.

Reviewed-by:paul
Reviewed-by:trond

5 files changed, 42 insertions, 36 deletions

diff --git a/src/gui/image/qimage.h b/src/gui/image/qimage.h
index 89d7de5..1ac56a7 100644
--- a/src/gui/image/qimage.h
+++ b/src/gui/image/qimage.h
@@ -314,7 +314,7 @@ private:
     QImageData *d;
 
     friend class QRasterPixmapData;
-    friend class QDetachedPixmap;
+    friend class QPixmapCacheEntry;
     friend Q_GUI_EXPORT qint64 qt_image_id(const QImage &image);
     friend const QVector<QRgb> *qt_image_colortable(const QImage &image);
 
diff --git a/src/gui/image/qpixmap.h b/src/gui/image/qpixmap.h
index a891637..d11bd03 100644
--- a/src/gui/image/qpixmap.h
+++ b/src/gui/image/qpixmap.h
@@ -270,7 +270,7 @@ private:
     friend class QWidgetPrivate;
     friend class QRasterPaintEngine;
     friend class QRasterBuffer;
-    friend class QDetachedPixmap;
+    friend class QPixmapCacheEntry;
 #if !defined(QT_NO_DATASTREAM)
     friend Q_GUI_EXPORT QDataStream &operator>>(QDataStream &, QPixmap &);
 #endif
diff --git a/src/gui/image/qpixmap_raster_p.h b/src/gui/image/qpixmap_raster_p.h
index 2af2399..da0405e 100644
--- a/src/gui/image/qpixmap_raster_p.h
+++ b/src/gui/image/qpixmap_raster_p.h
@@ -88,7 +88,7 @@ protected:
 private:
     friend class QPixmap;
     friend class QBitmap;
-    friend class QDetachedPixmap;
+    friend class QPixmapCacheEntry;
     friend class QRasterPaintEngine;
 };
 
diff --git a/src/gui/image/qpixmapcache.cpp b/src/gui/image/qpixmapcache.cpp
index 8029977..f12d397 100644
--- a/src/gui/image/qpixmapcache.cpp
+++ b/src/gui/image/qpixmapcache.cpp
@@ -167,7 +167,7 @@ QPixmapCache::Key &QPixmapCache::Key::operator =(const Key &other)
     return *this;
 }
 
-class QPMCache : public QObject, public QCache<QPixmapCache::Key, QDetachedPixmap>
+class QPMCache : public QObject, public QCache<QPixmapCache::Key, QPixmapCacheEntry>
 {
     Q_OBJECT
 public:
@@ -215,7 +215,7 @@ uint qHash(const QPixmapCache::Key &k)
 
 QPMCache::QPMCache()
     : QObject(0),
-      QCache<QPixmapCache::Key, QDetachedPixmap>(cache_limit * 1024),
+      QCache<QPixmapCache::Key, QPixmapCacheEntry>(cache_limit * 1024),
       keyArray(0), theid(0), ps(0), keyArraySize(0), freeKey(0), t(false)
 {
 }
@@ -238,7 +238,6 @@ void QPMCache::timerEvent(QTimerEvent *)
 {
     int mc = maxCost();
     bool nt = totalCost() == ps;
-    QList<QPixmapCache::Key> keys = QCache<QPixmapCache::Key, QDetachedPixmap>::keys();
     setMaxCost(nt ? totalCost() * 3 / 4 : totalCost() -1);
     setMaxCost(mc);
     ps = totalCost();
@@ -252,10 +251,6 @@ void QPMCache::timerEvent(QTimerEvent *)
             ++it;
         }
     }
-    for (int i = 0; i < keys.size(); ++i) {
-        if (!contains(keys.at(i)))
-            releaseKey(keys.at(i));
-    }
 
     if (!size()) {
         killTimer(theid);
@@ -274,11 +269,10 @@ QPixmap *QPMCache::object(const QString &key) const
         const_cast<QPMCache *>(this)->cacheKeys.remove(key);
         return 0;
     }
-    QPixmap *ptr = QCache<QPixmapCache::Key, QDetachedPixmap>::object(cacheKey);
+    QPixmap *ptr = QCache<QPixmapCache::Key, QPixmapCacheEntry>::object(cacheKey);
      //We didn't find the pixmap in the cache, the key is not valid anymore
     if (!ptr) {
         const_cast<QPMCache *>(this)->cacheKeys.remove(key);
-        const_cast<QPMCache *>(this)->releaseKey(cacheKey);
     }
     return ptr;
 }
@@ -286,7 +280,7 @@ QPixmap *QPMCache::object(const QString &key) const
 QPixmap *QPMCache::object(const QPixmapCache::Key &key) const
 {
     Q_ASSERT(key.d->isValid);
-    QPixmap *ptr = QCache<QPixmapCache::Key, QDetachedPixmap>::object(key);
+    QPixmap *ptr = QCache<QPixmapCache::Key, QPixmapCacheEntry>::object(key);
     //We didn't find the pixmap in the cache, the key is not valid anymore
     if (!ptr)
         const_cast<QPMCache *>(this)->releaseKey(key);
@@ -299,13 +293,14 @@ bool QPMCache::insert(const QString& key, const QPixmap &pixmap, int cost)
     QPixmapCache::Key oldCacheKey = cacheKeys.value(key);
     //If for the same key we add already a pixmap we should delete it
     if (oldCacheKey.d) {
-        QCache<QPixmapCache::Key, QDetachedPixmap>::remove(oldCacheKey);
-        cacheKey = oldCacheKey;
-    } else {
-         cacheKey = createKey();
-     }
+        QCache<QPixmapCache::Key, QPixmapCacheEntry>::remove(oldCacheKey);
+        cacheKeys.remove(key);
+    }
+
+    //we create a new key the old one has been removed
+    cacheKey = createKey();
 
-    bool success = QCache<QPixmapCache::Key, QDetachedPixmap>::insert(cacheKey, new QDetachedPixmap(pixmap), cost);
+    bool success = QCache<QPixmapCache::Key, QPixmapCacheEntry>::insert(cacheKey, new QPixmapCacheEntry(cacheKey, pixmap), cost);
     if (success) {
         cacheKeys.insert(key, cacheKey);
         if (!theid) {
@@ -322,7 +317,7 @@ bool QPMCache::insert(const QString& key, const QPixmap &pixmap, int cost)
 QPixmapCache::Key QPMCache::insert(const QPixmap &pixmap, int cost)
 {
     QPixmapCache::Key cacheKey = createKey();
-    bool success = QCache<QPixmapCache::Key, QDetachedPixmap>::insert(cacheKey, new QDetachedPixmap(pixmap), cost);
+    bool success = QCache<QPixmapCache::Key, QPixmapCacheEntry>::insert(cacheKey, new QPixmapCacheEntry(cacheKey, pixmap), cost);
     if (success) {
         if (!theid) {
             theid = startTimer(30000);
@@ -338,13 +333,21 @@ QPixmapCache::Key QPMCache::insert(const QPixmap &pixmap, int cost)
 bool QPMCache::replace(const QPixmapCache::Key &key, const QPixmap &pixmap, int cost)
 {
     Q_ASSERT(key.d->isValid);
-    //If for the same key we add already a pixmap we should delete it
-    QCache<QPixmapCache::Key, QDetachedPixmap>::remove(key);
+    //If for the same key we had already an entry so we should delete the pixmap and use the new one
+    QCache<QPixmapCache::Key, QPixmapCacheEntry>::remove(key);
+
+    QPixmapCache::Key cacheKey = createKey();
 
-    bool success = QCache<QPixmapCache::Key, QDetachedPixmap>::insert(key, new QDetachedPixmap(pixmap), cost);
-    if (success && !theid) {
-        theid = startTimer(30000);
-        t = false;
+    bool success = QCache<QPixmapCache::Key, QPixmapCacheEntry>::insert(cacheKey, new QPixmapCacheEntry(cacheKey, pixmap), cost);
+    if (success) {
+        if(!theid) {
+            theid = startTimer(30000);
+            t = false;
+        }
+        const_cast<QPixmapCache::Key&>(key) = cacheKey;
+    } else {
+        //Insertion failed we released the key
+        releaseKey(cacheKey);
     }
     return success;
 }
@@ -356,16 +359,12 @@ bool QPMCache::remove(const QString &key)
     if (!cacheKey.d)
         return false;
     cacheKeys.remove(key);
-    releaseKey(cacheKey);
-    return QCache<QPixmapCache::Key, QDetachedPixmap>::remove(cacheKey);
+    return QCache<QPixmapCache::Key, QPixmapCacheEntry>::remove(cacheKey);
 }
 
 bool QPMCache::remove(const QPixmapCache::Key &key)
 {
-    bool result = QCache<QPixmapCache::Key, QDetachedPixmap>::remove(key);
-    //We release the key after we removed it from the cache
-    releaseKey(key);
-    return result;
+    return QCache<QPixmapCache::Key, QPixmapCacheEntry>::remove(key);
 }
 
 void QPMCache::resizeKeyArray(int size)
@@ -409,10 +408,10 @@ void QPMCache::clear()
     freeKey = 0;
     keyArraySize = 0;
     //Mark all keys as invalid
-    QList<QPixmapCache::Key> keys = QCache<QPixmapCache::Key, QDetachedPixmap>::keys();
+    QList<QPixmapCache::Key> keys = QCache<QPixmapCache::Key, QPixmapCacheEntry>::keys();
     for (int i = 0; i < keys.size(); ++i)
         keys.at(i).d->isValid = false;
-    QCache<QPixmapCache::Key, QDetachedPixmap>::clear();
+    QCache<QPixmapCache::Key, QPixmapCacheEntry>::clear();
 }
 
 QPixmapCache::KeyData* QPMCache::getKeyData(QPixmapCache::Key *key)
@@ -424,6 +423,11 @@ QPixmapCache::KeyData* QPMCache::getKeyData(QPixmapCache::Key *key)
 
 Q_GLOBAL_STATIC(QPMCache, pm_cache)
 
+QPixmapCacheEntry::~QPixmapCacheEntry()
+{
+    pm_cache()->releaseKey(key);
+}
+
 /*!
     \obsolete
     \overload
diff --git a/src/gui/image/qpixmapcache_p.h b/src/gui/image/qpixmapcache_p.h
index 33f93bc..84e4a03 100644
--- a/src/gui/image/qpixmapcache_p.h
+++ b/src/gui/image/qpixmapcache_p.h
@@ -76,10 +76,10 @@ public:
 };
 
 // XXX: hw: is this a general concept we need to abstract?
-class QDetachedPixmap : public QPixmap
+class QPixmapCacheEntry : public QPixmap
 {
 public:
-    QDetachedPixmap(const QPixmap &pix) : QPixmap(pix)
+    QPixmapCacheEntry(const QPixmapCache::Key &key, const QPixmap &pix) : QPixmap(pix), key(key)
     {
         if (data && data->classId() == QPixmapData::RasterClass) {
             QRasterPixmapData *d = static_cast<QRasterPixmapData*>(data.data());
@@ -91,6 +91,8 @@ public:
             }
         }
     }
+    ~QPixmapCacheEntry();
+    QPixmapCache::Key key;
 };
 
 QT_END_NAMESPACE