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 fec4ac12e7)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit a9b0aec18c)
This commit is contained in:
Vladimir Belyavsky 2024-01-05 20:03:46 +03:00
parent 77a448c87f
commit 4b4af2a02c
2 changed files with 44 additions and 1 deletions

View File

@ -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

View File

@ -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"