Fix binding loop issue during implicit size change in pane

The pane progagates implicit size change to its children for adjusting
size. There are cases where Pane refers to incorrect child item
(QQuickPanePrivate::firstChild) to notify implicit size change and this
could trigger binding loop issue.

Pane determines its first child from the content item and this varies
depending on how the content item is configured.

The following are typical configuration within pane

1:   Pane {
         contentItem: Layout { ... }
     }
     In this case, Pane uses content item as provided QQuickControl.

2:   Pane {
         Layout { ... }
     }
     In this case, Pane create QQuickContentItem and adds item
     configured within it as children to QQuickContentItem.

This patch fixes the issue by notifying implicit size changes to correct
child item.

Fixes: QTBUG-116164
Change-Id: I54ef27fae531518b98200ea829f1b4138273aa26
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>
(cherry picked from commit 5eaf5afa981a2eb3ce2059c3e2b21382c9829728)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
(cherry picked from commit 181e1080c5fa33f0260c64861551ec9e288e92f4)

4 files changed, 99 insertions, 52 deletions

diff --git a/src/quicktemplates/qquickpane.cpp b/src/quicktemplates/qquickpane.cpp
index e61967b9b1..2def00b77e 100644
--- a/src/quicktemplates/qquickpane.cpp
+++ b/src/quicktemplates/qquickpane.cpp
@@ -143,14 +143,30 @@ void QQuickPanePrivate::itemImplicitHeightChanged(QQuickItem *item)
         updateImplicitContentHeight();
 }
 
