qiurui
/
qtdeclarative
mirror of https://github.com/qt/qtdeclarative.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Reset sub-menu and content item during deletion in the quick menu 

The sub-menu item within the menu can have invalid reference when
its created and destroyed through loader.

The loader uses the incubator to create the items asynchronously
(when configured asynchronous), when it becomes active and places
them on the object stack. These items are cleared when the loader
becomes inactive. In a case where we have the parent menu at the
root, the loader places the parent menu at the bottom of the stack,
and the sub-menu or any other child item within the menu is placed
on top of it during its creation, and the deletion of these items
happens from the top, which leads to the paren menu being deleted
last. These menu items containing the sub-menu reference have not
been reset when the sub-menus are deleted in this way, thus it can
lead to a crash as the parent menu refers to an invalid item while
resetting its state. The same happens for the contentItem within
the menu.

This patch ensures that the content item and sub-menu within the
menu item are reset during deletion, which avoids the parent menu
referring an invalid item.

Fixes: QTBUG-137160
Task-number: QTBUG-139552
Pick-to: 6.9 6.8 6.5
Change-Id: Ia9688db90d6a8c8f4e4fa2aadb7e90cb3d8ea25b
Reviewed-by: Oliver Eftevaag <oliver.eftevaag@qt.io>
(cherry picked from commit 36216db956)
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
This commit is contained in:
Santhosh Kumar 2025-08-13 18:02:40 +02:00
parent 5031605471
commit e5f4aa242c
5 changed files with 147 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
src/quicktemplates/qquickmenu.cpp
View File

