Fixed a potential crash in QGraphicsScenePrivate::_q_polishItems()

This patch make sure that we always start from the beginning of
the unpolished items list and we erase the first value at each iteration.
The patch also convert the list to a set that is more appropriate here.

Merge-request: 1707
Reviewed-by: Alexis Menard <alexis.menard@nokia.com>

3 files changed, 36 insertions, 5 deletions

diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index 961f44f..056a7ce 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -420,8 +420,12 @@ void QGraphicsScenePrivate::unregisterTopLevelItem(QGraphicsItem *item)
 */
 void QGraphicsScenePrivate::_q_polishItems()
 {
+    QSet<QGraphicsItem *>::Iterator it;
     const QVariant booleanTrueVariant(true);
-    foreach (QGraphicsItem *item, unpolishedItems) {
+    while (!unpolishedItems.isEmpty()) {
+        it = unpolishedItems.begin();
+        QGraphicsItem *item = *it;
+        unpolishedItems.erase(it);
         if (!item->d_ptr->explicitlyHidden) {
             item->itemChange(QGraphicsItem::ItemVisibleChange, booleanTrueVariant);
             item->itemChange(QGraphicsItem::ItemVisibleHasChanged, booleanTrueVariant);
@@ -431,7 +435,6 @@ void QGraphicsScenePrivate::_q_polishItems()
             QApplication::sendEvent((QGraphicsWidget *)item, &event);
         }
     }
-    unpolishedItems.clear();
 }
 
 void QGraphicsScenePrivate::_q_processDirtyItems()
@@ -549,7 +552,7 @@ void QGraphicsScenePrivate::removeItemHelper(QGraphicsItem *item)
     selectedItems.remove(item);
     hoverItems.removeAll(item);
     cachedItemsUnderMouse.removeAll(item);
-    unpolishedItems.removeAll(item);
+    unpolishedItems.remove(item);
     resetDirtyItem(item);
 
     //We remove all references of item from the sceneEventFilter arrays
@@ -2484,7 +2487,7 @@ void QGraphicsScene::addItem(QGraphicsItem *item)
     if (!item->d_ptr->explicitlyHidden) {
        if (d->unpolishedItems.isEmpty())
            QMetaObject::invokeMethod(this, "_q_polishItems", Qt::QueuedConnection);
-       d->unpolishedItems << item;
+       d->unpolishedItems.insert(item);
     }
 
     // Reenable selectionChanged() for individual items
diff --git a/src/gui/graphicsview/qgraphicsscene_p.h b/src/gui/graphicsview/qgraphicsscene_p.h
index 5000860..8073695 100644
--- a/src/gui/graphicsview/qgraphicsscene_p.h
+++ b/src/gui/graphicsview/qgraphicsscene_p.h
@@ -108,7 +108,7 @@ public:
     QPainterPath selectionArea;
     int selectionChanging;
     QSet<QGraphicsItem *> selectedItems;
-    QList<QGraphicsItem *> unpolishedItems;
+    QSet<QGraphicsItem *> unpolishedItems;
     QList<QGraphicsItem *> topLevelItems;
     bool needSortTopLevelItems;
     bool holesInTopLevelSiblingIndex;
diff --git a/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp b/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp
index 8459331..6c5fe90 100644
--- a/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp
+++ b/tests/auto/qgraphicsscene/tst_qgraphicsscene.cpp
@@ -266,6 +266,7 @@ private slots:
     void dispatchHoverOnPress();
     void initialFocus_data();
     void initialFocus();
+    void polishItems();
 
     // task specific tests below me
     void task139710_bspTreeCrash();
@@ -3884,5 +3885,32 @@ void tst_QGraphicsScene::initialFocus()
     QCOMPARE(rect->hasFocus(), shouldHaveFocus);
 }
 
+class PolishItem : public QGraphicsTextItem
+{
+public:
+    PolishItem(QGraphicsItem *parent = 0) : QGraphicsTextItem(parent) { }
+
+protected:
+    QVariant itemChange(GraphicsItemChange change, const QVariant& value)
+    {
+        if (change == ItemVisibleChange) {
+            if (value.toBool())
+                qDeleteAll(childItems());
+        }
+        return QGraphicsItem::itemChange(change, value);
+    }
+};
+
+void tst_QGraphicsScene::polishItems()
+{
+    QGraphicsScene scene;
+    PolishItem *parent = new PolishItem;
+    scene.addItem(parent);
+    PolishItem *child = new PolishItem(parent);
+    Q_UNUSED(child)
+    // test that QGraphicsScenePrivate::_q_polishItems() doesn't crash
+    QMetaObject::invokeMethod(&scene,"_q_polishItems");
+}
+
 QTEST_MAIN(tst_QGraphicsScene)
 #include "tst_qgraphicsscene.moc"