From 4b4af2a02cd85b2e4acf5940b256ff84817edb56 Mon Sep 17 00:00:00 2001
From: Vladimir Belyavsky <belyavskyv@gmail.com>
Date: Fri, 5 Jan 2024 20:03:46 +0300
Subject: [PATCH] AnimatedImage: avoid loading the same web source in parallel

QQuickAnimatedImage::load() is an overloaded method that is called implicitly from QQuickImage/QQuickImageBase whenever an user changes basic properties such as fillMode, mipmap, sourceSize, etc.

In the case of a web source, this can cause a real problem with
loading failure. For example, when the user changes `fillMode`
immediately after setting the `source` property, like:

 anim.source = <some web image>
 anim.fillMode = Image.PreserveAspectFit

Currently the code above lead to the error "QML AnimatedImage: Error
Reading Animated Image File". This happens because QQAI::load()
initiates a new network request at the same time that one is already
in progress. And when the first reply finishes, we try to read data from the new reply that is not ready yet.

To fix this, we can simply ignore and do nothing on QQAI::load() if there is already active network request (i.e. d->reply is not
nullptr). This has no effect on the actual source change, since we explicitly remove and nullify the active reply in QQAI::setSource().

By this change we also fix potential memory leak, because the old reply
was previously not destroyed properly in QQuickAnimatedImage::load().

Fixes: QTBUG-120555
Pick-to: 6.5
Change-Id: I28f964b51c059855c04a4c80bdce127b3e9974a7
Reviewed-by: Shawn Rutledge <shawn.rutledge@qt.io>
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
(cherry picked from commit fec4ac12e7a60dbd83a2b9c8bb75ad0f88d13a6a)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit a9b0aec18ca5d22537d350fafd795ae8d30d4053)
---
 src/quick/items/qquickanimatedimage.cpp       | 14 ++++++++-
 .../tst_qquickanimatedimage.cpp               | 31 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/src/quick/items/qquickanimatedimage.cpp b/src/quick/items/qquickanimatedimage.cpp
index e779982222..6218b3791e 100644
--- a/src/quick/items/qquickanimatedimage.cpp
+++ b/src/quick/items/qquickanimatedimage.cpp
@@ -335,6 +335,9 @@ void QQuickAnimatedImage::load()
             movieRequestFinished();
         } else {
 #if QT_CONFIG(qml_network)
+            if (d->reply)
+                return;
+
             d->setStatus(Loading);
             d->setProgress(0);
             QNetworkRequest req(d->url);
@@ -368,7 +371,16 @@ void QQuickAnimatedImage::movieRequestFinished()
         }
 
         d->redirectCount=0;
-        d->setMovie(new QMovie(d->reply));
+
+        auto movie = new QMovie(d->reply);
+        // From this point, we no longer need to handle the reply.
+        // I.e. it will be used only as a data source for QMovie,
+        // so it should live as long as the movie lives.
+        d->reply->disconnect(this);
+        d->reply->setParent(movie);
+        d->reply = nullptr;
+
+        d->setMovie(movie);
     }
 #endif
 
diff --git a/tests/auto/quick/qquickanimatedimage/tst_qquickanimatedimage.cpp b/tests/auto/quick/qquickanimatedimage/tst_qquickanimatedimage.cpp
index cb81ec8a07..92dc810508 100644
--- a/tests/auto/quick/qquickanimatedimage/tst_qquickanimatedimage.cpp
+++ b/tests/auto/quick/qquickanimatedimage/tst_qquickanimatedimage.cpp
@@ -62,6 +62,7 @@ private slots:
     void noCaching();
     void sourceChangesOnFrameChanged();
     void currentFrame();
+    void qtbug_120555();
 };
 
 void tst_qquickanimatedimage::cleanup()
@@ -646,6 +647,36 @@ void tst_qquickanimatedimage::currentFrame()
     QCOMPARE(anim->property("frameChangeCount"), 2);
 }
 
+void tst_qquickanimatedimage::qtbug_120555()
+{
+    TestHTTPServer server;
+    QVERIFY2(server.listen(), qPrintable(server.errorString()));
+    server.serveDirectory(dataDirectory());
+
+    QQmlEngine engine;
+    QQmlComponent component(&engine);
+    component.setData("import QtQuick 2.0\nAnimatedImage {}", {});
+
+    QQuickAnimatedImage *anim = qobject_cast<QQuickAnimatedImage*>(component.create());
+    QVERIFY(anim);
+
+    anim->setSource(server.url("/stickman.gif"));
+    QTRY_COMPARE(anim->status(), QQuickImage::Loading);
+
+    anim->setFillMode(QQuickImage::PreserveAspectFit);
+    QCOMPARE(anim->fillMode(), QQuickImage::PreserveAspectFit);
+    anim->setMipmap(true);
+    QCOMPARE(anim->mipmap(), true);
+    anim->setCache(false);
+    QCOMPARE(anim->cache(), false);
+    anim->setSourceSize(QSize(200, 200));
+    QCOMPARE(anim->sourceSize(), QSize(200, 200));
+
+    QTRY_COMPARE(anim->status(), QQuickImage::Ready);
+
+    delete anim;
+}
+
 QTEST_MAIN(tst_qquickanimatedimage)
 
 #include "tst_qquickanimatedimage.moc"