@ -884,11 +884,17 @@ void QQuickMenuPrivate::itemSiblingOrderChanged(QQuickItem *)
void QQuickMenuPrivate::itemDestroyed(QQuickItem *item)
{
if (item == contentItem) {
resetContentItem();
} else {
QQuickPopupPrivate::itemDestroyed(item);
if (contentModel) {
int index = contentModel->indexOf(item, nullptr);
if (index != -1)
removeItem(index, item);
}
}
}
void QQuickMenuPrivate::itemGeometryChanged(QQuickItem *item, QQuickGeometryChange, const QRectF &)
{
@ -1320,6 +1326,20 @@ void QQuickMenuPrivate::contentData_clear(QQmlListProperty<QObject> *prop)
QQuickMenuPrivate::get(q)->contentData.clear();
}
void QQuickMenuPrivate::resetContentItem()
{
if (contentItem) {
QQuickItemPrivate::get(contentItem)->removeItemChangeListener(this, QQuickItemPrivate::Children);
QQuickItemPrivate::get(contentItem)->removeItemChangeListener(this, QQuickItemPrivate::Destroyed);
QQuickItemPrivate::get(contentItem)->removeItemChangeListener(this, QQuickItemPrivate::Geometry);
const auto children = contentItem->childItems();
for (QQuickItem *child : std::as_const(children))
QQuickItemPrivate::get(child)->removeItemChangeListener(this, QQuickItemPrivate::SiblingOrder);
contentItem = nullptr;
}
}
QQuickMenu::QQuickMenu(QObject *parent)
: QQuickPopup(*(new QQuickMenuPrivate), parent)
{
@ -1336,6 +1356,15 @@ QQuickMenu::~QQuickMenu()
<< "item count:"
<< d->contentModel->count()
<< "native item count:" << d->nativeItems.count();
// It would be better to reset the sub-menu within the menu-item during its destruction
// as there can be a chance that the parent menu use invalid reference leading to
// application crash (as mentioned in the bug report QTBUG-137160)
if (auto *menuItem = qobject_cast<QQuickMenuItem *>(d->parentItem)) {
if (menuItem->subMenu() == this) {
auto *menuItemPriv = QQuickMenuItemPrivate::get(menuItem);
menuItemPriv->setSubMenu(nullptr);
}
}
// We have to remove items to ensure that our change listeners on the item
// are removed. It's too late to do this in ~QQuickMenuPrivate, as
// contentModel has already been destroyed before that is called.
@ -1344,14 +1373,7 @@ QQuickMenu::~QQuickMenu()
while (d->contentModel->count() > 0)
d->removeItem(0, d->itemAt(0), QQuickMenuPrivate::DestructionPolicy::Destroy);
if (d->contentItem) {
QQuickItemPrivate::get(d->contentItem)->removeItemChangeListener(d, QQuickItemPrivate::Children);
QQuickItemPrivate::get(d->contentItem)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
const auto children = d->contentItem->childItems();
for (QQuickItem *child : std::as_const(children))
QQuickItemPrivate::get(child)->removeItemChangeListener(d, QQuickItemPrivate::SiblingOrder);
}
d->resetContentItem();
}
/*!
@ -2133,10 +2155,12 @@ void QQuickMenu::contentItemChange(QQuickItem *newItem, QQuickItem *oldItem)
if (oldItem) {
QQuickItemPrivate::get(oldItem)->removeItemChangeListener(d, QQuickItemPrivate::Children);
QQuickItemPrivate::get(newItem)->removeItemChangeListener(d, QQuickItemPrivate::Destroyed);
QQuickItemPrivate::get(oldItem)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
}
if (newItem) {
QQuickItemPrivate::get(newItem)->addItemChangeListener(d, QQuickItemPrivate::Children);
QQuickItemPrivate::get(newItem)->addItemChangeListener(d, QQuickItemPrivate::Destroyed);
QQuickItemPrivate::get(newItem)->updateOrAddGeometryChangeListener(d, QQuickGeometryChange::Width);
}

6
src/quicktemplates/qquickmenu_p_p.h
View File

@ -129,6 +129,8 @@ public:
QPalette defaultPalette() const override;
virtual QQuickPopup::PopupType resolvedPopupType() const override;
void resetContentItem();
bool cascade = false;
bool triedToCreateNativeMenu = false;
int hoverTimer = 0;
@ -137,9 +139,9 @@ public:
qreal textPadding = 0;
QPointer<QQuickMenu> parentMenu;
QPointer<QQuickMenuItem> currentItem;
QQuickItem *contentItem = nullptr; // TODO: cleanup
QPointer<QQuickItem> contentItem;
QList<QObject *> contentData;
QQmlObjectModel *contentModel;
QPointer<QQmlObjectModel> contentModel;
QQmlComponent *delegate = nullptr;
QString title;
QQuickIcon icon;

2
src/quicktemplates/qquickmenuitem_p_p.h
View File

@ -47,7 +47,7 @@ public:
bool highlighted = false;
QQuickDeferredPointer<QQuickItem> arrow;
QQuickMenu *menu = nullptr;
QQuickMenu *subMenu = nullptr;
QPointer<QQuickMenu> subMenu;
qreal implicitTextPadding;
};

62
tests/auto/quickcontrols/qquickmenu/data/loadMenuAsynchronously.qml Normal file
View File

@ -0,0 +1,62 @@
import QtQuick
import QtQuick.Controls
Item {
width:100
height:100
property alias loader: menuLoader
property bool activateLoader: false
onActivateLoaderChanged: {
if (activateLoader) {
menuTimer.start()
menuLoader.active = true
}
}
Timer {
id: menuTimer
interval: 20
onTriggered: menuLoader.active = false
}
Loader {
id: menuLoader
active: false
asynchronous: true
sourceComponent: Menu {
contentItem: ListView {}
Menu {}
MenuSeparator {}
Action {}
Menu {
Action {}
Action {}
Menu {}
}
MenuItem {}
Item {
Menu {}
}
Menu {
MenuItem {}
Action {}
MenuItem {}
}
// Dummy menu items
Menu {}
Menu {}
MenuSeparator {}
Menu {}
Menu {}
MenuSeparator {}
Menu {}
Menu {}
MenuSeparator {}
Menu {}
Menu {}
}
}
}

44
tests/auto/quickcontrols/qquickmenu/tst_qquickmenu.cpp
View File

@ -19,8 +19,10 @@
#include <QtQuick/private/qquicklistview_p.h>
#include <QtQuick/private/qquickmousearea_p.h>
#include <QtQuick/private/qquickrectangle_p.h>
#include <QtQuick/private/qquickloader_p.h>
#include <QtQuickTest/quicktest.h>
#include <QtQuickTestUtils/private/qmlutils_p.h>
#include <QtQuickTestUtils/private/viewtestutils_p.h>
#include <QtQuickTestUtils/private/visualtestutils_p.h>
#include <QtQuickControlsTestUtils/private/controlstestutils_p.h>
#include <QtQuickControlsTestUtils/private/qtest_quickcontrols_p.h>
@ -126,6 +128,7 @@ private slots:
void mousePropagationWithinPopup();
void shortcutInNestedSubMenuAction();
void animationOnHeight();
void loadMenuAsynchronously();
private:
bool nativeMenuSupported = false;
@ -3537,6 +3540,47 @@ void tst_QQuickMenu::animationOnHeight()
QCOMPARE_GE(listView->height(), listView->contentHeight());
}
void tst_QQuickMenu::loadMenuAsynchronously()
{
QQuickView window(testFileUrl("loadMenuAsynchronously.qml"));
QCOMPARE(window.status(), QQuickView::Ready);
window.show();
window.requestActivate();
QVERIFY(QTest::qWaitForWindowActive(&window));
auto *rootItem = window.rootObject();
QVERIFY(rootItem);
auto *loader = rootItem->property("loader").value<QQuickLoader*>();
QVERIFY(loader);
QCOMPARE(loader->active(), false);
QCOMPARE(loader->asynchronous(), true);
auto activateLoader = rootItem->property("activateLoader").value<bool>();
QCOMPARE(activateLoader, false);
QVERIFY(rootItem->setProperty("activateLoader", true));
activateLoader = rootItem->property("activateLoader").value<bool>();
QCOMPARE(activateLoader, true);
QTRY_COMPARE(loader->active(), false);
rootItem->setProperty("activateLoader", false);
activateLoader = rootItem->property("activateLoader").value<bool>();
QCOMPARE(activateLoader, false);
QVERIFY(rootItem->setProperty("activateLoader", true));
activateLoader = rootItem->property("activateLoader").value<bool>();
QCOMPARE(activateLoader, true);
QTRY_COMPARE(loader->active(), false);
// The ignored warning message described below is triggered from the loader
// when progress creations not been reset. This needs to be analyzed.
// The bug report QTBUG-139552 raised to track the investigation.
auto *enginePriv = QQmlEnginePrivate::get(window.engine());
if (enginePriv->inProgressCreations)
QTest::ignoreMessage(QtWarningMsg, QRegularExpression("There are still \\\\\"(\\d+)\\\\\" items in the process of being created at engine destruction\\."));
}
QTEST_QUICKCONTROLS_MAIN(tst_QQuickMenu)
#include "tst_qquickmenu.moc"