QComboBox: change showNativePopup to be non-blocking

QPlatformMenu::showPopup() is supposed to be non-blocking.
So ensure that we don't start cleaning up the menu items directly
after the call in showNativePopup, but wait until we need to reload
the items, or the combobox is destroyed.

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

2 files changed, 81 insertions, 51 deletions

diff --git a/src/widgets/widgets/qcombobox.cpp b/src/widgets/widgets/qcombobox.cpp
index ab4c7bfff6..17a036c5e4 100644
--- a/src/widgets/widgets/qcombobox.cpp
+++ b/src/widgets/widgets/qcombobox.cpp
@@ -93,12 +93,22 @@ QComboBoxPrivate::QComboBoxPrivate()
       hoverControl(QStyle::SC_None),
       autoCompletionCaseSensitivity(Qt::CaseInsensitive),
       indexBeforeChange(-1)
+#ifdef Q_OS_OSX
+      , m_platformMenu(0)
+#endif
 #ifndef QT_NO_COMPLETER
       , completer(0)
 #endif
 {
 }
 
+QComboBoxPrivate::~QComboBoxPrivate()
+{
+#ifdef Q_OS_OSX
+    cleanupNativePopup();
+#endif
+}
+
 QStyleOptionMenuItem QComboMenuDelegate::getStyleOption(const QStyleOptionViewItem &option,
                                                         const QModelIndex &index) const
 {
@@ -2409,6 +2419,19 @@ struct IndexSetter {
 };
 }
 
