Fix crash in QMenu when using QWidgetAction

If the QWidgetAction is not the first in a menu, it crashed.
This is because when the QEvent::ActionRemoved is sent, the
event->before() is not set.  We need to find another way to
clean up the widgetItems list.

This basically revert 4b6ab00e6d68c7

Reviewed-by: Thierry

3 files changed, 21 insertions, 24 deletions

diff --git a/src/gui/widgets/qmenu.cpp b/src/gui/widgets/qmenu.cpp
index 1b5d1cd..e78c1b4 100644
--- a/src/gui/widgets/qmenu.cpp
+++ b/src/gui/widgets/qmenu.cpp
@@ -273,7 +273,7 @@ void QMenuPrivate::updateActionRects() const
 
     for (int i = 0; i < actions.count(); ++i) {
         QAction *action = actions.at(i);
-        if (action->isSeparator() || !action->isVisible() || widgetItems.at(i))
+        if (action->isSeparator() || !action->isVisible() || widgetItems.contains(action))
             continue;
         //..and some members
         hasCheckableItems |= action->isCheckable();
@@ -301,7 +301,7 @@ void QMenuPrivate::updateActionRects() const
         const QFontMetrics &fm = opt.fontMetrics;
 
         QSize sz;
-        if (QWidget *w = widgetItems.at(i)) {
+        if (QWidget *w = widgetItems.value(action)) {
           sz = w->sizeHint().expandedTo(w->minimumSize()).expandedTo(w->minimumSizeHint()).boundedTo(w->maximumSize());
         } else {
             //calc what I think the size is..
@@ -370,7 +370,7 @@ void QMenuPrivate::updateActionRects() const
         rect.setWidth(max_column_width); //uniform width
 
         //we need to update the widgets geometry
-        if (QWidget *widget = widgetItems.at(i)) {
+        if (QWidget *widget = widgetItems.value(actions.at(i))) {
             widget->setGeometry(rect);
             widget->setVisible(actions.at(i)->isVisible());
         }
@@ -583,8 +583,7 @@ void QMenuPrivate::setCurrentAction(QAction *action, int popup, SelectionReason
             q->update(actionRect(action));
 
             if (reason == SelectedFromKeyboard) {
-                const int actionIndex = actions.indexOf(action);
-                QWidget *widget = widgetItems.at(actionIndex);
+                QWidget *widget = widgetItems.value(action);
                 if (widget) {
                     if (widget->focusPolicy() != Qt::NoFocus)
                         widget->setFocus(Qt::TabFocusReason);
@@ -800,7 +799,7 @@ void QMenuPrivate::scrollMenu(QAction *action, QMenuScroller::ScrollLocation loc
             current.moveTop(current.top() + delta);
 
             //we need to update the widgets geometry
-            if (QWidget *w = widgetItems.at(i))
+            if (QWidget *w = widgetItems.value(actions.at(i)))
                 w->setGeometry(current);
         }
     }
@@ -1392,11 +1391,12 @@ QMenu::QMenu(QMenuPrivate &dd, QWidget *parent)
 QMenu::~QMenu()
 {
     Q_D(QMenu);
-    for (int i = 0; i < d->widgetItems.count(); ++i) {
-        if (QWidget *widget = d->widgetItems.at(i)) {
-            QWidgetAction *action = static_cast<QWidgetAction *>(d->actions.at(i));
+    QHash<QAction *, QWidget *>::iterator it = d->widgetItems.begin();
+    for (; it != d->widgetItems.end(); ++it) {
+        if (QWidget *widget = it.value()) {
+            QWidgetAction *action = static_cast<QWidgetAction *>(it.key());
             action->releaseWidget(widget);
-            d->widgetItems[i] = 0;
+            *it = 0;
         }
     }
 
@@ -2151,7 +2151,7 @@ void QMenu::paintEvent(QPaintEvent *e)
         QAction *action = d->actions.at(i);
         QRect adjustedActionRect = d->actionRects.at(i);
         if (!e->rect().intersects(adjustedActionRect)
-            || d->widgetItems.at(i))
+            || d->widgetItems.value(action))
            continue;
         //set the clip region to be extra safe (and adjust for the scrollers)
         QRegion adjustedActionReg(adjustedActionRect);
@@ -2862,25 +2862,20 @@ void QMenu::actionEvent(QActionEvent *e)
             connect(e->action(), SIGNAL(triggered()), this, SLOT(_q_actionTriggered()));
             connect(e->action(), SIGNAL(hovered()), this, SLOT(_q_actionHovered()));
         }
-        QWidget *widget = 0;
-        if (QWidgetAction *wa = qobject_cast<QWidgetAction *>(e->action()))
-            widget = wa->requestWidget(this);
-
-        int index = d->actions.indexOf(e->action());
-        Q_ASSERT(index != -1);
-        d->widgetItems.insert(index, widget);
-
+        if (QWidgetAction *wa = qobject_cast<QWidgetAction *>(e->action())) {
+            QWidget *widget = wa->requestWidget(this);
+            if (widget)
+                d->widgetItems.insert(wa, widget);
+        }
     } else if (e->type() == QEvent::ActionRemoved) {
         e->action()->disconnect(this);
         if (e->action() == d->currentAction)
             d->currentAction = 0;
-        int index = d->actions.indexOf(e->before()) + 1;
         if (QWidgetAction *wa = qobject_cast<QWidgetAction *>(e->action())) {
-            if (QWidget *widget = d->widgetItems.at(index))
+            if (QWidget *widget = d->widgetItems.value(wa))
                 wa->releaseWidget(widget);
         }
-        Q_ASSERT(index != -1);
-        d->widgetItems.removeAt(index);
+        d->widgetItems.remove(e->action());
     }
 
 #ifdef Q_WS_MAC
diff --git a/src/gui/widgets/qmenu_p.h b/src/gui/widgets/qmenu_p.h
index 9348f7b..a5bde7c 100644
--- a/src/gui/widgets/qmenu_p.h
+++ b/src/gui/widgets/qmenu_p.h
@@ -190,7 +190,7 @@ public:
     QRect actionRect(QAction *) const;
 
     mutable QVector<QRect> actionRects;
-    mutable QWidgetList widgetItems;
+    mutable QHash<QAction *, QWidget *> widgetItems;
     void updateActionRects() const;
     QRect popupGeometry(const QWidget *widget) const;
     QRect popupGeometry(int screen = -1) const;
diff --git a/tests/auto/qwidgetaction/tst_qwidgetaction.cpp b/tests/auto/qwidgetaction/tst_qwidgetaction.cpp
index 50b3337..d25738f 100644
--- a/tests/auto/qwidgetaction/tst_qwidgetaction.cpp
+++ b/tests/auto/qwidgetaction/tst_qwidgetaction.cpp
@@ -395,7 +395,9 @@ void tst_QWidgetAction::releaseWidgetCrash()
     QMainWindow *w = new QMainWindow;
     QAction *a = new CrashedAction(w);
     QMenu *menu = w->menuBar()->addMenu("Test");
+    menu->addAction("foo");
     menu->addAction(a);
+    menu->addAction("bar");
     delete w;
 }