Prevent QPixmapCache potentially growing indefinitely.

QPixmapCache has until now refused to throw out shared pixmaps, i.e.
ones that still have shallow copies lying around. This leads to problems
when someone inserts two shallow copies using different keys, causing
the cache itself containing multiple shallow copies and thus forever
refusing to throw out those entries.

It's rather easy for this to accidentally happen in a user application
since QPixmap::load() or QPixmap(const QString &fileName, ...)
automatically cache the pixmap in the QPixmapCache, thus if the user
then calls QPixmapCache::insert() on the same pixmap or a shallow copy
it is locked in the QPixmapCache forever.

The only reason for not throwing out a pixmap that's shared would be to
prevent re-loading a pixmap from file when a user has a direct reference
to it in his application, but in that case the user is unlikely to
re-load the pixmap from file in any case. Therefore it seems the best
fix is to get rid of this limitation.

Task-number: QTBUG-21359
Reviewed-by: John Brooks
Reviewed-by: Olivier Goffart
(cherry picked from commit 7ab0bed3a56d46c386e65abc381264c57137cb43)

Change-Id: I46dce19227e19a25e0287cf7372177430da15a66
Reviewed-on: http://codereview.qt-project.org/4563
Reviewed-by: Paul Olav Tvete <paul.tvete@nokia.com>

diff --git a/src/corelib/tools/qcache.h b/src/corelib/tools/qcache.h
index c42a908..2928140 100644 (file)
--- a/src/corelib/tools/qcache.h
+++ b/src/corelib/tools/qcache.h
@@ -196,8 +196,7 @@ void QCache<Key,T>::trim(int m)
     while (n && total > m) {
         Node *u = n;
         n = n->p;
-        if (qIsDetached(*u->t))
-            unlink(*u);
+        unlink(*u);
     }
 }
 

diff --git a/tests/auto/collections/tst_collections.cpp b/tests/auto/collections/tst_collections.cpp
index e8ce83c..7c13ecf 100644 (file)
--- a/tests/auto/collections/tst_collections.cpp
+++ b/tests/auto/collections/tst_collections.cpp
@@ -2413,7 +2413,7 @@ void tst_Collections::cache()
                two = s;
        }
        QVERIFY(!cache.contains(3));
-       QVERIFY(cache.contains(2));
+       QVERIFY(!cache.contains(2));
     }
     {
        QCache<int, int> cache(100);

diff --git a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp
index e8f749d..25a4340 100644 (file)
--- a/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp
+++ b/tests/auto/gui/image/qpixmapcache/tst_qpixmapcache.cpp
@@ -72,6 +72,7 @@ private slots:
     void clear();
     void pixmapKey();
     void noLeak();
+    void strictCacheLimit();
 };
 
 static QPixmapCache::KeyData* getPrivate(QPixmapCache::Key &key)
@@ -517,5 +518,29 @@ void tst_QPixmapCache::noLeak()
     QCOMPARE(oldSize, newSize);
 }
 
+
+void tst_QPixmapCache::strictCacheLimit()
+{
+    const int limit = 1024; // 1024 KB
+
+    QPixmapCache::clear();
+    QPixmapCache::setCacheLimit(limit);
+
+    // insert 200 64x64 pixmaps
+    // 3200 KB for 32-bit depths
+    // 1600 KB for 16-bit depths
+    // not counting the duplicate entries
+    for (int i = 0; i < 200; ++i) {
+        QPixmap pixmap(64, 64);
+        pixmap.fill(Qt::transparent);
+
+        QString id = QString::number(i);
+        QPixmapCache::insert(id + "-a", pixmap);
+        QPixmapCache::insert(id + "-b", pixmap);
+    }
+
+    QVERIFY(QPixmapCache::totalUsed() <= limit);
+}
+
 QTEST_MAIN(tst_QPixmapCache)
 #include "tst_qpixmapcache.moc"