Fix and test for multiple-access-to-same-URL

Not reproduced (except by test), but may fix a Bauhaus issue.

7 files changed, 260 insertions, 1 deletions

diff --git a/src/declarative/fx/qfxpixmapcache.cpp b/src/declarative/fx/qfxpixmapcache.cpp
index 7fc713b..0cc09cd 100644
--- a/src/declarative/fx/qfxpixmapcache.cpp
+++ b/src/declarative/fx/qfxpixmapcache.cpp
@@ -63,6 +63,7 @@ public:
         reply->deleteLater();
     }
     QNetworkReply *reply;
+    QPixmap pixmap; // ensure reference to pixmap to QPixmapCache does not discard
 
     int refCount;
     void addRef()
@@ -169,6 +170,9 @@ bool QFxPixmapCache::find(const QUrl& url, QPixmap *pixmap)
                     qWarning() << "Format error loading" << url;
                     *pixmap = QPixmap();
                     ok = false;
+                } else {
+                    if ((*iter)->refCount > 1)
+                        (*iter)->pixmap = *pixmap;
                 }
                 (*iter)->release();
             }
@@ -176,6 +180,15 @@ bool QFxPixmapCache::find(const QUrl& url, QPixmap *pixmap)
         QPixmapCache::insert(key, *pixmap);
     } else {
         ok = !pixmap->isNull();
+#ifndef QT_NO_LOCALFILE_OPTIMIZED_QML
+        if (url.scheme()!=QLatin1String("file"))
+#endif
+        // We may be the second finder. Still need to check for active replies.
+        {
+            QFxSharedNetworkReplyHash::Iterator iter = qfxActiveNetworkReplies.find(key);
+            if (iter != qfxActiveNetworkReplies.end())
+                (*iter)->release();
+        }
     }
     return ok;
 }
@@ -246,4 +259,13 @@ void QFxPixmapCache::cancelGet(const QUrl& url, QObject* obj)
     (*iter)->release();
 }
 
+/*!
+    This function is mainly for test verification. It returns the number of
+    requests that are still unfinished.
+*/
+int QFxPixmapCache::pendingRequests()
+{
+    return qfxActiveNetworkReplies.count();
+}
+
 QT_END_NAMESPACE
diff --git a/src/declarative/fx/qfxpixmapcache.h b/src/declarative/fx/qfxpixmapcache.h
index ca5d47b..fb5b88a 100644
--- a/src/declarative/fx/qfxpixmapcache.h
+++ b/src/declarative/fx/qfxpixmapcache.h
@@ -60,7 +60,9 @@ public:
     static QNetworkReply *get(QmlEngine *, const QUrl& url, QPixmap *pixmap);
     static void cancelGet(const QUrl& url, QObject* obj);
 
-    static bool find(const QUrl& url, QPixmap *pixmap); // url must have been passed to QFxPixmapCache::get, and finished. Or must be a local file.
+    static bool find(const QUrl& url, QPixmap *pixmap); // url must have been passed to QFxPixmapCache::get, and any returned reply finished.
+
+    static int pendingRequests(); // mainly for test verification
 };
 
 QT_END_NAMESPACE
