QQuickTabBar: fix implicit size calculation

Before, the implicit size of TabBar was calculated based on the content
size of the ListView that TabBar uses internally. The problem was that
ListView calculates the content size based on the explicit size of the
items. There was a circular dependency, because TabBar resized the tabs
to the size of the view. To avoid the circular dependency, TabBar now
calculates the content size based on the total implicit size of the
tabs. As before, explicit size is respected for tabs that have it set.

[ChangeLog][Controls][TabBar] Added contentWidth and contentHeight
properties that are automatically calculated based on the total size
of the tab items, but can be manually overridden if desired. This fixes
an issue that TabBar was not able to reliably calculate an implicit
size, and could in certain scenarios enter an infinite loop due to a
circular dependency between the items' sizes and the tabbar's size.

Task-number: QTBUG-57858
Change-Id: Ie303cbc54247e87b0affc6bf32c7bf99acea4571
Reviewed-by: Mitch Curtis <mitch.curtis@qt.io>

7 files changed, 201 insertions, 39 deletions

diff --git a/src/imports/controls/TabBar.qml b/src/imports/controls/TabBar.qml
index e40017f6..53e18c40 100644
--- a/src/imports/controls/TabBar.qml
+++ b/src/imports/controls/TabBar.qml
@@ -41,16 +41,14 @@ T.TabBar {
     id: control
 
     implicitWidth: Math.max(background ? background.implicitWidth : 0,
-                            contentItem.implicitWidth + leftPadding + rightPadding)
+                            contentWidth + leftPadding + rightPadding)
     implicitHeight: Math.max(background ? background.implicitHeight : 0,
-                             contentItem.implicitHeight + topPadding + bottomPadding)
+                             contentHeight + topPadding + bottomPadding)
 
     spacing: 1
+    contentHeight: 40
 
     contentItem: ListView {
-        implicitWidth: contentWidth
-        implicitHeight: 40
-
         model: control.contentModel
         currentIndex: control.currentIndex
 
diff --git a/src/imports/controls/material/TabBar.qml b/src/imports/controls/material/TabBar.qml
index 39660c92..3d1e2d82 100644
--- a/src/imports/controls/material/TabBar.qml
+++ b/src/imports/controls/material/TabBar.qml
@@ -43,16 +43,14 @@ T.TabBar {
     id: control
 
     implicitWidth: Math.max(background ? background.implicitWidth : 0,
-                            contentItem.implicitWidth + leftPadding + rightPadding)
+                            contentWidth + leftPadding + rightPadding)
     implicitHeight: Math.max(background ? background.implicitHeight : 0,
-                             contentItem.implicitHeight + topPadding + bottomPadding)
+                             contentHeight + topPadding + bottomPadding)
 
     spacing: 1
+    contentHeight: 48
 
     contentItem: ListView {
-        implicitWidth: contentWidth
-        implicitHeight: 48
-
         model: control.contentModel
         currentIndex: control.currentIndex
 
diff --git a/src/imports/controls/universal/TabBar.qml b/src/imports/controls/universal/TabBar.qml
index 5a95cf61..083934aa 100644
--- a/src/imports/controls/universal/TabBar.qml
+++ b/src/imports/controls/universal/TabBar.qml
@@ -42,14 +42,13 @@ T.TabBar {
     id: control
 
     implicitWidth: Math.max(background ? background.implicitWidth : 0,
-                            contentItem.implicitWidth + leftPadding + rightPadding)
+                            contentWidth + leftPadding + rightPadding)
     implicitHeight: Math.max(background ? background.implicitHeight : 0,
-                             contentItem.implicitHeight + topPadding + bottomPadding)
+                             contentHeight + topPadding + bottomPadding)
 
