From 370e89f06465a4d61c7b72291115cd7b5a0d576a Mon Sep 17 00:00:00 2001
From: Gabriel de Dietrich <gabriel.dedietrich@digia.com>
Date: Wed, 24 Apr 2013 13:55:00 +0200
Subject: [PATCH] Cocoa: Reflect menu hierarchy in QCocoaMenu* objects
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

QCocoaMenu is child of either a QCocoaMenuBar, a QCocoaMenuItem as a
submenu, or nothing as a standalone menu. QCocoaMenuItem is child of
its containing QCocoaMenu.

The parent is set during insertion and cleared during removal.

QMenu needs to be updated to avoid double deletion and leaking its
own platform menu.

Change-Id: Iadf60d8062d7466fa616f84f3761fe322fc9aa2e
Reviewed-by: Morten Johan Sørvig <morten.sorvig@digia.com>
---
 src/plugins/platforms/cocoa/qcocoamenu.mm     | 5 +++++
 src/plugins/platforms/cocoa/qcocoamenubar.mm  | 3 +++
 src/plugins/platforms/cocoa/qcocoamenuitem.mm | 3 +++
 src/widgets/widgets/qmenu.cpp                 | 8 ++++----
 src/widgets/widgets/qmenu_p.h                 | 5 +++--
 5 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index 2b4c2eaaab..df338a1109 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -154,6 +154,7 @@ void QCocoaMenu::insertMenuItem(QPlatformMenuItem *menuItem, QPlatformMenuItem *
     QCocoaMenuItem *cocoaItem = static_cast<QCocoaMenuItem *>(menuItem);
     QCocoaMenuItem *beforeItem = static_cast<QCocoaMenuItem *>(before);
 
+    menuItem->setParent(this);
     cocoaItem->sync();
     if (beforeItem) {
         int index = m_menuItems.indexOf(beforeItem);
@@ -209,6 +210,10 @@ void QCocoaMenu::removeMenuItem(QPlatformMenuItem *menuItem)
         qWarning() << Q_FUNC_INFO << "Menu does not contain the item to be removed";
         return;
     }
+
+    if (menuItem->parent() == this)
+        menuItem->setParent(0);
+
     m_menuItems.removeOne(cocoaItem);
     if (!cocoaItem->isMerged()) {
         if (m_nativeMenu != [cocoaItem->nsItem() menu]) {
diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.mm b/src/plugins/platforms/cocoa/qcocoamenubar.mm
index b880db16a2..b0a53244f4 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.mm
@@ -109,6 +109,7 @@ void QCocoaMenuBar::insertMenu(QPlatformMenu *platformMenu, QPlatformMenu *befor
         [m_nativeMenu addItem: menu->nsMenuItem()];
     }
 
+    platformMenu->setParent(this);
     [m_nativeMenu setSubmenu: menu->nsMenu() forItem: menu->nsMenuItem()];
 }
 
@@ -123,6 +124,8 @@ void QCocoaMenuBar::removeMenu(QPlatformMenu *platformMenu)
     }
     m_menus.removeOne(menu);
 
+    if (platformMenu->parent() == this)
+        platformMenu->setParent(0);
     NSUInteger realIndex = [m_nativeMenu indexOfItem:menu->nsMenuItem()];
     [m_nativeMenu removeItemAtIndex: realIndex];
 }
diff --git a/src/plugins/platforms/cocoa/qcocoamenuitem.mm b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
index d6f0d06be7..51d9a83345 100644
--- a/src/plugins/platforms/cocoa/qcocoamenuitem.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenuitem.mm
@@ -124,10 +124,13 @@ void QCocoaMenuItem::setMenu(QPlatformMenu *menu)
 {
     if (menu == m_menu)
         return;
+    if (m_menu && m_menu->parent() == this)
+        m_menu->setParent(0);
 
     QCocoaAutoReleasePool pool;
     m_menu = static_cast<QCocoaMenu *>(menu);
     if (m_menu) {
+        m_menu->setParent(this);
     } else {
         // we previously had a menu, but no longer
         // clear out our item so the nexy sync() call builds a new one
diff --git a/src/widgets/widgets/qmenu.cpp b/src/widgets/widgets/qmenu.cpp
index aaedd7ffee..fde46c9729 100644
--- a/src/widgets/widgets/qmenu.cpp
+++ b/src/widgets/widgets/qmenu.cpp
@@ -154,7 +154,7 @@ void QMenuPrivate::init()
     }
 
     platformMenu = QGuiApplicationPrivate::platformTheme()->createPlatformMenu();
-    if (platformMenu) {
+    if (!platformMenu.isNull()) {
         QObject::connect(platformMenu, SIGNAL(aboutToShow()), q, SIGNAL(aboutToShow()));
         QObject::connect(platformMenu, SIGNAL(aboutToHide()), q, SIGNAL(aboutToHide()));
     }
@@ -2411,7 +2411,7 @@ void QMenu::changeEvent(QEvent *e)
         if (d->tornPopup) // torn-off menu
             d->tornPopup->setEnabled(isEnabled());
         d->menuAction->setEnabled(isEnabled());
-        if (d->platformMenu)
+        if (!d->platformMenu.isNull())
             d->platformMenu->setEnabled(isEnabled());
     }
     QWidget::changeEvent(e);
@@ -2992,7 +2992,7 @@ void QMenu::actionEvent(QActionEvent *e)
         d->widgetItems.remove(e->action());
     }
 
-    if (d->platformMenu) {
+    if (!d->platformMenu.isNull()) {
         if (e->type() == QEvent::ActionAdded) {
             QPlatformMenuItem *menuItem =
                 QGuiApplicationPrivate::platformTheme()->createPlatformMenuItem();
@@ -3201,7 +3201,7 @@ void QMenu::setSeparatorsCollapsible(bool collapse)
         d->updateActionRects();
         update();
     }
-    if (d->platformMenu)
+    if (!d->platformMenu.isNull())
         d->platformMenu->syncSeparatorsCollapsible(collapse);
 }
 
diff --git a/src/widgets/widgets/qmenu_p.h b/src/widgets/widgets/qmenu_p.h
index 6cc88c56e2..15f3c92127 100644
--- a/src/widgets/widgets/qmenu_p.h
+++ b/src/widgets/widgets/qmenu_p.h
@@ -101,7 +101,8 @@ public:
     ~QMenuPrivate()
     {
         delete scroll;
-        delete platformMenu;
+        if (!platformMenu.isNull() && !platformMenu->parent())
+            delete platformMenu.data();
 #if defined(Q_OS_WINCE) && !defined(QT_NO_MENUBAR)
         delete wce_menu;
 #endif
@@ -228,7 +229,7 @@ public:
     //menu fading/scrolling effects
     bool doChildEffects;
 
-    QPlatformMenu *platformMenu;
+    QPointer<QPlatformMenu> platformMenu;
 
     QPointer<QAction> actionAboutToTrigger;