Do not crash when deleting a layout

onItemVisibleChanged() was in some cases called after the dtor of
QQuickGridLayoutBase was called.

After having called the dtor of QQuickGridLayoutBase it would
continue to call the dtors of its base classes until it entered the
dtor of QQuickItem.  In QQuickItem it would call setParentItem(0) on
all its child items. This caused the item to become visible again,
thus it would emit visibleChanged() and finally invoke
QQuickGridLayoutBase::onItemVisibleChanged() which lead to the crash
(while trying to call the virtual invalidate())

The fix is to do an early return if we know that the layout is
in the destruction phase.
isReady() will return only true *after* the component is completed,
and before the component is destructing.

Task-number: QTBUG-31276
Change-Id: I191e348278e3d052c109bffb92a1ccd9326859bd
Reviewed-by: Jens Bache-Wiig <jens.bache-wiig@digia.com>

3 files changed, 46 insertions, 10 deletions

diff --git a/src/layouts/qquicklinearlayout.cpp b/src/layouts/qquicklinearlayout.cpp
index b9a8e224..13b582f9 100644
--- a/src/layouts/qquicklinearlayout.cpp
+++ b/src/layouts/qquicklinearlayout.cpp
@@ -184,12 +184,18 @@ QSizeF QQuickGridLayoutBase::sizeHint(Qt::SizeHint whichSizeHint) const
     return d->engine.sizeHint(whichSizeHint, QSizeF());
 }
 