+void QQuickPanePrivate::itemDestroyed(QQuickItem *item)
+{
+    // Do this check before calling the base class implementation, as that clears contentItem.
+    if (item == firstChild)
+        firstChild = nullptr;
+
+    QQuickControlPrivate::itemDestroyed(item);
+}
+
 void QQuickPanePrivate::contentChildrenChange()
 {
     Q_Q(QQuickPane);
-    QQuickItem *newFirstChild = contentChildItems().value(0);
+
+    // The first child varies depending on how the content item is declared.
+    // If it's declared as a child of the Pane, it will be parented to the
+    // default QQuickContentItem. If it's assigned to the contentItem property
+    // directly, QQuickControl::contentItem will be used."
+    QQuickItem *newFirstChild = ((qobject_cast<QQuickContentItem *>(contentItem))
+        ? contentChildItems().value(0) : *contentItem);
+
     if (newFirstChild != firstChild) {
         if (firstChild)
             removeImplicitSizeListener(firstChild);
-        if (newFirstChild)
+        if (newFirstChild && newFirstChild != contentItem)
             addImplicitSizeListener(newFirstChild);
         firstChild = newFirstChild;
     }
diff --git a/src/quicktemplates/qquickpane_p_p.h b/src/quicktemplates/qquickpane_p_p.h
index 7a8e68a068..fc77d52d00 100644
--- a/src/quicktemplates/qquickpane_p_p.h
+++ b/src/quicktemplates/qquickpane_p_p.h
@@ -39,6 +39,7 @@ public:
 
     void itemImplicitWidthChanged(QQuickItem *item) override;
     void itemImplicitHeightChanged(QQuickItem *item) override;
+    void itemDestroyed(QQuickItem *item) override;
 
     void contentChildrenChange();
 
diff --git a/tests/auto/quickcontrols/controls/data/tst_pane.qml b/tests/auto/quickcontrols/controls/data/tst_pane.qml
index f823031c79..484ac9888a 100644
--- a/tests/auto/quickcontrols/controls/data/tst_pane.qml
+++ b/tests/auto/quickcontrols/controls/data/tst_pane.qml
@@ -4,6 +4,7 @@
 import QtQuick
 import QtTest
 import QtQuick.Controls
+import QtQuick.Layouts
 
 TestCase {
     id: testCase
@@ -18,51 +19,14 @@ TestCase {
         Pane { }
     }
 
-    Component {
-        id: oneChildPane
-        Pane {
-            Item {
-                implicitWidth: 100
-                implicitHeight: 30
-            }
-        }
-    }
-
-    Component {
-        id: twoChildrenPane
-        Pane {
-            Item {
-                implicitWidth: 100
-                implicitHeight: 30
-            }
-            Item {
-                implicitWidth: 200
-                implicitHeight: 60
-            }
-        }
-    }
-
-    Component {
-        id: contentPane
-        Pane {
-            contentItem: Item {
-                implicitWidth: 100
-                implicitHeight: 30
-            }
-        }
-    }
+    function test_implicitContentItem() {
+        var control = createTemporaryObject(pane, testCase, {width: 100, height: 100})
+        verify(control)
 
-    Component {
-        id: pressPane
-        MouseArea {
-            width: 200
-            height: 200
-            property int pressCount
-            onPressed: ++pressCount
-            Pane {
-                anchors.fill: parent
-            }
-        }
+        compare(control.width, 100)
+        compare(control.height, 100)
+        compare(control.contentItem.width, control.availableWidth)
+        compare(control.contentItem.height, control.availableHeight)
     }
 
     function test_empty() {
@@ -78,6 +42,16 @@ TestCase {
         compare(control.implicitContentHeight, 0)
     }
 
+    Component {
+        id: oneChildPane
+        Pane {
+            Item {
+                implicitWidth: 100
+                implicitHeight: 30
+            }
+        }
+    }
+
     function test_oneChild() {
         var control = createTemporaryObject(oneChildPane, testCase)
         verify(control)
@@ -101,6 +75,20 @@ TestCase {
         verify(control.implicitHeight > 40)
     }
 
+    Component {
+        id: twoChildrenPane
+        Pane {
+            Item {
+                implicitWidth: 100
+                implicitHeight: 30
+            }
+            Item {
+                implicitWidth: 200
+                implicitHeight: 60
+            }
+        }
+    }
+
     function test_twoChildren() {
         var control = createTemporaryObject(twoChildrenPane, testCase)
         verify(control)
@@ -113,6 +101,16 @@ TestCase {
         verify(control.implicitHeight > 0)
     }
 
+    Component {
+        id: contentPane
+        Pane {
+            contentItem: Item {
+                implicitWidth: 100
+                implicitHeight: 30
+            }
+        }
+    }
+
     function test_contentItem() {
         var control = createTemporaryObject(contentPane, testCase)
         verify(control)
@@ -125,14 +123,45 @@ TestCase {
         verify(control.implicitHeight > 30)
     }
 
-    function test_implicitContentItem() {
-        var control = createTemporaryObject(pane, testCase, {width: 100, height: 100})
+    Component {
+        id: contentItemPane
+        Pane {
+            property string description: ""
+            contentItem: ColumnLayout {
+                Label {
+                    Layout.maximumWidth: 100
+                    text: description
+                    elide: Label.ElideRight
+                }
+            }
+            Component.onCompleted: {
+                description = "Binding loop issue ".repeat(100)
+            }
+        }
+    }
+
+    function test_paneBindingLoop() {
+        // Fails if there is any warning due to binding loop
+        failOnWarning(/.?/)
+        var control = createTemporaryObject(contentItemPane, testCase)
         verify(control)
+        // Wait for content item to be polished
+        waitForPolish(control.contentItem)
 
-        compare(control.width, 100)
-        compare(control.height, 100)
-        compare(control.contentItem.width, control.availableWidth)
-        compare(control.contentItem.height, control.availableHeight)
+        compare(control.contentWidth, 100)
+    }
+
+    Component {
+        id: pressPane
+        MouseArea {
+            width: 200
+            height: 200
+            property int pressCount
+            onPressed: ++pressCount
+            Pane {
+                anchors.fill: parent
+            }
+        }
     }
 
     function test_press() {
diff --git a/tests/auto/quickcontrols/controls/data/tst_scrollview.qml b/tests/auto/quickcontrols/controls/data/tst_scrollview.qml
index 5c9c6f1184..bef3fe2eec 100644
--- a/tests/auto/quickcontrols/controls/data/tst_scrollview.qml
+++ b/tests/auto/quickcontrols/controls/data/tst_scrollview.qml
@@ -673,6 +673,7 @@ TestCase {
 
             ListView {
                 id: listView
+                objectName: "customListView"
                 model: 20
                 delegate: Text {
                     text: modelData