-    contentItem: PathView {
-        implicitWidth: 200
-        implicitHeight: 48
+    contentHeight: 48
 
+    contentItem: PathView {
         model: control.contentModel
         currentIndex: control.currentIndex
 
diff --git a/src/imports/templates/qtquicktemplates2plugin.cpp b/src/imports/templates/qtquicktemplates2plugin.cpp
index d7f6cf18..1f00a99f 100644
--- a/src/imports/templates/qtquicktemplates2plugin.cpp
+++ b/src/imports/templates/qtquicktemplates2plugin.cpp
@@ -233,6 +233,7 @@ void QtQuickTemplates2Plugin::registerTypes(const char *uri)
     qmlRegisterType<QQuickSlider, 2>(uri, 2, 2, "Slider");
     qmlRegisterType<QQuickSpinBox, 2>(uri, 2, 2, "SpinBox");
     qmlRegisterType<QQuickSwipeDelegate, 2>(uri, 2, 2, "SwipeDelegate");
+    qmlRegisterType<QQuickTabBar, 2>(uri, 2, 2, "TabBar");
     qmlRegisterType<QQuickTumbler, 2>(uri, 2, 2, "Tumbler");
 }
 
diff --git a/src/quicktemplates2/qquicktabbar.cpp b/src/quicktemplates2/qquicktabbar.cpp
index 9aead901..96e12f6a 100644
--- a/src/quicktemplates2/qquicktabbar.cpp
+++ b/src/quicktemplates2/qquicktabbar.cpp
@@ -105,14 +105,26 @@ public:
     void updateLayout();
 
     void itemGeometryChanged(QQuickItem *item, QQuickGeometryChange change, const QRectF &diff) override;
+    void itemImplicitWidthChanged(QQuickItem *item) override;
+    void itemImplicitHeightChanged(QQuickItem *item) override;
 
     bool updatingLayout;
+    bool hasContentWidth;
+    bool hasContentHeight;
+    qreal contentWidth;
+    qreal contentHeight;
     QQuickTabBar::Position position;
 };
 
-QQuickTabBarPrivate::QQuickTabBarPrivate() : updatingLayout(false), position(QQuickTabBar::Header)
+QQuickTabBarPrivate::QQuickTabBarPrivate()
+    : updatingLayout(false),
+      hasContentWidth(false),
+      hasContentHeight(false),
+      contentWidth(0),
+      contentHeight(0),
+      position(QQuickTabBar::Header)
 {
-    changeTypes |= Geometry;
+    changeTypes |= Geometry | ImplicitWidth | ImplicitHeight;
 }
 
 void QQuickTabBarPrivate::updateCurrentItem()
