From f7be6402e089aefd445a5ab98912ff5aea511cea Mon Sep 17 00:00:00 2001
From: Andreas Aardal Hanssen <andreas.aardal.hanssen@nokia.com>
Date: Fri, 4 Sep 2009 11:56:43 +0200
Subject: Fix crashes when moving descendents during destruction.

This fix prevents crashes in
QGraphicsItem caused by calls to pure virtual functions as an item
tries to access its dying parent. This happens when an item is moved
by an ancestor during this ancestor's destructor.

Autotests included.

Reviewed-by: bnilsen
---
 src/gui/graphicsview/qgraphicsitem.cpp         | 18 +++++---
 tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp | 60 ++++++++++++++++++++++++++
 2 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/src/gui/graphicsview/qgraphicsitem.cpp b/src/gui/graphicsview/qgraphicsitem.cpp
index 16a9a0a..eae3e63 100644
--- a/src/gui/graphicsview/qgraphicsitem.cpp
+++ b/src/gui/graphicsview/qgraphicsitem.cpp
@@ -4770,7 +4770,7 @@ void QGraphicsItemPrivate::updateCachedClipPathFromSetPosHelper(const QPointF &n
 {
     Q_ASSERT(inSetPosHelper);
 
-    if (!(ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren))
+    if (inDestructor || !(ancestorFlags & QGraphicsItemPrivate::AncestorClipsChildren))
         return; // Not clipped by any ancestor.
 
     // Find closest clip ancestor and transform.
@@ -4784,13 +4784,15 @@ void QGraphicsItemPrivate::updateCachedClipPathFromSetPosHelper(const QPointF &n
         thisToParentTransform *= clipParent->d_ptr->transformToParent();
         clipParent = clipParent->d_ptr->parent;
     }
-    if (clipParent && clipParent->d_ptr->inDestructor)
-        return;
 
-    // thisToParentTransform is now the same as q->itemTransform(clipParent), except
-    // that the new position (which is not yet set on the item) is taken into account.
-    Q_ASSERT(clipParent);
-    Q_ASSERT(clipParent->d_ptr->flags & QGraphicsItem::ItemClipsChildrenToShape);
+    // Ensure no parents are currently being deleted. This can only
+    // happen if the item is moved by a dying ancestor.
+    QGraphicsItem *p = clipParent;
+    while (p) {
+        if (p->d_ptr->inDestructor)
+            return;
+        p = p->d_ptr->parent;
+    }
 
     // From here everything is calculated in clip parent's coordinates.
     const QRectF parentBoundingRect(clipParent->boundingRect());
@@ -6852,6 +6854,8 @@ void QGraphicsItem::removeFromIndex()
 */
 void QGraphicsItem::prepareGeometryChange()
 {
+    if (d_ptr->inDestructor)
+        return;
     if (d_ptr->scene) {
         d_ptr->scene->d_func()->dirtyGrowingItemsBoundingRect = true;
         d_ptr->geometryChanged = 1;
diff --git a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
index 408decc..77c2d45 100644
--- a/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
+++ b/tests/auto/qgraphicsitem/tst_qgraphicsitem.cpp
@@ -292,6 +292,7 @@ private slots:
     void activate();
     void setActivePanelOnInactiveScene();
     void activationOnShowHide();
+    void moveWhileDeleting();
 
     // task specific tests below me
     void task141694_textItemEnsureVisible();
@@ -8103,5 +8104,64 @@ void tst_QGraphicsItem::activationOnShowHide()
     QVERIFY(otherItem->isActive());
 }
 
+class MoveWhileDying : public QGraphicsRectItem
+{
+public:
+    MoveWhileDying(QGraphicsItem *parent = 0)
+        : QGraphicsRectItem(parent)
+    { }
+    ~MoveWhileDying()
+    {
+        foreach (QGraphicsItem *c, childItems()) {
+            foreach (QGraphicsItem *cc, c->childItems()) {
+                cc->moveBy(10, 10);
+            }
+            c->moveBy(10, 10);
+        }
+        if (QGraphicsItem *p = parentItem()) { p->moveBy(10, 10); }
+    }
+};
+
+void tst_QGraphicsItem::moveWhileDeleting()
+{
+    QGraphicsScene scene;
+    QGraphicsRectItem *rect = new QGraphicsRectItem;
+    MoveWhileDying *silly = new MoveWhileDying(rect);
+    QGraphicsRectItem *child = new QGraphicsRectItem(silly);
+    scene.addItem(rect);
+    delete rect; // don't crash!
+
+    rect = new QGraphicsRectItem;
+    silly = new MoveWhileDying(rect);
+    child = new QGraphicsRectItem(silly);
+
+    QGraphicsView view(&scene);
+    view.show();
+#ifdef Q_WS_X11
+    qt_x11_wait_for_window_manager(&view);
+#endif
+    QTest::qWait(125);
+
+    delete rect;
+
+    rect = new QGraphicsRectItem;
+    rect->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+    silly = new MoveWhileDying(rect);
+    child = new QGraphicsRectItem(silly);
+
+    QTest::qWait(125);
+
+    delete rect;
+
+    rect = new MoveWhileDying;
+    rect->setFlag(QGraphicsItem::ItemClipsChildrenToShape);
+    child = new QGraphicsRectItem(rect);
+    silly = new MoveWhileDying(child);
+
+    QTest::qWait(125);
+
+    delete rect;
+}
+
 QTEST_MAIN(tst_QGraphicsItem)
 #include "tst_qgraphicsitem.moc"
-- 
cgit v0.12