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>

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

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

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;

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::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)
 {
-    QCocoaAutoReleasePool pool;
-
     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()];