1berry/qtbase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

tst_QComboBox: fix memleak in task_QTBUG_56693_itemFontFromModel()

Browse Source 
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.9 6.8 6.5 5.15
Change-Id: I2abf4a0bf54158254fd8d526de55ad486ca4e296
Reviewed-by: Axel Spoerl <axel.spoerl@qt.io>
This commit is contained in:
Marc Mutz 2025-03-31 17:39:23 +02:00
parent 27279e01d2
commit d884591391
2 changed files with 25 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/widgets/styles/qstyle_p.h
View File

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

27
tests/auto/widgets/widgets/qcombobox/tst_qcombobox.cpp
View File

@ -6,8 +6,10 @@
#include <QSortFilterProxyModel>
#include "qcombobox.h"
#include <private/qcombobox_p.h>
#include <private/qguiapplication_p.h>
#include <QtWidgets/private/qstyle_p.h>
#include <qpa/qplatformintegration.h>
#include <qpa/qplatformtheme.h>
@ -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();
}