Menu: Provide a safe clear() function

The menu items are generally owned by QML. Therefore we cannot delete
them on clear(). The containers, however, are owned by the menu, and we
have to delete them also when clearing the QQmlListProperty for a proxy
menu. The QQmlListProperty, when clearing the items for a non-proxy
menu, will still delete them, as that seems to be the way QML expresses
its desire to get rid of them.

Fixes: QTBUG-64464
Change-Id: Ied0b86496b289c2e2a83f3d6fd53d7045929beb0
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

2 files changed, 55 insertions, 22 deletions

diff --git a/src/controls/qquickmenu.cpp b/src/controls/qquickmenu.cpp
index 3c6d91ef..bfa0f673 100644
--- a/src/controls/qquickmenu.cpp
+++ b/src/controls/qquickmenu.cpp
@@ -789,33 +789,41 @@ void QQuickMenu1::insertItem(int index, QQuickMenuBase1 *menuItem)
 
 void QQuickMenu1::removeItem(QQuickMenuBase1 *menuItem)
 {
-    if (!menuItem)
-        return;
-    menuItem->setParentMenu(0);
-
-    QQuickMenuItemContainer1 *container = menuItem->parent() != this ? m_containers[menuItem->parent()] : 0;
-    if (container)
-        container->removeItem(menuItem);
-    else
-        m_menuItems.removeOne(menuItem);
-
-    --m_itemsCount;
-    emit itemsChanged();
+    // Removes the item, but if it's a container, the container is kept
+    if (menuItem) {
+        unparentItem(menuItem);
+        emit itemsChanged();
+    }
 }
 
 void QQuickMenu1::clear()
 {
-    m_containers.clear();
-    m_containersCount = 0;
+    if (m_itemsCount > 0) {
+        while (m_itemsCount > 0)
+            unparentItem(menuItemAtIndex(0));
 
-    // QTBUG-48927: a proxy menu (ApplicationWindowStyle.qml) must not
-    // delete its items, because they are owned by the menubar
-    if (m_proxy)
+        // We can delete the containers now, as there cannot be any further items in them.
+        qDeleteAll(m_containers);
+        m_containers.clear();
+        m_containersCount = 0;
+
+        // The containers are also kept in m_menuItems, so we have to clear explicitly.
         m_menuItems.clear();
 
-    while (!m_menuItems.empty())
-        delete m_menuItems.takeFirst();
-    m_itemsCount = 0;
+        emit itemsChanged();
+    }
+}
+
+void QQuickMenu1::unparentItem(QQuickMenuBase1 *menuItem)
+{
+    menuItem->setParentMenu(nullptr);
+    QQuickMenuItemContainer1 *container = (menuItem->parent() != this)
+            ? m_containers[menuItem->parent()] : nullptr;
+    if (container)
+        container->removeItem(menuItem);
+    else
+        m_menuItems.removeOne(menuItem);
+    --m_itemsCount;
 }
 
 void QQuickMenu1::setupMenuItem(QQuickMenuBase1 *item, int platformIndex)
@@ -871,8 +879,31 @@ QObject *QQuickMenu1::at_menuItems(QQuickMenuItems *list, int index)
 
 void QQuickMenu1::clear_menuItems(QQuickMenuItems *list)
 {
-    if (QQuickMenu1 *menu = qobject_cast<QQuickMenu1 *>(list->object))
-        menu->clear();
+    if (QQuickMenu1 *menu = qobject_cast<QQuickMenu1 *>(list->object)) {
+        // There may be stray containers that don't appear in m_menuItems. This is because we may
+        // remove a container with removeItem(), which will only remove it from m_menuItems.
+        // Therefore, make sure that all containers are removed from m_menuItems first.
+        for (QQuickMenuItemContainer1 *container : menu->m_containers)
+            menu->m_menuItems.removeOne(container);
+
+        // Delete or unparent the items first. They may have references to the containers.
+        // QTBUG-48927: a proxy menu (ApplicationWindowStyle.qml) must not
+        // delete its items, because they are owned by the menubar
+        // We still do own the containers, though. We create them on append_menuItems, after all.
+        while (!menu->m_menuItems.empty()) {
+            if (menu->m_proxy)
+                menu->unparentItem(menu->m_menuItems[0]);
+            else
+                delete menu->m_menuItems.takeFirst();
+        }
+        menu->m_menuItems.clear();
+
+        qDeleteAll(menu->m_containers);
+        menu->m_containers.clear();
+        menu->m_containersCount = 0;
+
+        menu->m_itemsCount = 0;
+    }
 }
 
 QT_END_NAMESPACE
diff --git a/src/controls/qquickmenu_p.h b/src/controls/qquickmenu_p.h
index 800981dd..0594f391 100644
--- a/src/controls/qquickmenu_p.h
+++ b/src/controls/qquickmenu_p.h
@@ -190,6 +190,8 @@ private:
     static int count_menuItems(QQuickMenuItems *list);
     static QObject *at_menuItems(QQuickMenuItems *list, int index);
     static void clear_menuItems(QQuickMenuItems *list);
+
+    void unparentItem(QQuickMenuBase1 *menuItem);
     void setupMenuItem(QQuickMenuBase1 *item, int platformIndex = -1);
 
     QPlatformMenu *m_platformMenu;