From cf6f7de3f03d52174f3dfa57031159caa4e7be31 Mon Sep 17 00:00:00 2001
From: Jan Arve Saether <jan-arve.saether@digia.com>
Date: Wed, 29 May 2013 15:34:48 +0200
Subject: [PATCH] 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>
---
src/layouts/qquicklinearlayout.cpp | 29 ++++++++++++++++------
src/layouts/qquicklinearlayout_p.h | 7 +++---
tests/auto/controls/data/tst_rowlayout.qml | 20 +++++++++++++++
3 files changed, 46 insertions(+), 10 deletions(-)
diff --git a/src/layouts/qquicklinearlayout.cpp b/src/layouts/qquicklinearlayout.cpp
index b9a8e2249..13b582f9a 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 76cc206b9..b38bd97f7 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 fd21f4756..458a2885f 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
+ }
}
}
--
GitLab