Fix a bug where a layout could crash or become non-responsive

This happened because of that QQuickText is ill-behaving: When the width
on a QQuickText with word wrap enabled is changed to something less than
its implicitWidth, its implicitHeight is changed to reflect the height
it needs in order to fit all lines. This will cause the layout to be
invalidated, and rearranged again.

However, at the same time it will also change its implicitWidth actually
become wider than its initial implicitWidth (this seems to be a bug). So
the next time it is rearranged it will actually be wide enough so that
it doesn't need to wrap. This again will cause its implicitWidth and
implicitHeight to change to reflect that only one line is needed, which
again will cause it to rearrange, but since the item has a too small
width for that it will again change the implicitHeight to reflect that
it needs more lines.....

This went on forever until there was a stack overflow. In addition it
also caused an endless (that is, if it didn't stack overflow)
updatePolish()/polish() loop (basically polish() was called from within
updatePolish() continuously).

To make the layout more robust for such "ill-behaving" items we have to
add one recursion guard, and one polish-loop guard.

Change-Id: I9f752ed320a100c8d0f0fd340347a556e63318e5
Task-number: QTBUG-73683
Reviewed-by: Richard Moe Gustavsen <richard.gustavsen@qt.io>

5 files changed, 65 insertions, 5 deletions

diff --git a/src/imports/layouts/qquicklayout.cpp b/src/imports/layouts/qquicklayout.cpp
index 450cf26cea..17d37b5c4e 100644
--- a/src/imports/layouts/qquicklayout.cpp
+++ b/src/imports/layouts/qquicklayout.cpp
@@ -701,8 +701,10 @@ QQuickItem *QQuickLayoutAttached::item() const
 
 
 QQuickLayout::QQuickLayout(QQuickLayoutPrivate &dd, QQuickItem *parent)
-    : QQuickItem(dd, parent),
-      m_dirty(false)
+    : QQuickItem(dd, parent)
+    , m_dirty(false)
+    , m_inUpdatePolish(false)
+    , m_polishInsideUpdatePolish(0)
 {
 }
 
@@ -729,7 +731,9 @@ QQuickLayoutAttached *QQuickLayout::qmlAttachedProperties(QObject *object)
 
 void QQuickLayout::updatePolish()
 {
+    m_inUpdatePolish = true;
     rearrange(QSizeF(width(), height()));
+    m_inUpdatePolish = false;
 }
 
 void QQuickLayout::componentComplete()
@@ -750,7 +754,18 @@ void QQuickLayout::invalidate(QQuickItem * /*childItem*/)
 
     if (!qobject_cast<QQuickLayout *>(parentItem())) {
         quickLayoutDebug() << "QQuickLayout::invalidate(), polish()";
-        polish();
+
+        if (m_inUpdatePolish)
+            ++m_polishInsideUpdatePolish;
+        else
+            m_polishInsideUpdatePolish = 0;
+
+        if (m_polishInsideUpdatePolish <= 2)
+            // allow at most two consecutive loops in order to respond to height-for-width
+            // (e.g QQuickText changes implicitHeight when its width gets changed)
+            polish();
+        else
+            qWarning() << "Qt Quick Layouts: Polish loop detected. Aborting after two iterations.";
     }
 }
 
diff --git a/src/imports/layouts/qquicklayout_p.h b/src/imports/layouts/qquicklayout_p.h
index 3f076c7304..d2421667be 100644
--- a/src/imports/layouts/qquicklayout_p.h
+++ b/src/imports/layouts/qquicklayout_p.h
@@ -119,7 +119,9 @@ protected slots:
     void invalidateSenderItem();
 
 private:
-    bool m_dirty;
+    unsigned m_dirty : 1;
+    unsigned m_inUpdatePolish : 1;
+    unsigned m_polishInsideUpdatePolish : 2;
 
     Q_DECLARE_PRIVATE(QQuickLayout)
 
