Cocoa:Fixed crash when sharing QMenu between two QMenuBar instances

Don't insert a NSMenuItem into a NSMenu if this one already belongs to another NSMenu, this is forbidden
in the Cocoa framework and raises an Exception. The solution consists in tagging the menu as sharable
and moving it from one menubar to another when the window gets focus.

Task-number: QTBUG-31342

Change-Id: Ic3bfadd4704f363ac26122ae15547543a0f6d44d
Reviewed-by: Gabriel de Dietrich <gabriel.dedietrich@digia.com>

4 files changed, 66 insertions, 22 deletions

diff --git a/src/plugins/platforms/cocoa/qcocoamenu.h b/src/plugins/platforms/cocoa/qcocoamenu.h
index 9100b9b15f..7224ee2ff8 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.h
+++ b/src/plugins/platforms/cocoa/qcocoamenu.h
@@ -49,6 +49,8 @@
 
 QT_BEGIN_NAMESPACE
 
+class QCocoaMenuBar;
+
 class QCocoaMenu : public QPlatformMenu
 {
 public:
@@ -87,6 +89,8 @@ public:
 
     QList<QCocoaMenuItem *> items() const;
     QList<QCocoaMenuItem *> merged() const;
+    void setMenuBar(QCocoaMenuBar *menuBar);
+    QCocoaMenuBar *menuBar() const;
 private:
     QCocoaMenuItem *itemOrNull(int index) const;
     void insertNative(QCocoaMenuItem *item, QCocoaMenuItem *beforeItem);
@@ -97,6 +101,7 @@ private:
     NSObject *m_delegate;
     bool m_enabled;
     quintptr m_tag;
+    QCocoaMenuBar *m_menuBar;
 };
 
 QT_END_NAMESPACE
diff --git a/src/plugins/platforms/cocoa/qcocoamenu.mm b/src/plugins/platforms/cocoa/qcocoamenu.mm
index 25ece7349c..d4cf83a380 100644
--- a/src/plugins/platforms/cocoa/qcocoamenu.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenu.mm
@@ -215,7 +215,8 @@ QT_BEGIN_NAMESPACE
 
 QCocoaMenu::QCocoaMenu() :
     m_enabled(true),
-    m_tag(0)
+    m_tag(0),
+    m_menuBar(0)
 {
     m_delegate = [[QT_MANGLE_NAMESPACE(QCocoaMenuDelegate) alloc] initWithMenu:this];
     m_nativeItem = [[NSMenuItem alloc] initWithTitle:@"" action:nil keyEquivalent:@""];
@@ -534,4 +535,14 @@ void QCocoaMenu::syncModalState(bool modal)
     }
 }
 
+void QCocoaMenu::setMenuBar(QCocoaMenuBar *menuBar)
+{
+    m_menuBar = menuBar;
+}
+
+QCocoaMenuBar *QCocoaMenu::menuBar() const
+{
+    return m_menuBar;
+}
+
 QT_END_NAMESPACE
diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.h b/src/plugins/platforms/cocoa/qcocoamenubar.h
index 8086676cc5..7a1bda74a4 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.h
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.h
@@ -75,6 +75,8 @@ private:
     static QCocoaMenuBar *findGlobalMenubar();
 
     bool shouldDisable(QCocoaWindow *active) const;
+    void insertNativeMenu(QCocoaMenu *menu, QCocoaMenu *beforeMenu);
+    void removeNativeMenu(QCocoaMenu *menu);
 
     QList<QCocoaMenu*> m_menus;
     NSMenu *m_nativeMenu;
diff --git a/src/plugins/platforms/cocoa/qcocoamenubar.mm b/src/plugins/platforms/cocoa/qcocoamenubar.mm
index e280cf4581..73331db40d 100644
--- a/src/plugins/platforms/cocoa/qcocoamenubar.mm
+++ b/src/plugins/platforms/cocoa/qcocoamenubar.mm
@@ -83,9 +83,24 @@ QCocoaMenuBar::~QCocoaMenuBar()
     }
 }
 
