ScrollView: fix default Flickable content item creation

Short version: do not emit contentItemChanged() while contentItem()
is being called to avoid binding loops.

Long version: ScrollView is a thin wrapper around Flickable, and
when necessary, instantiates a Flickable behind the scenes. However,
ScrollView cannot instantiate one unconditionally, because it has to
work with existing Flickables, such as ListView and GridView, too.

  ScrollView { Item { } } // creates a Flickable
  ScrollView { Flickable { } } // does not create a Flickable
  ScrollView { contentItem: Flickable { } } // does not create a Flickable

When a Flickable is created behind the scenes, it is assigned as
the contentItem of the ScrollView. However, when the Flickable is
created lazily as a result of ScrollView::contentItem being accessed,
it must NOT emit contentItemChanged() while the contentItem() getter
is being called, because that results to a binding loop. This problem
was exposed by the recent attempts to enable QML caching at build time
(QTBUG-58571).

Change-Id: I712f2f30da454a6c22a722afe8a00ae240733571
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

3 files changed, 33 insertions, 21 deletions

diff --git a/src/quicktemplates2/qquickcontrol.cpp b/src/quicktemplates2/qquickcontrol.cpp
index c039253d..a2b16a10 100644
--- a/src/quicktemplates2/qquickcontrol.cpp
+++ b/src/quicktemplates2/qquickcontrol.cpp
@@ -281,6 +281,27 @@ QQuickItem *QQuickControlPrivate::getContentItem()
     return contentItem;
 }
 
+void QQuickControlPrivate::setContentItem_helper(QQuickItem *item, bool notify)
+{
+    Q_Q(QQuickControl);
+    if (contentItem == item)
+        return;
+
+    q->contentItemChange(item, contentItem);
+    destroyDelegate(contentItem, q);
+    contentItem = item;
+
+    if (item) {
+        if (!item->parentItem())
+            item->setParentItem(q);
+        if (componentComplete)
+            resizeContent();
+    }
+
+    if (notify)
+        emit q->contentItemChanged();
+}
+
 #if QT_CONFIG(accessibility)
 void QQuickControlPrivate::accessibilityActiveChanged(bool active)
 {
@@ -1243,25 +1264,15 @@ void QQuickControl::setBackground(QQuickItem *background)
 QQuickItem *QQuickControl::contentItem() const
 {
     QQuickControlPrivate *d = const_cast<QQuickControlPrivate *>(d_func());
-    return d->getContentItem();
+    if (!d->contentItem)
+        d->setContentItem_helper(d->getContentItem(), false);
+    return d->contentItem;
 }
 
 void QQuickControl::setContentItem(QQuickItem *item)
 {
     Q_D(QQuickControl);
-    if (d->contentItem == item)
-        return;
-
-    contentItemChange(item, d->contentItem);
-    QQuickControlPrivate::destroyDelegate(d->contentItem, this);
-    d->contentItem = item;
-    if (item) {
-        if (!item->parentItem())
-            item->setParentItem(this);
-        if (isComponentComplete())
-            d->resizeContent();
-    }
-    emit contentItemChanged();
+    d->setContentItem_helper(item, true);
 }
 
 void QQuickControl::classBegin()
diff --git a/src/quicktemplates2/qquickcontrol_p_p.h b/src/quicktemplates2/qquickcontrol_p_p.h
index 4dd6f32a..42924875 100644
--- a/src/quicktemplates2/qquickcontrol_p_p.h
+++ b/src/quicktemplates2/qquickcontrol_p_p.h
@@ -95,6 +95,7 @@ public:
     virtual void resizeContent();
 
     virtual QQuickItem *getContentItem();
+    void setContentItem_helper(QQuickItem *item, bool notify = true);
 
 #if QT_CONFIG(accessibility)
     void accessibilityActiveChanged(bool active) override;
diff --git a/src/quicktemplates2/qquickscrollview.cpp b/src/quicktemplates2/qquickscrollview.cpp
index fc763511..c05e4848 100644
--- a/src/quicktemplates2/qquickscrollview.cpp
+++ b/src/quicktemplates2/qquickscrollview.cpp
@@ -108,7 +108,7 @@ public:
 
     QQuickItem *getContentItem() override;
 
-    QQuickFlickable *ensureFlickable();
+    QQuickFlickable *ensureFlickable(bool content);
     bool setFlickable(QQuickFlickable *flickable, bool content);
 
     void updateContentWidth();
@@ -146,14 +146,14 @@ QQuickScrollViewPrivate::QQuickScrollViewPrivate()
 
 QQuickItem *QQuickScrollViewPrivate::getContentItem()
 {
-    return ensureFlickable();
+    return ensureFlickable(false);
 }
 
-QQuickFlickable *QQuickScrollViewPrivate::ensureFlickable()
+QQuickFlickable *QQuickScrollViewPrivate::ensureFlickable(bool content)
 {
     Q_Q(QQuickScrollView);
     if (!flickable)
-        setFlickable(new QQuickFlickable(q), true);
+        setFlickable(new QQuickFlickable(q), content);
     return flickable;
 }
 
@@ -271,7 +271,7 @@ void QQuickScrollViewPrivate::contentData_append(QQmlListProperty<QObject> *prop
     if (!p->flickable && p->setFlickable(qobject_cast<QQuickFlickable *>(obj), true))
         return;
 
-    QQuickFlickable *flickable = p->ensureFlickable();
+    QQuickFlickable *flickable = p->ensureFlickable(true);
     Q_ASSERT(flickable);
     QQmlListProperty<QObject> data = flickable->flickableData();
     data.append(&data, obj);
@@ -313,7 +313,7 @@ void QQuickScrollViewPrivate::contentChildren_append(QQmlListProperty<QQuickItem
     if (!p->flickable)
         p->setFlickable(qobject_cast<QQuickFlickable *>(item), true);
 
-    QQuickFlickable *flickable = p->ensureFlickable();
+    QQuickFlickable *flickable = p->ensureFlickable(true);
     Q_ASSERT(flickable);
     QQmlListProperty<QQuickItem> children = flickable->flickableChildren();
     children.append(&children, item);
@@ -552,7 +552,7 @@ void QQuickScrollView::componentComplete()
     Q_D(QQuickScrollView);
     QQuickControl::componentComplete();
     if (!d->contentItem)
-        d->ensureFlickable();
+        d->ensureFlickable(true);
 }
 
 void QQuickScrollView::contentItemChange(QQuickItem *newItem, QQuickItem *oldItem)