diff --git a/tests/auto/declarative/qfxpixmapcache/data/exists.png b/tests/auto/declarative/qfxpixmapcache/data/exists.png
new file mode 100644
index 0000000..399bd0b
--- /dev/null
+++ b/tests/auto/declarative/qfxpixmapcache/data/exists.png
diff --git a/tests/auto/declarative/qfxpixmapcache/data/exists1.png b/tests/auto/declarative/qfxpixmapcache/data/exists1.png
new file mode 100644
index 0000000..399bd0b
--- /dev/null
+++ b/tests/auto/declarative/qfxpixmapcache/data/exists1.png
diff --git a/tests/auto/declarative/qfxpixmapcache/data/exists2.png b/tests/auto/declarative/qfxpixmapcache/data/exists2.png
new file mode 100644
index 0000000..399bd0b
--- /dev/null
+++ b/tests/auto/declarative/qfxpixmapcache/data/exists2.png
diff --git a/tests/auto/declarative/qfxpixmapcache/qfxpixmapcache.pro b/tests/auto/declarative/qfxpixmapcache/qfxpixmapcache.pro
new file mode 100644
index 0000000..e9b0417
--- /dev/null
+++ b/tests/auto/declarative/qfxpixmapcache/qfxpixmapcache.pro
@@ -0,0 +1,9 @@
+load(qttest_p4)
+contains(QT_CONFIG,declarative): QT += declarative
+QT += network
+SOURCES += tst_qfxpixmapcache.cpp
+HEADERS = 
+macx:CONFIG -= app_bundle
+
+# QMAKE_CXXFLAGS = -fprofile-arcs -ftest-coverage
+# LIBS += -lgcov
diff --git a/tests/auto/declarative/qfxpixmapcache/tst_qfxpixmapcache.cpp b/tests/auto/declarative/qfxpixmapcache/tst_qfxpixmapcache.cpp
new file mode 100644
index 0000000..4d3ad55
--- /dev/null
+++ b/tests/auto/declarative/qfxpixmapcache/tst_qfxpixmapcache.cpp
@@ -0,0 +1,226 @@
+#include <qtest.h>
+#include <QtTest/QtTest>
+#include <QtDeclarative/qfxpixmapcache.h>
+#include <QtDeclarative/qmlengine.h>
+#include <QNetworkReply>
+
+// These don't let normal people run tests!
+//#include "../network-settings.h"
+
+class tst_qfxpixmapcache : public QObject
+{
+    Q_OBJECT
+public:
+    tst_qfxpixmapcache() :
+        thisfile("file://" __FILE__)
+    {
+    }
+
+private slots:
+    void single();
+    void single_data();
+    void parallel();
+    void parallel_data();
+
+private:
+    QmlEngine engine;
+    QUrl thisfile;
+};
+
+
+static int slotters=0;
+
+class Slotter : public QObject
+{
+    Q_OBJECT
+public:
+    Slotter()
+    {
+        gotslot = false;
+        slotters++;
+    }
+    bool gotslot;
+
+public slots:
+    void got()
+    {
+        gotslot = true;
+        --slotters;
+        if (slotters==0)
+            QTestEventLoop::instance().exitLoop();
+    }
+};
+
+#ifndef QT_NO_LOCALFILE_OPTIMIZED_QML
+static const bool localfile_optimized = true;
+#else
+static const bool localfile_optimized = false;
+#endif
+
+void tst_qfxpixmapcache::single_data()
+{
+    // Note, since QFxPixmapCache is shared, tests affect each other!
+    // so use different files fore all test functions.
+
+    QTest::addColumn<QUrl>("target");
+    QTest::addColumn<bool>("incache");
+    QTest::addColumn<bool>("exists");
+
+    // File URLs are optimized
+    QTest::newRow("local") << thisfile.resolved(QUrl("data/exists.png")) << localfile_optimized << true;
+    QTest::newRow("local") << thisfile.resolved(QUrl("data/notexists.png")) << localfile_optimized << false;
+    QTest::newRow("remote") << QUrl("http://qt.nokia.com/logo.png") << false << true;
+    QTest::newRow("remote") << QUrl("http://qt.nokia.com/thereisnologo.png") << false << false;
+}
+
+void tst_qfxpixmapcache::single()
+{
+    QFETCH(QUrl, target);
+    QFETCH(bool, incache);
+    QFETCH(bool, exists);
+
+    QPixmap pixmap;
+    QVERIFY(pixmap.width() <= 0); // Check Qt assumption
+    QNetworkReply *reply= QFxPixmapCache::get(&engine, target, &pixmap);
+
+    if (incache) {
+        QVERIFY(!reply);
+        if (exists)
+            QVERIFY(pixmap.width() > 0);
+        else
+            QVERIFY(pixmap.width() <= 0);
+    } else {
+        QVERIFY(reply);
+        QVERIFY(pixmap.width() <= 0);
+
+        Slotter getter;
+        connect(reply, SIGNAL(finished()), &getter, SLOT(got()));
+        QTestEventLoop::instance().enterLoop(10);
+        QVERIFY(!QTestEventLoop::instance().timeout());
+        QVERIFY(getter.gotslot);
+        if (exists) {
+            QVERIFY(QFxPixmapCache::find(target, &pixmap));
+            QVERIFY(pixmap.width() > 0);
+        } else {
+            QVERIFY(!QFxPixmapCache::find(target, &pixmap));
+            QVERIFY(pixmap.width() <= 0);
+        }
+    }
+
+    QCOMPARE(QFxPixmapCache::pendingRequests(), 0);
+}
+
+void tst_qfxpixmapcache::parallel_data()
+{
+    // Note, since QFxPixmapCache is shared, tests affect each other!
+    // so use different files fore all test functions.
+
+    QTest::addColumn<QUrl>("target1");
+    QTest::addColumn<QUrl>("target2");
+    QTest::addColumn<int>("incache");
+    QTest::addColumn<int>("cancel"); // which one to cancel
+    QTest::addColumn<int>("requests");
+
+    QTest::newRow("local")
+            << thisfile.resolved(QUrl("data/exists1.png"))
+            << thisfile.resolved(QUrl("data/exists2.png"))
+            << (localfile_optimized ? 2 : 0)
+            << -1
+            << (localfile_optimized ? 0 : 2)
+            ;
+
+    QTest::newRow("remote")
+            << QUrl("http://qt.nokia.com/images/template/checkbox-on.png")
+            << QUrl("http://qt.nokia.com/images/products/qt-logo/image_tile")
+            << 0
+            << -1
+            << 2
+            ;
+
+    QTest::newRow("remoteagain")
+            << QUrl("http://qt.nokia.com/images/template/checkbox-on.png")
+            << QUrl("http://qt.nokia.com/images/products/qt-logo/image_tile")
+            << 2
+            << -1
+            << 0
+            ;
+
+    QTest::newRow("remotecopy")
+            << QUrl("http://qt.nokia.com/images/template/checkbox-off.png")
+            << QUrl("http://qt.nokia.com/images/template/checkbox-off.png")
+            << 0
+            << -1
+            << 1
+            ;
+
+    QTest::newRow("remotecopycancel")
+            << QUrl("http://qt.nokia.com/rounded_block_bg.png")
+            << QUrl("http://qt.nokia.com/rounded_block_bg.png")
+            << 0
+            << 0
+            << 1
+            ;
+}
+
+void tst_qfxpixmapcache::parallel()
+{
+    QFETCH(QUrl, target1);
+    QFETCH(QUrl, target2);
+    QFETCH(int, incache);
+    QFETCH(int, cancel);
+    QFETCH(int, requests);
+
+    QList<QUrl> targets;
+    targets << target1 << target2;
+
+    QList<QNetworkReply*> replies;
+    QList<Slotter*> getters;
+    for (int i=0; i<targets.count(); ++i) {
+        QUrl target = targets.at(i);
+        QPixmap pixmap;
+        QNetworkReply *reply = QFxPixmapCache::get(&engine, target, &pixmap);
+        replies.append(reply);
+        if (!reply) {
+            QVERIFY(pixmap.width() > 0);
+            getters.append(0);
+        } else {
+            QVERIFY(pixmap.width() <= 0);
+            getters.append(new Slotter);
+            connect(reply, SIGNAL(finished()), getters[i], SLOT(got()));
+        }
+    }
+
+    QCOMPARE(incache+slotters, targets.count());
+    QCOMPARE(QFxPixmapCache::pendingRequests(), requests);
+
+    if (cancel >= 0) {
+        QFxPixmapCache::cancelGet(targets.at(cancel), getters[cancel]);
+        slotters--;
+    }
+
+    if (slotters) {
+        QTestEventLoop::instance().enterLoop(10);
+        QVERIFY(!QTestEventLoop::instance().timeout());
+    }
+
+    for (int i=0; i<targets.count(); ++i) {
+        QNetworkReply *reply = replies[i];
+        if (reply) {
+            if (i == cancel) {
+                QVERIFY(!getters[i]->gotslot);
+            } else {
+                QVERIFY(getters[i]->gotslot);
+                QPixmap pixmap;
+                QVERIFY(QFxPixmapCache::find(targets[i], &pixmap));
+                QVERIFY(pixmap.width() > 0);
+            }
+            delete getters[i];
+        }
+    }
+
+    QCOMPARE(QFxPixmapCache::pendingRequests(), 0);
+}
+
+QTEST_MAIN(tst_qfxpixmapcache)
+
+#include "tst_qfxpixmapcache.moc"