From f261357c1a919ebc1d5dbc9fcf7dd58165a1a40f Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Mon, 31 Mar 2025 17:39:23 +0200 Subject: [PATCH] tst_QComboBox: fix memleak in task_QTBUG_56693_itemFontFromModel() QProxyStyle's ctor arguments are not QObject parents, but the proxied style, which becomes the proxy's QObject ... get this ... _child_. What a broken concept... It means that this test function took ownership of QApplication::style(), and the only way it wasn't going to wreak havoc on the rest of the test functions was to leak it after use. To fix, create the style on the stack, so it gets automatically freed by the compiler on all exists from the function. As a drive-by, use {} instead of () to call the ctor (not to prevent C++' Most Vexing Parse, just because it's easier on the eye). The meat of the change, however, is in making the helper proxy-style defend against QProxyStyle's grabbing of the baseStyle() as its child, and managing baseStyle()->d->proxyStyle so we properly dissociate from qApp->style() after we're done. Needed to use ctor delegation to extract original parent and proxy model from the incoming style before QProxyStyle, our first subobject, gets first picking on the style, destroying the very state we're trying to preserve. This should be become some QTest::LocalProxyStyle, if we find more users of this pattern. Amends 5cff7d2b679d48a247b4630cb9e3d5b04fab0b55. Pick-to: 6.8 6.5 5.15 Change-Id: I2abf4a0bf54158254fd8d526de55ad486ca4e296 Reviewed-by: Axel Spoerl (cherry picked from commit d88459139119b898e81c1a104e6fd3e28c8f01cf) Reviewed-by: Qt Cherry-pick Bot --- src/widgets/styles/qstyle_p.h | 3 +++ .../widgets/qcombobox/tst_qcombobox.cpp | 27 +++++++++++++++---- 2 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/widgets/styles/qstyle_p.h b/src/widgets/styles/qstyle_p.h index 2df2757c4e7..95ebe754952 100644 --- a/src/widgets/styles/qstyle_p.h +++ b/src/widgets/styles/qstyle_p.h @@ -28,6 +28,9 @@ class QStylePrivate: public QObjectPrivate { Q_DECLARE_PUBLIC(QStyle) public: + static QStylePrivate *get(QStyle *s) { return s ? s->d_func() : nullptr; } + static const QStylePrivate *get(const QStyle *s) { return s ? s->d_func() : nullptr; } + static bool useFullScreenForPopup(); QStyle *proxyStyle; diff --git a/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp b/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp index 91c0d10b7cf..e77fd804f21 100644 --- a/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp +++ b/tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp @@ -6,8 +6,10 @@ #include #include "qcombobox.h" + #include #include +#include #include #include @@ -3401,11 +3403,25 @@ public: class QTBUG_56693_ProxyStyle : public QProxyStyle { + QStyle *oldProxyStyle; public: QTBUG_56693_ProxyStyle(QStyle *style) - : QProxyStyle(style), italicItemsNo(0) - { + : QTBUG_56693_ProxyStyle(style, style->parent(), + QStylePrivate::get(style)->proxyStyle) {} +private: + QTBUG_56693_ProxyStyle(QStyle *style, QObject *styleParent, QStyle *proxy) + : QProxyStyle(style), oldProxyStyle(proxy), italicItemsNo(0) + { + // Undo the reparenting of QProxyStyle ctor again: + // We should not take ownership of the qApp->style()! + style->setParent(styleParent); + } +public: + ~QTBUG_56693_ProxyStyle() + { + // private in QStyle: baseStyle()->setProxy(nullptr); + QStylePrivate::get(baseStyle())->proxyStyle = oldProxyStyle; } void drawControl(ControlElement element, const QStyleOption *opt, QPainter *p, const QWidget *w = nullptr) const override @@ -3418,6 +3434,7 @@ public: baseStyle()->drawControl(element, opt, p, w); } +public: mutable int italicItemsNo; }; @@ -3430,8 +3447,8 @@ void tst_QComboBox::task_QTBUG_56693_itemFontFromModel() QTBUG_56693_Model model; box.setModel(&model); - QTBUG_56693_ProxyStyle *proxyStyle = new QTBUG_56693_ProxyStyle(box.style()); - box.setStyle(proxyStyle); + QTBUG_56693_ProxyStyle proxyStyle{box.style()}; // does _not_ take ownership of box.style()! + box.setStyle(&proxyStyle); box.setFont(QApplication::font()); for (int i = 0; i < 10; i++) @@ -3444,7 +3461,7 @@ void tst_QComboBox::task_QTBUG_56693_itemFontFromModel() QVERIFY(container); QVERIFY(QTest::qWaitForWindowExposed(container)); - QCOMPARE(proxyStyle->italicItemsNo, 5); + QCOMPARE(proxyStyle.italicItemsNo, 5); box.hidePopup(); }