From 5be58ac9271555eed3a2884d827f30c164d2277d Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sat, 22 Mar 2025 10:01:38 +0100 Subject: [PATCH] tst_QWidget: don't leak QStyle objects The result of QStyleFactory::create() must be deleted by the caller. Some test functions stored it in a QScopedPointer, while others didn't, each causing a leak of a QWindowsStyle object. DRY the test functions (except the one that checks that 'delete widget.style();' works) by introducing an owning pointer at the test class level, to be reused by test functions. This is not so much for caching the object across test functions (though this is a nice benefit, too), but in order to keep the owning pointer out of the test functions, and allow most functions to just do setStyle(deterministicStyle()). Found by ASAN. On my machine, this test is now ASAN-clean, except a) the usual cp-demangle.c leak and b) destroyedSignal(), which still leaks. As a drive-by, pass a QStringLiteral instead of QL1SV to QStyleFactory::create() and port one QVERIFY( != ) to QCOMPARE_NE(). No amends, it'd be too many. Not picking back far, because others don't care, and I expect lots of conflicts when backporting, due to many different amended commits. Pick-to: 6.9 Change-Id: Ie9e7faf21bc177e08af56c659dc58870ad3cf6ff Reviewed-by: Volker Hilsheimer --- .../widgets/kernel/qwidget/tst_qwidget.cpp | 35 ++++++++++--------- 1 file changed, 19 insertions(+), 16 deletions(-) diff --git a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp index 46600956b5e..614496d2144 100644 --- a/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp +++ b/tests/auto/widgets/kernel/qwidget/tst_qwidget.cpp @@ -483,6 +483,15 @@ private: const int m_fuzz; QPalette simplePalette(); + mutable std::unique_ptr m_style; + QStyle *deterministicStyle() const + { + if (!m_style) + m_style.reset(QStyleFactory::create(u"Windows"_s)); + [&] { QVERIFY(m_style); }(); + return m_style.get(); + } + private: enum class ScreenPosition { OffAbove, @@ -500,11 +509,11 @@ void tst_QWidget::getSetCheck() QWidget child1(&obj1); // QStyle * QWidget::style() // void QWidget::setStyle(QStyle *) - QScopedPointer var1(QStyleFactory::create(QLatin1String("Windows"))); - obj1.setStyle(var1.data()); - QCOMPARE(static_cast(var1.data()), obj1.style()); + auto *style = deterministicStyle(); + obj1.setStyle(style); + QCOMPARE(style, obj1.style()); obj1.setStyle(nullptr); - QVERIFY(var1.data() != obj1.style()); + QCOMPARE_NE(style, obj1.style()); QVERIFY(obj1.style() != nullptr); // style can never be 0 for a widget const QRegularExpression negativeNotPossible(u"^.*Negative sizes \\(.*\\) are not possible$"_s); @@ -645,7 +654,6 @@ void tst_QWidget::getSetCheck() obj1.setAutoFillBackground(true); QCOMPARE(true, obj1.autoFillBackground()); - var1.reset(); #if defined (Q_OS_WIN) obj1.setWindowFlags(Qt::FramelessWindowHint | Qt::WindowSystemMenuHint); const HWND handle = reinterpret_cast(obj1.winId()); // explicitly create window handle @@ -6644,8 +6652,7 @@ void tst_QWidget::moveChild() ColorWidget parent(nullptr, Qt::Window | Qt::WindowStaysOnTopHint); // prevent custom styles - const QScopedPointer style(QStyleFactory::create(QLatin1String("Windows"))); - parent.setStyle(style.data()); + parent.setStyle(deterministicStyle()); ColorWidget child(&parent, Qt::Widget, Qt::blue); parent.setGeometry(QRect(m_availableTopLeft + QPoint(50, 50), QSize(200, 200))); @@ -6701,8 +6708,7 @@ void tst_QWidget::showAndMoveChild() QWidget parent(nullptr, Qt::Window | Qt::WindowStaysOnTopHint); parent.setWindowTitle(QLatin1String(QTest::currentTestFunction())); // prevent custom styles - const QScopedPointer style(QStyleFactory::create(QLatin1String("Windows"))); - parent.setStyle(style.data()); + parent.setStyle(deterministicStyle()); QRect desktopDimensions = parent.screen()->availableGeometry(); desktopDimensions = desktopDimensions.adjusted(64, 64, -64, -64); @@ -7989,7 +7995,7 @@ void tst_QWidget::renderChildFillsBackground() window.setWindowTitle(QLatin1String(QTest::currentTestFunction())); window.resize(100, 100); // prevent custom styles - window.setStyle(QStyleFactory::create(QLatin1String("Windows"))); + window.setStyle(deterministicStyle()); window.show(); QVERIFY(QTest::qWaitForWindowExposed(&window)); QWidget child(&window); @@ -8016,7 +8022,7 @@ void tst_QWidget::renderTargetOffset() widget.setAutoFillBackground(true); widget.setPalette(Qt::red); // prevent custom styles - widget.setStyle(QStyleFactory::create(QLatin1String("Windows"))); + widget.setStyle(deterministicStyle()); widget.show(); QVERIFY(QTest::qWaitForWindowExposed(&widget)); QImage image(widget.size(), QImage::Format_RGB32); @@ -8266,9 +8272,7 @@ void tst_QWidget::renderWithPainter() { QWidget widget(nullptr, Qt::Tool); // prevent custom styles - - const QScopedPointer style(QStyleFactory::create(QLatin1String("Windows"))); - widget.setStyle(style.data()); + widget.setStyle(deterministicStyle()); widget.show(); widget.resize(70, 50); widget.setAutoFillBackground(true); @@ -8370,12 +8374,11 @@ void tst_QWidget::renderRTL() { QFont f; f.setStyleStrategy(QFont::NoAntialias); - const QScopedPointer style(QStyleFactory::create(QLatin1String("Windows"))); QMenu menu; menu.setMinimumWidth(200); menu.setFont(f); - menu.setStyle(style.data()); + menu.setStyle(deterministicStyle()); menu.addAction("I"); menu.show(); menu.setLayoutDirection(Qt::LeftToRight);