@@ -134,34 +146,61 @@ void QQuickTabBarPrivate::updateLayout()
 {
     Q_Q(QQuickTabBar);
     const int count = contentModel->count();
-    if (count > 0 && contentItem) {
-        qreal reservedWidth = 0;
-        QVector<QQuickItem *> resizableItems;
-        resizableItems.reserve(count);
-
-        for (int i = 0; i < count; ++i) {
-            QQuickItem *item = q->itemAt(i);
-            if (item) {
-                QQuickItemPrivate *p = QQuickItemPrivate::get(item);
-                if (!p->widthValid)
-                    resizableItems += item;
-                else
-                    reservedWidth += item->width();
+    if (count <= 0 || !contentItem)
+        return;
+
+    qreal maxHeight = 0;
+    qreal totalWidth = 0;
+    qreal reservedWidth = 0;
+
+    QVector<QQuickItem *> resizableItems;
+    resizableItems.reserve(count);
+
+    for (int i = 0; i < count; ++i) {
+        QQuickItem *item = q->itemAt(i);
+        if (item) {
+            QQuickItemPrivate *p = QQuickItemPrivate::get(item);
+            if (!p->widthValid) {
+                resizableItems += item;
+                totalWidth += item->implicitWidth();
+            } else {
+                reservedWidth += item->width();
+                totalWidth += item->width();
             }
+            maxHeight = qMax(maxHeight, item->implicitHeight());
         }
+    }
 
-        if (!resizableItems.isEmpty()) {
-            const qreal totalSpacing = qMax(0, count - 1) * spacing;
-            const qreal itemWidth = (contentItem->width() - reservedWidth - totalSpacing) / resizableItems.count();
+    const qreal totalSpacing = qMax(0, count - 1) * spacing;
+    totalWidth += totalSpacing;
 
-            updatingLayout = true;
-            for (QQuickItem *item : qAsConst(resizableItems)) {
-                item->setWidth(itemWidth);
-                QQuickItemPrivate::get(item)->widthValid = false;
-            }
-            updatingLayout = false;
+    if (!resizableItems.isEmpty()) {
+        const qreal itemWidth = (contentItem->width() - reservedWidth - totalSpacing) / resizableItems.count();
+
+        updatingLayout = true;
+        for (QQuickItem *item : qAsConst(resizableItems)) {
+            item->setWidth(itemWidth);
+            QQuickItemPrivate::get(item)->widthValid = false;
         }
+        updatingLayout = false;
+    }
+
+    bool contentWidthChange = false;
+    if (!hasContentWidth && !qFuzzyCompare(contentWidth, totalWidth)) {
+        contentWidth = totalWidth;
+        contentWidthChange = true;
+    }
+
+    bool contentHeightChange = false;
+    if (!hasContentHeight && !qFuzzyCompare(contentHeight, maxHeight)) {
+        contentHeight = maxHeight;
+        contentHeightChange = true;
     }
+
+    if (contentWidthChange)
+        emit q->contentWidthChanged();
+    if (contentHeightChange)
+        emit q->contentHeightChanged();
 }
 
 void QQuickTabBarPrivate::itemGeometryChanged(QQuickItem *, QQuickGeometryChange, const QRectF &)
@@ -170,6 +209,18 @@ void QQuickTabBarPrivate::itemGeometryChanged(QQuickItem *, QQuickGeometryChange
         updateLayout();
 }
 
+void QQuickTabBarPrivate::itemImplicitWidthChanged(QQuickItem *)
+{
+    if (!updatingLayout && !hasContentWidth)
+        updateLayout();
+}
+
+void QQuickTabBarPrivate::itemImplicitHeightChanged(QQuickItem *)
+{
+    if (!updatingLayout && !hasContentHeight)
+        updateLayout();
+}
+
 QQuickTabBar::QQuickTabBar(QQuickItem *parent) :
     QQuickContainer(*(new QQuickTabBarPrivate), parent)
 {
@@ -210,6 +261,87 @@ void QQuickTabBar::setPosition(Position position)
     emit positionChanged();
 }
 
+/*!
+    \since QtQuick.Controls 2.2
+    \qmlproperty real QtQuick.Controls::TabBar::contentWidth
+
+    This property holds the content width. It is used for calculating the total
+    implicit width of the tab bar.
+
+    Unless explicitly overridden, the content width is automatically calculated
+    based on the total implicit width of the tabs and the \l {Control::}{spacing}
+    of the tab bar.
+
+    \sa contentHeight
+*/
+qreal QQuickTabBar::contentWidth() const
+{
+    Q_D(const QQuickTabBar);
+    return d->contentWidth;
+}
+
+void QQuickTabBar::setContentWidth(qreal width)
+{
+    Q_D(QQuickTabBar);
+    d->hasContentWidth = true;
+    if (qFuzzyCompare(d->contentWidth, width))
+        return;
+
+    d->contentWidth = width;
+    emit contentWidthChanged();
+}
+
+void QQuickTabBar::resetContentWidth()
+{
+    Q_D(QQuickTabBar);
+    if (!d->hasContentWidth)
+        return;
+
+    d->hasContentWidth = false;
+    if (isComponentComplete())
+        d->updateLayout();
+}
+
+/*!
+    \since QtQuick.Controls 2.2
+    \qmlproperty real QtQuick.Controls::TabBar::contentHeight
+
+    This property holds the content height. It is used for calculating the total
+    implicit height of the tab bar.
+
+    Unless explicitly overridden, the content height is automatically calculated
+    based on the maximum implicit height of the tabs.
+
+    \sa contentWidth
+*/
+qreal QQuickTabBar::contentHeight() const
+{
+    Q_D(const QQuickTabBar);
+    return d->contentHeight;
+}
+
+void QQuickTabBar::setContentHeight(qreal height)
+{
+    Q_D(QQuickTabBar);
+    d->hasContentHeight = true;
+    if (qFuzzyCompare(d->contentHeight, height))
+        return;
+
+    d->contentHeight = height;
+    emit contentHeightChanged();
+}
+
+void QQuickTabBar::resetContentHeight()
+{
+    Q_D(QQuickTabBar);
+    if (!d->hasContentHeight)
+        return;
+
+    d->hasContentHeight = false;
+    if (isComponentComplete())
+        d->updateLayout();
+}
+
 void QQuickTabBar::updatePolish()
 {
     Q_D(QQuickTabBar);
diff --git a/src/quicktemplates2/qquicktabbar_p.h b/src/quicktemplates2/qquicktabbar_p.h
index 7e590009..2fec756b 100644
--- a/src/quicktemplates2/qquicktabbar_p.h
+++ b/src/quicktemplates2/qquicktabbar_p.h
@@ -58,6 +58,8 @@ class Q_QUICKTEMPLATES2_PRIVATE_EXPORT QQuickTabBar : public QQuickContainer
 {
     Q_OBJECT
     Q_PROPERTY(Position position READ position WRITE setPosition NOTIFY positionChanged FINAL)
+    Q_PROPERTY(qreal contentWidth READ contentWidth WRITE setContentWidth RESET resetContentWidth NOTIFY contentWidthChanged FINAL REVISION 2)
+    Q_PROPERTY(qreal contentHeight READ contentHeight WRITE setContentHeight RESET resetContentHeight NOTIFY contentHeightChanged FINAL REVISION 2)
 
 public:
     explicit QQuickTabBar(QQuickItem *parent = nullptr);
@@ -71,8 +73,18 @@ public:
     Position position() const;
     void setPosition(Position position);
 
+    qreal contentWidth() const;
+    void setContentWidth(qreal width);
+    void resetContentWidth();
+
+    qreal contentHeight() const;
+    void setContentHeight(qreal height);
+    void resetContentHeight();
+
 Q_SIGNALS:
     void positionChanged();
+    void contentWidthChanged();
+    void contentHeightChanged();
 
 protected:
     void updatePolish() override;
diff --git a/tests/auto/controls/data/tst_tabbar.qml b/tests/auto/controls/data/tst_tabbar.qml
index 22c7a083..cf5987c8 100644
--- a/tests/auto/controls/data/tst_tabbar.qml
+++ b/tests/auto/controls/data/tst_tabbar.qml
@@ -496,25 +496,47 @@ TestCase {
     function test_layout(data) {
         var control = createTemporaryObject(tabBar, testCase, {spacing: data.spacing, width: 200})
 
+        // remove the implicit size from the background so that it won't affect
+        // the implicit size of the tabbar, so the implicit sizes tested below
+        // are entirely based on the content size
+        control.background.implicitWidth = 0
+
         var tab1 = tabButton.createObject(control, {text: "First"})
         control.addItem(tab1)
         tryCompare(tab1, "width", control.width)
+        compare(control.contentWidth, tab1.implicitWidth)
+        compare(control.implicitWidth, control.contentWidth + control.leftPadding + control.rightPadding)
 
         var tab2 = tabButton.createObject(control, {text: "Second"})
         control.addItem(tab2)
         tryCompare(tab1, "width", (control.width - data.spacing) / 2)
         compare(tab2.width, (control.width - data.spacing) / 2)
+        compare(control.contentWidth, tab1.implicitWidth + tab2.implicitWidth + data.spacing)
+        compare(control.implicitWidth, control.contentWidth + control.leftPadding + control.rightPadding)
 
         var tab3 = tabButton.createObject(control, {width: 50, text: "Third"})
         control.addItem(tab3)
         tryCompare(tab1, "width", (control.width - 2 * data.spacing - 50) / 2)
         compare(tab2.width, (control.width - 2 * data.spacing - 50) / 2)
         compare(tab3.width, 50)
+        compare(control.contentWidth, tab1.implicitWidth + tab2.implicitWidth + tab3.width + 2 * data.spacing)
+        compare(control.implicitWidth, control.contentWidth + control.leftPadding + control.rightPadding)
 
         var expectedWidth = tab3.contentItem.implicitWidth + tab3.leftPadding + tab3.rightPadding
         tab3.width = tab3.implicitWidth
         tryCompare(tab1, "width", (control.width - 2 * data.spacing - expectedWidth) / 2)
         tryCompare(tab2, "width", (control.width - 2 * data.spacing - expectedWidth) / 2)
         compare(tab3.width, expectedWidth)
+        compare(control.contentWidth, tab1.implicitWidth + tab2.implicitWidth + tab3.implicitWidth + 2 * data.spacing)
+        compare(control.implicitWidth, control.contentWidth + control.leftPadding + control.rightPadding)
+
+        tab3.width = undefined
+        control.width = undefined
+
+        control.contentWidth = 300
+        expectedWidth = (control.contentWidth - 2 * data.spacing) / 3
+        tryCompare(tab1, "width", expectedWidth)
+        tryCompare(tab2, "width", expectedWidth)
+        tryCompare(tab3, "width", expectedWidth)
     }
 }