Fix crash in QGraphicsView BSP discovered in Amarok.

Basically some items were not properly remove in the BSP which means
that if you delete one of items, the BSP tree may contain dangling pointers.
The problem was in removeItemHelper in QGraphicsScene were the child
were removed after reparenting to 0 the topmost parent. The
sceneBoundingRect for children was invalid which means that we were removing
them in the wrong position inside the BSP. Reparenting to 0 means that the
sceneBoundingRect will be the boundingRect but wasn't the case before
(for the topmost parent).

Reviewed-by:bnilsen

2 files changed, 66 insertions, 6 deletions

diff --git a/src/gui/graphicsview/qgraphicsscene.cpp b/src/gui/graphicsview/qgraphicsscene.cpp
index a624b10..03c8a97 100644
--- a/src/gui/graphicsview/qgraphicsscene.cpp
+++ b/src/gui/graphicsview/qgraphicsscene.cpp
@@ -518,6 +518,14 @@ void QGraphicsScenePrivate::removeItemHelper(QGraphicsItem *item)
 
     item->d_func()->scene = 0;
 
+    //We need to remove all children first because they might use their parent
+    //attributes (e.g. sceneTransform).
+    if (!item->d_ptr->inDestructor) {
+        // Remove all children recursively
+        for (int i = 0; i < item->d_ptr->children.size(); ++i)
+            q->removeItem(item->d_ptr->children.at(i));
+    }
+
     // Unregister focus proxy.
     item->d_ptr->resetFocusProxy();
 
@@ -564,12 +572,6 @@ void QGraphicsScenePrivate::removeItemHelper(QGraphicsItem *item)
             ++iterator;
     }
 
-    if (!item->d_ptr->inDestructor) {
-        // Remove all children recursively
-        for (int i = 0; i < item->d_ptr->children.size(); ++i)
-            q->removeItem(item->d_ptr->children.at(i));
-    }
-
     if (item->isPanel() && item->isVisible() && item->panelModality() != QGraphicsItem::NonModal)
         leaveModal(item);
 
diff --git a/tests/auto/qgraphicssceneindex/tst_qgraphicssceneindex.cpp b/tests/auto/qgraphicssceneindex/tst_qgraphicssceneindex.cpp
index 1109e5e..9dfd486 100644
--- a/tests/auto/qgraphicssceneindex/tst_qgraphicssceneindex.cpp
+++ b/tests/auto/qgraphicssceneindex/tst_qgraphicssceneindex.cpp
@@ -66,6 +66,7 @@ private slots:
     void movingItems();
     void connectedToSceneRectChanged();
     void items();
+    void removeItems();
     void clear();
 
 private:
@@ -268,6 +269,63 @@ void tst_QGraphicsSceneIndex::items()
     QCOMPARE(scene.items().size(), 3);
 }
 
+class RectWidget : public QGraphicsWidget
+{
+    Q_OBJECT
+public:
+    RectWidget(QGraphicsItem *parent = 0) : QGraphicsWidget(parent)
+    {
+    }
+
+    void paint(QPainter *painter, const QStyleOptionGraphicsItem *option, QWidget *widget)
+    {
+        painter->setBrush(brush);
+        painter->drawRect(boundingRect());
+    }
+public:
+    QBrush brush;
+};
+
+void tst_QGraphicsSceneIndex::removeItems()
+{
+     QGraphicsScene scene;
+
+    RectWidget *parent = new RectWidget;
+    parent->brush = QBrush(QColor(Qt::magenta));
+    parent->setGeometry(250, 250, 400, 400);
+
+    RectWidget *widget = new RectWidget(parent);
+    widget->brush = QBrush(QColor(Qt::blue));
+    widget->setGeometry(10, 10, 200, 200);
+
+    RectWidget *widgetChild1 = new RectWidget(widget);
+    widgetChild1->brush = QBrush(QColor(Qt::green));
+    widgetChild1->setGeometry(20, 20, 100, 100);
+
+    RectWidget *widgetChild2 = new RectWidget(widgetChild1);
+    widgetChild2->brush = QBrush(QColor(Qt::yellow));
+    widgetChild2->setGeometry(25, 25, 50, 50);
+
+    scene.addItem(parent);
+
+    QGraphicsView view(&scene);
+    view.resize(600, 600);
+    view.show();
+    QApplication::setActiveWindow(&view);
+    QTest::qWaitForWindowShown(&view);
+
+    QApplication::processEvents();
+
+    scene.removeItem(widgetChild1);
+
+    delete widgetChild1;
+
+    //We move the parent
+    scene.items(295, 295, 50, 50);
+
+    //This should not crash
+}
+
 void tst_QGraphicsSceneIndex::clear()
 {
     class MyItem : public QGraphicsItem