+QQuickGridLayoutBase::~QQuickGridLayoutBase()
+{
+    d_func()->m_isReady = false;
+}
+
 void QQuickGridLayoutBase::componentComplete()
 {
     Q_D(QQuickGridLayoutBase);
     quickLayoutDebug() << objectName() << "QQuickGridLayoutBase::componentComplete()" << parent();
     d->m_disableRearrange = true;
     QQuickLayout::componentComplete();    // will call our geometryChange(), (where isComponentComplete() == true)
+    d->m_isReady = true;
     d->m_disableRearrange = false;
     updateLayoutItems();
 
@@ -235,7 +241,7 @@ void QQuickGridLayoutBase::componentComplete()
 void QQuickGridLayoutBase::invalidate(QQuickItem *childItem)
 {
     Q_D(QQuickGridLayoutBase);
-    if (!isComponentComplete())
+    if (!isReady())
         return;
     quickLayoutDebug() << "QQuickGridLayoutBase::invalidate()";
 
@@ -274,7 +280,7 @@ void QQuickGridLayoutBase::invalidate(QQuickItem *childItem)
 void QQuickGridLayoutBase::updateLayoutItems()
 {
     Q_D(QQuickGridLayoutBase);
-    if (!isComponentComplete() || !isVisible())
+    if (!isReady() || !isVisible())
         return;
     quickLayoutDebug() << "QQuickGridLayoutBase::updateLayoutItems";
     d->engine.deleteItems();
@@ -294,7 +300,7 @@ void QQuickGridLayoutBase::itemChange(ItemChange change, const ItemChangeData &v
         QObject::connect(item, SIGNAL(implicitWidthChanged()), this, SLOT(onItemImplicitSizeChanged()));
         QObject::connect(item, SIGNAL(implicitHeightChanged()), this, SLOT(onItemImplicitSizeChanged()));
 
-        if (isComponentComplete() && isVisible())
+        if (isReady() && isVisible())
             updateLayoutItems();
     } else if (change == ItemChildRemovedChange) {
         quickLayoutDebug() << "ItemChildRemovedChange";
@@ -303,7 +309,7 @@ void QQuickGridLayoutBase::itemChange(ItemChange change, const ItemChangeData &v
         QObject::disconnect(item, SIGNAL(visibleChanged()), this, SLOT(onItemVisibleChanged()));
         QObject::disconnect(item, SIGNAL(implicitWidthChanged()), this, SLOT(onItemImplicitSizeChanged()));
         QObject::disconnect(item, SIGNAL(implicitHeightChanged()), this, SLOT(onItemImplicitSizeChanged()));
-        if (isComponentComplete() && isVisible())
+        if (isReady() && isVisible())
             updateLayoutItems();
     }
 
@@ -314,7 +320,7 @@ void QQuickGridLayoutBase::geometryChanged(const QRectF &newGeometry, const QRec
 {
     Q_D(QQuickGridLayoutBase);
     QQuickLayout::geometryChanged(newGeometry, oldGeometry);
-    if (d->m_disableRearrange || !isComponentComplete() || !newGeometry.isValid())
+    if (d->m_disableRearrange || !isReady() || !newGeometry.isValid())
         return;
     quickLayoutDebug() << "QQuickGridLayoutBase::geometryChanged" << newGeometry << oldGeometry;
     rearrange(newGeometry.size());
@@ -328,6 +334,11 @@ void QQuickGridLayoutBase::removeGridItem(QGridLayoutItem *gridItem)
     d->engine.removeRows(index, 1, d->orientation);
 }
 
+bool QQuickGridLayoutBase::isReady() const
+{
+    return d_func()->m_isReady;
+}
+
 void QQuickGridLayoutBase::removeLayoutItem(QQuickItem *item)
 {
     Q_D(QQuickGridLayoutBase);
@@ -341,7 +352,7 @@ void QQuickGridLayoutBase::removeLayoutItem(QQuickItem *item)
 
 void QQuickGridLayoutBase::onItemVisibleChanged()
 {
-    if (!isComponentComplete())
+    if (!isReady())
         return;
     quickLayoutDebug() << "QQuickGridLayoutBase::onItemVisibleChanged";
     updateLayoutItems();
@@ -349,6 +360,8 @@ void QQuickGridLayoutBase::onItemVisibleChanged()
 
 void QQuickGridLayoutBase::onItemDestroyed()
 {
+    if (!isReady())
+        return;
     Q_D(QQuickGridLayoutBase);
     quickLayoutDebug() << "QQuickGridLayoutBase::onItemDestroyed";
     QQuickItem *inDestruction = static_cast<QQuickItem *>(sender());
@@ -361,6 +374,8 @@ void QQuickGridLayoutBase::onItemDestroyed()
 
 void QQuickGridLayoutBase::onItemImplicitSizeChanged()
 {
+    if (!isReady())
+        return;
     QQuickItem *item = static_cast<QQuickItem *>(sender());
     Q_ASSERT(item);
     invalidate(item);
@@ -369,7 +384,7 @@ void QQuickGridLayoutBase::onItemImplicitSizeChanged()
 void QQuickGridLayoutBase::rearrange(const QSizeF &size)
 {
     Q_D(QQuickGridLayoutBase);
-    if (!isComponentComplete())
+    if (!isReady())
         return;
 
     quickLayoutDebug() << objectName() << "QQuickGridLayoutBase::rearrange()" << size;
diff --git a/src/layouts/qquicklinearlayout_p.h b/src/layouts/qquicklinearlayout_p.h
index 76cc206b..b38bd97f 100644
--- a/src/layouts/qquicklinearlayout_p.h
+++ b/src/layouts/qquicklinearlayout_p.h
@@ -62,8 +62,7 @@ public:
     explicit QQuickGridLayoutBase(QQuickGridLayoutBasePrivate &dd,
                                   Qt::Orientation orientation,
                                   QQuickItem *parent = 0);
-    ~QQuickGridLayoutBase() {}
-
+    ~QQuickGridLayoutBase();
     void componentComplete();
     void invalidate(QQuickItem *childItem = 0);
     Qt::Orientation orientation() const;
@@ -86,6 +85,7 @@ protected slots:
 
 private:
     void removeGridItem(QGridLayoutItem *gridItem);
+    bool isReady() const;
     Q_DECLARE_PRIVATE(QQuickGridLayoutBase)
 };
 
@@ -95,10 +95,11 @@ class QQuickGridLayoutBasePrivate : public QQuickLayoutPrivate
     Q_DECLARE_PUBLIC(QQuickGridLayoutBase)
 
 public:
-    QQuickGridLayoutBasePrivate() : m_disableRearrange(true) { }
+    QQuickGridLayoutBasePrivate() : m_disableRearrange(true), m_isReady(false) { }
     QQuickGridLayoutEngine engine;
     Qt::Orientation orientation;
     bool m_disableRearrange;
+    bool m_isReady;
     QSet<QQuickItem *> m_ignoredItems;
 };
 
diff --git a/tests/auto/controls/data/tst_rowlayout.qml b/tests/auto/controls/data/tst_rowlayout.qml
index fd21f475..458a2885 100644
--- a/tests/auto/controls/data/tst_rowlayout.qml
+++ b/tests/auto/controls/data/tst_rowlayout.qml
@@ -633,5 +633,25 @@ Item {
             compare(r1.y, 6) // 5.5
             layout.destroy();
         }
+
+
+        Component {
+            id: layout_deleteLayout
+            ColumnLayout {
+                property int dummyproperty: 0   // yes really - its needed
+                RowLayout {
+                    Text { text: "label1" }     // yes, both are needed
+                    Text { text: "label2" }
+                }
+            }
+        }
+
+        function test_destroyLayout()
+        {
+            var layout = layout_deleteLayout.createObject(container)
+            layout.children[0].children[0].visible = true
+            layout.visible = false
+            layout.destroy()    // Do not crash
+        }
     }
 }