-void QCocoaMenuBar::insertMenu(QPlatformMenu *platformMenu, QPlatformMenu *before)
+void QCocoaMenuBar::insertNativeMenu(QCocoaMenu *menu, QCocoaMenu *beforeMenu)
 {
     QCocoaAutoReleasePool pool;
+
+    if (beforeMenu) {
+        NSUInteger nativeIndex = [m_nativeMenu indexOfItem:beforeMenu->nsMenuItem()];
+        [m_nativeMenu insertItem: menu->nsMenuItem() atIndex: nativeIndex];
+    } else {
+        [m_nativeMenu addItem: menu->nsMenuItem()];
+    }
+
+    menu->setMenuBar(this);
+    syncMenu(static_cast<QPlatformMenu *>(menu));
+    [m_nativeMenu setSubmenu: menu->nsMenu() forItem: menu->nsMenuItem()];
+}
+
+void QCocoaMenuBar::insertMenu(QPlatformMenu *platformMenu, QPlatformMenu *before)
+{
     QCocoaMenu *menu = static_cast<QCocoaMenu *>(platformMenu);
     QCocoaMenu *beforeMenu = static_cast<QCocoaMenu *>(before);
 #ifdef QT_COCOA_ENABLE_MENU_DEBUG
@@ -96,39 +111,36 @@ void QCocoaMenuBar::insertMenu(QPlatformMenu *platformMenu, QPlatformMenu *befor
         qWarning() << Q_FUNC_INFO << "This menu already belongs to the menubar, remove it first";
         return;
     }
-    if (beforeMenu) {
-        if (!m_menus.contains(beforeMenu)) {
-            qWarning() << Q_FUNC_INFO << "The before menu does not belong to the menubar";
-            return;
-        }
-        m_menus.insert(m_menus.indexOf(beforeMenu), menu);
-        NSUInteger nativeIndex = [m_nativeMenu indexOfItem:beforeMenu->nsMenuItem()];
-        [m_nativeMenu insertItem: menu->nsMenuItem() atIndex: nativeIndex];
-    } else {
-        m_menus.append(menu);
-        [m_nativeMenu addItem: menu->nsMenuItem()];
+
+    if (beforeMenu && !m_menus.contains(beforeMenu)) {
+        qWarning() << Q_FUNC_INFO << "The before menu does not belong to the menubar";
+        return;
     }
 
-    platformMenu->setParent(this);
-    syncMenu(platformMenu);
-    [m_nativeMenu setSubmenu: menu->nsMenu() forItem: menu->nsMenuItem()];
+    m_menus.insert(beforeMenu ? m_menus.indexOf(beforeMenu) : m_menus.size(), menu);
+    if (!menu->menuBar())
+        insertNativeMenu(menu, beforeMenu);
 }
 
-void QCocoaMenuBar::removeMenu(QPlatformMenu *platformMenu)
+void QCocoaMenuBar::removeNativeMenu(QCocoaMenu *menu)
 {
     QCocoaAutoReleasePool pool;
 
+    if (menu->menuBar() == this)
+        menu->setMenuBar(0);
+    NSUInteger realIndex = [m_nativeMenu indexOfItem:menu->nsMenuItem()];
+    [m_nativeMenu removeItemAtIndex: realIndex];
+}
+
+void QCocoaMenuBar::removeMenu(QPlatformMenu *platformMenu)
+{
     QCocoaMenu *menu = static_cast<QCocoaMenu *>(platformMenu);
     if (!m_menus.contains(menu)) {
         qWarning() << Q_FUNC_INFO << "Trying to remove a menu that does not belong to the menubar";
         return;
     }
     m_menus.removeOne(menu);
-
-    if (platformMenu->parent() == this)
-        platformMenu->setParent(0);
-    NSUInteger realIndex = [m_nativeMenu indexOfItem:menu->nsMenuItem()];
-    [m_nativeMenu removeItemAtIndex: realIndex];
+    removeNativeMenu(menu);
 }
 
 void QCocoaMenuBar::syncMenu(QPlatformMenu *menu)
@@ -207,6 +219,20 @@ void QCocoaMenuBar::updateMenuBarImmediately()
         m->syncModalState(disableForModal);
     }
 
+    // reparent shared menu items if necessary.
+    // We browse the list in reverse order to be sure that the next items are redrawn before the current ones,
+    // in this way we are sure that "beforeMenu" (see below) is part of the native menu before "m" is redraw
+    for (int i = mb->m_menus.size() - 1; i >= 0; i--) {
+        QCocoaMenu *m = mb->m_menus.at(i);
+        QCocoaMenuBar *menuBar = m->menuBar();
+        if (menuBar != mb) {
+            QCocoaMenu *beforeMenu  = i < (mb->m_menus.size() - 1) ? mb->m_menus.at(i + 1) : 0;
+            if (menuBar)
+                menuBar->removeNativeMenu(m);
+            mb->insertNativeMenu(m, beforeMenu);
+        }
+    }
+
     QT_MANGLE_NAMESPACE(QCocoaMenuLoader) *loader = getMenuLoader();
     [loader ensureAppMenuInMenu:mb->nsMenu()];