diff --git a/src/imports/layouts/qquicklinearlayout.cpp b/src/imports/layouts/qquicklinearlayout.cpp
index c8d2ac5eb2..911793d748 100644
--- a/src/imports/layouts/qquicklinearlayout.cpp
+++ b/src/imports/layouts/qquicklinearlayout.cpp
@@ -469,6 +469,16 @@ void QQuickGridLayoutBase::rearrange(const QSizeF &size)
     if (!isReady())
         return;
 
+    const auto refCounter = qScopeGuard([&d] {
+        --(d->m_recurRearrangeCounter);
+    });
+    if (d->m_recurRearrangeCounter++ == 2) {
+        // allow a recursive depth of two in order to respond to height-for-width
+        // (e.g QQuickText changes implicitHeight when its width gets changed)
+        qWarning() << "Qt Quick Layouts: Detected recursive rearrange. Aborting after two iterations.";
+        return;
+    }
+
     d->m_rearranging = true;
     quickLayoutDebug() << objectName() << "QQuickGridLayoutBase::rearrange()" << size;
     Qt::LayoutDirection visualDir = effectiveLayoutDirection();
diff --git a/src/imports/layouts/qquicklinearlayout_p.h b/src/imports/layouts/qquicklinearlayout_p.h
index 6706ebf9fa..866d0884a2 100644
--- a/src/imports/layouts/qquicklinearlayout_p.h
+++ b/src/imports/layouts/qquicklinearlayout_p.h
@@ -104,7 +104,8 @@ class QQuickGridLayoutBasePrivate : public QQuickLayoutPrivate
     Q_DECLARE_PUBLIC(QQuickGridLayoutBase)
 
 public:
-    QQuickGridLayoutBasePrivate() : m_rearranging(false)
+    QQuickGridLayoutBasePrivate() : m_recurRearrangeCounter(0)
+                                    , m_rearranging(false)
                                     , m_updateAfterRearrange(false)
                                     , m_layoutDirection(Qt::LeftToRight)
                                     {}
@@ -117,6 +118,7 @@ public:
 
     QQuickGridLayoutEngine engine;
     Qt::Orientation orientation;
+    unsigned m_recurRearrangeCounter : 2;
     unsigned m_rearranging : 1;
     unsigned m_updateAfterRearrange : 1;
     QVector<QQuickItem *> m_invalidateAfterRearrange;
diff --git a/tests/auto/quick/qquicklayouts/data/tst_rowlayout.qml b/tests/auto/quick/qquicklayouts/data/tst_rowlayout.qml
index 07af6a77ac..0732884c97 100644
--- a/tests/auto/quick/qquicklayouts/data/tst_rowlayout.qml
+++ b/tests/auto/quick/qquicklayouts/data/tst_rowlayout.qml
@@ -1100,5 +1100,36 @@ Item {
             waitForRendering(rootRect.layout)
             compare(rootRect.item1.width, 100)
         }
+
+//---------------------------
+        Component {
+            id: rowlayoutWithTextItems_Component
+            RowLayout {
+                Text {
+                    Layout.fillWidth: true
+                    text: "OneWord"
+                    wrapMode: Text.WrapAtWordBoundaryOrAnywhere
+                }
+                Text {
+                    Layout.fillWidth: true
+                    text: "OneWord"
+                    wrapMode: Text.WrapAtWordBoundaryOrAnywhere
+                }
+            }
+        }
+
+        // QTBUG-73683
+        function test_rowlayoutWithTextItems() {
+            var layout = createTemporaryObject(rowlayoutWithTextItems_Component, container)
+            waitForRendering(layout)
+            for (var i = 0; i < 3; i++) {
+                ignoreWarning(/Qt Quick Layouts: Detected recursive rearrange. Aborting after two iterations./)
+            }
+            ignoreWarning(/Qt Quick Layouts: Polish loop detected. Aborting after two iterations./)
+            layout.width = layout.width - 2     // set the size to be smaller than its "minimum size"
+            waitForRendering(layout)    // do not exit before all warnings have been received
+
+            // DO NOT CRASH due to stack overflow (or loop endlessly due to updatePolish()/polish() loop)
+        }
     }
 }