+void QComboBoxPrivate::cleanupNativePopup()
+{
+    if (!m_platformMenu)
+        return;
+
+    int count = int(m_platformMenu->tag());
+    for (int i = 0; i < count; ++i)
+        m_platformMenu->menuItemAt(i)->deleteLater();
+
+    delete m_platformMenu;
+    m_platformMenu = 0;
+}
+
 /*!
  * \internal
  *
@@ -2419,60 +2442,62 @@ bool QComboBoxPrivate::showNativePopup()
 {
     Q_Q(QComboBox);
 
-    QPlatformTheme *theme = QGuiApplicationPrivate::instance()->platformTheme();
-    if (QPlatformMenu *menu = theme->createPlatformMenu()) {
-        int itemsCount = q->count();
-
-        QList<QPlatformMenuItem *> items;
-        items.reserve(itemsCount);
-        QPlatformMenuItem *currentItem = 0;
-        int currentIndex = q->currentIndex();
-
-        for (int i = 0; i < itemsCount; ++i) {
-            QPlatformMenuItem *item = theme->createPlatformMenuItem();
-            QModelIndex rowIndex = model->index(i, modelColumn, root);
-            QVariant textVariant = model->data(rowIndex, Qt::EditRole);
-            item->setText(textVariant.toString());
-            QVariant iconVariant = model->data(rowIndex, Qt::DecorationRole);
-            if (iconVariant.canConvert<QIcon>())
-                item->setIcon(iconVariant.value<QIcon>());
-            item->setCheckable(true);
-            item->setChecked(i == currentIndex);
-            if (!currentItem || i == currentIndex)
-                currentItem = item;
-
-            IndexSetter setter = { i, q };
-            QObject::connect(item, &QPlatformMenuItem::activated, setter);
-
-            menu->insertMenuItem(item, 0);
-            menu->syncMenuItem(item);
-        }
+    cleanupNativePopup();
 
-        QWindow *tlw = q->window()->windowHandle();
-        menu->setFont(q->font());
-        menu->setMinimumWidth(q->rect().width());
-        QPoint offset = QPoint(0, 7);
-        if (q->testAttribute(Qt::WA_MacSmallSize))
-            offset = QPoint(-1, 7);
-        else if (q->testAttribute(Qt::WA_MacMiniSize))
-            offset = QPoint(-2, 6);
-        menu->showPopup(tlw, QRect(tlw->mapFromGlobal(q->mapToGlobal(offset)), QSize()), currentItem);
-        menu->deleteLater();
-        Q_FOREACH (QPlatformMenuItem *item, items)
-            item->deleteLater();
-
-        // The Cocoa popup will swallow any mouse release event.
-        // We need to fake one here to un-press the button.
-        QMouseEvent mouseReleased(QEvent::MouseButtonRelease, q->pos(), Qt::LeftButton,
-                                  Qt::MouseButtons(Qt::LeftButton), Qt::KeyboardModifiers());
-        qApp->sendEvent(q, &mouseReleased);
+    QPlatformTheme *theme = QGuiApplicationPrivate::instance()->platformTheme();
+    m_platformMenu = theme->createPlatformMenu();
+    if (!m_platformMenu)
+        return false;
+
+    int itemsCount = q->count();
+    m_platformMenu->setTag(quintptr(itemsCount));
+
+    QPlatformMenuItem *currentItem = 0;
+    int currentIndex = q->currentIndex();
+
+    for (int i = 0; i < itemsCount; ++i) {
+        QPlatformMenuItem *item = theme->createPlatformMenuItem();
+        QModelIndex rowIndex = model->index(i, modelColumn, root);
+        QVariant textVariant = model->data(rowIndex, Qt::EditRole);
+        item->setText(textVariant.toString());
+        QVariant iconVariant = model->data(rowIndex, Qt::DecorationRole);
+        if (iconVariant.canConvert<QIcon>())
+            item->setIcon(iconVariant.value<QIcon>());
+        item->setCheckable(true);
+        item->setChecked(i == currentIndex);
+        if (!currentItem || i == currentIndex)
+            currentItem = item;
+
+        IndexSetter setter = { i, q };
+        QObject::connect(item, &QPlatformMenuItem::activated, setter);
+
+        m_platformMenu->insertMenuItem(item, 0);
+        m_platformMenu->syncMenuItem(item);
+    }
+
+    QWindow *tlw = q->window()->windowHandle();
+    m_platformMenu->setFont(q->font());
+    m_platformMenu->setMinimumWidth(q->rect().width());
+    QPoint offset = QPoint(0, 7);
+    if (q->testAttribute(Qt::WA_MacSmallSize))
+        offset = QPoint(-1, 7);
+    else if (q->testAttribute(Qt::WA_MacMiniSize))
+        offset = QPoint(-2, 6);
+
+    m_platformMenu->showPopup(tlw, QRect(tlw->mapFromGlobal(q->mapToGlobal(offset)), QSize()), currentItem);
 
-        return true;
-    }
+#ifdef Q_OS_OSX
+    // The Cocoa popup will swallow any mouse release event.
+    // We need to fake one here to un-press the button.
+    QMouseEvent mouseReleased(QEvent::MouseButtonRelease, q->pos(), Qt::LeftButton,
+                              Qt::MouseButtons(Qt::LeftButton), Qt::KeyboardModifiers());
+    qApp->sendEvent(q, &mouseReleased);
+#endif
 
-    return false;
+    return true;
 }
-#endif // Q_OS_OSX
+
+#endif // Q_OS_MAC
 
 /*!
     Displays the list of items in the combobox. If the list is empty
diff --git a/src/widgets/widgets/qcombobox_p.h b/src/widgets/widgets/qcombobox_p.h
index 5ee7d72e8e..88b360257f 100644
--- a/src/widgets/widgets/qcombobox_p.h
+++ b/src/widgets/widgets/qcombobox_p.h
@@ -71,6 +71,7 @@
 QT_BEGIN_NAMESPACE
 
 class QAction;
+class QPlatformMenu;
 
 class QComboBoxListView : public QListView
 {
@@ -331,7 +332,7 @@ class Q_AUTOTEST_EXPORT QComboBoxPrivate : public QWidgetPrivate
     Q_DECLARE_PUBLIC(QComboBox)
 public:
     QComboBoxPrivate();
-    ~QComboBoxPrivate() {}
+    ~QComboBoxPrivate();
     void init();
     QComboBoxPrivateContainer* viewContainer();
     void updateLineEditGeometry();
@@ -373,6 +374,7 @@ public:
     void updateFocusPolicy();
 
 #ifdef Q_OS_OSX
+    void cleanupNativePopup();
     bool showNativePopup();
 #endif
 
@@ -401,6 +403,9 @@ public:
     QPersistentModelIndex root;
     Qt::CaseSensitivity autoCompletionCaseSensitivity;
     int indexBeforeChange;
+#ifdef Q_OS_OSX
+    QPlatformMenu *m_platformMenu;
+#endif
 #ifndef QT_NO_COMPLETER
     QPointer<QCompleter> completer;
 #endif