Fix ScrollBar & ScrollIndicator for the new Gallery

The Gallery example exposed some new bugs. We must let the scroll
bar/indicator initialize its size before layouting it, or else it
will be posioned on the edge and eventually grows outside of the
flickable it is attached to. Therefore, we must monitor the size
changes of the scroll bar/indicator the same way we're monitoring
the size changes of the attached flickable. Furthermore, while
debugging the issue, I noticed that QQuickScrollBar/Indicator were
unnecessarily monitoring all geometry changes. Monitoring only size
changes is enough.

Change-Id: I2581dba29bb4606642ba470dce85534632d7752e
Reviewed-by: J-P Nurmi <jpnurmi@theqtcompany.com>

4 files changed, 34 insertions, 8 deletions

diff --git a/src/templates/qquickscrollbar.cpp b/src/templates/qquickscrollbar.cpp
index f6499004..d999d212 100644
--- a/src/templates/qquickscrollbar.cpp
+++ b/src/templates/qquickscrollbar.cpp
@@ -386,12 +386,11 @@ void QQuickScrollBarAttachedPrivate::itemGeometryChanged(QQuickItem *item, const
 {
     Q_UNUSED(item);
     Q_UNUSED(newGeometry);
-    Q_ASSERT(item == flickable);
-    if (horizontal) {
+    if (horizontal && horizontal->height() > 0) {
         bool move = qFuzzyIsNull(horizontal->y()) || qFuzzyCompare(horizontal->y(), oldGeometry.height() - horizontal->height());
         layoutHorizontal(move);
     }
-    if (vertical) {
+    if (vertical && vertical->width() > 0) {
         bool move = qFuzzyIsNull(vertical->x()) || qFuzzyCompare(vertical->x(), oldGeometry.width() - vertical->width());
         layoutVertical(move);
     }
@@ -402,7 +401,16 @@ QQuickScrollBarAttached::QQuickScrollBarAttached(QQuickFlickable *flickable) :
 {
     Q_D(QQuickScrollBarAttached);
     QQuickItemPrivate *p = QQuickItemPrivate::get(flickable);
-    p->addItemChangeListener(d, QQuickItemPrivate::Geometry);
+    p->updateOrAddGeometryChangeListener(d, QQuickItemPrivate::SizeChange);
+}
+
+QQuickScrollBarAttached::~QQuickScrollBarAttached()
+{
+    Q_D(QQuickScrollBarAttached);
+    if (d->horizontal)
+        QQuickItemPrivate::get(d->horizontal)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
+    if (d->vertical)
+        QQuickItemPrivate::get(d->vertical)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
 }
 
 /*!
@@ -428,6 +436,7 @@ void QQuickScrollBarAttached::setHorizontal(QQuickScrollBar *horizontal)
     Q_D(QQuickScrollBarAttached);
     if (d->horizontal != horizontal) {
         if (d->horizontal) {
+            QQuickItemPrivate::get(d->horizontal)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
             QObjectPrivate::disconnect(d->horizontal, &QQuickScrollBar::positionChanged, d, &QQuickScrollBarAttachedPrivate::scrollHorizontal);
             QObjectPrivate::disconnect(d->flickable, &QQuickFlickable::movingHorizontallyChanged, d, &QQuickScrollBarAttachedPrivate::activateHorizontal);
 
@@ -444,6 +453,7 @@ void QQuickScrollBarAttached::setHorizontal(QQuickScrollBar *horizontal)
                 horizontal->setParentItem(d->flickable);
             horizontal->setOrientation(Qt::Horizontal);
 
+            QQuickItemPrivate::get(horizontal)->updateOrAddGeometryChangeListener(d, QQuickItemPrivate::SizeChange);
             QObjectPrivate::connect(horizontal, &QQuickScrollBar::positionChanged, d, &QQuickScrollBarAttachedPrivate::scrollHorizontal);
             QObjectPrivate::connect(d->flickable, &QQuickFlickable::movingHorizontallyChanged, d, &QQuickScrollBarAttachedPrivate::activateHorizontal);
 
@@ -483,6 +493,7 @@ void QQuickScrollBarAttached::setVertical(QQuickScrollBar *vertical)
     Q_D(QQuickScrollBarAttached);
     if (d->vertical != vertical) {
         if (d->vertical) {
+            QQuickItemPrivate::get(d->vertical)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
             QObjectPrivate::disconnect(d->vertical, &QQuickScrollBar::positionChanged, d, &QQuickScrollBarAttachedPrivate::scrollVertical);
             QObjectPrivate::disconnect(d->flickable, &QQuickFlickable::movingVerticallyChanged, d, &QQuickScrollBarAttachedPrivate::activateVertical);
 
@@ -499,6 +510,7 @@ void QQuickScrollBarAttached::setVertical(QQuickScrollBar *vertical)
                 vertical->setParentItem(d->flickable);
             vertical->setOrientation(Qt::Vertical);
 
+            QQuickItemPrivate::get(vertical)->updateOrAddGeometryChangeListener(d, QQuickItemPrivate::SizeChange);
             QObjectPrivate::connect(vertical, &QQuickScrollBar::positionChanged, d, &QQuickScrollBarAttachedPrivate::scrollVertical);
             QObjectPrivate::connect(d->flickable, &QQuickFlickable::movingVerticallyChanged, d, &QQuickScrollBarAttachedPrivate::activateVertical);
 
diff --git a/src/templates/qquickscrollbar_p.h b/src/templates/qquickscrollbar_p.h
index 6171d26e..df0eb8ee 100644
--- a/src/templates/qquickscrollbar_p.h
+++ b/src/templates/qquickscrollbar_p.h
@@ -127,6 +127,7 @@ class Q_LABSTEMPLATES_EXPORT QQuickScrollBarAttached : public QObject
 
 public:
     explicit QQuickScrollBarAttached(QQuickFlickable *flickable);
+    ~QQuickScrollBarAttached();
 
     QQuickScrollBar *horizontal() const;
     void setHorizontal(QQuickScrollBar *horizontal);
diff --git a/src/templates/qquickscrollindicator.cpp b/src/templates/qquickscrollindicator.cpp
index 94b47e4d..b041e2ab 100644
--- a/src/templates/qquickscrollindicator.cpp
+++ b/src/templates/qquickscrollindicator.cpp
@@ -268,12 +268,11 @@ void QQuickScrollIndicatorAttachedPrivate::itemGeometryChanged(QQuickItem *item,
 {
     Q_UNUSED(item);
     Q_UNUSED(newGeometry);
-    Q_ASSERT(item == flickable);
-    if (horizontal) {
+    if (horizontal && horizontal->height() > 0) {
         bool move = qFuzzyIsNull(horizontal->y()) || qFuzzyCompare(horizontal->y(), oldGeometry.height() - horizontal->height());
         layoutHorizontal(move);
     }
-    if (vertical) {
+    if (vertical && vertical->width() > 0) {
         bool move = qFuzzyIsNull(vertical->x()) || qFuzzyCompare(vertical->x(), oldGeometry.width() - vertical->width());
         layoutVertical(move);
     }
@@ -284,7 +283,16 @@ QQuickScrollIndicatorAttached::QQuickScrollIndicatorAttached(QQuickFlickable *fl
 {
     Q_D(QQuickScrollIndicatorAttached);
     QQuickItemPrivate *p = QQuickItemPrivate::get(flickable);
-    p->addItemChangeListener(d, QQuickItemPrivate::Geometry);
+    p->updateOrAddGeometryChangeListener(d, QQuickItemPrivate::SizeChange);
+}
+
+QQuickScrollIndicatorAttached::~QQuickScrollIndicatorAttached()
+{
+    Q_D(QQuickScrollIndicatorAttached);
+    if (d->horizontal)
+        QQuickItemPrivate::get(d->horizontal)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
+    if (d->vertical)
+        QQuickItemPrivate::get(d->vertical)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
 }
 
 /*!
@@ -310,6 +318,7 @@ void QQuickScrollIndicatorAttached::setHorizontal(QQuickScrollIndicator *horizon
     Q_D(QQuickScrollIndicatorAttached);
     if (d->horizontal != horizontal) {
         if (d->horizontal) {
+            QQuickItemPrivate::get(d->horizontal)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
             QObjectPrivate::disconnect(d->flickable, &QQuickFlickable::movingHorizontallyChanged, d, &QQuickScrollIndicatorAttachedPrivate::activateHorizontal);
 
             // TODO: export QQuickFlickableVisibleArea
@@ -325,6 +334,7 @@ void QQuickScrollIndicatorAttached::setHorizontal(QQuickScrollIndicator *horizon
                 horizontal->setParentItem(d->flickable);
             horizontal->setOrientation(Qt::Horizontal);
 
+            QQuickItemPrivate::get(horizontal)->updateOrAddGeometryChangeListener(d, QQuickItemPrivate::SizeChange);
             QObjectPrivate::connect(d->flickable, &QQuickFlickable::movingHorizontallyChanged, d, &QQuickScrollIndicatorAttachedPrivate::activateHorizontal);
 
             // TODO: export QQuickFlickableVisibleArea
@@ -363,6 +373,7 @@ void QQuickScrollIndicatorAttached::setVertical(QQuickScrollIndicator *vertical)
     Q_D(QQuickScrollIndicatorAttached);
     if (d->vertical != vertical) {
         if (d->vertical) {
+            QQuickItemPrivate::get(d->vertical)->removeItemChangeListener(d, QQuickItemPrivate::Geometry);
             QObjectPrivate::disconnect(d->flickable, &QQuickFlickable::movingVerticallyChanged, d, &QQuickScrollIndicatorAttachedPrivate::activateVertical);
 
             // TODO: export QQuickFlickableVisibleArea
@@ -378,6 +389,7 @@ void QQuickScrollIndicatorAttached::setVertical(QQuickScrollIndicator *vertical)
                 vertical->setParentItem(d->flickable);
             vertical->setOrientation(Qt::Vertical);
 
+            QQuickItemPrivate::get(vertical)->updateOrAddGeometryChangeListener(d, QQuickItemPrivate::SizeChange);
             QObjectPrivate::connect(d->flickable, &QQuickFlickable::movingVerticallyChanged, d, &QQuickScrollIndicatorAttachedPrivate::activateVertical);
 
             // TODO: export QQuickFlickableVisibleArea
diff --git a/src/templates/qquickscrollindicator_p.h b/src/templates/qquickscrollindicator_p.h
index e67118b4..25fdc2fa 100644
--- a/src/templates/qquickscrollindicator_p.h
+++ b/src/templates/qquickscrollindicator_p.h
@@ -115,6 +115,7 @@ class Q_LABSTEMPLATES_EXPORT QQuickScrollIndicatorAttached : public QObject
 
 public:
     explicit QQuickScrollIndicatorAttached(QQuickFlickable *flickable);
+    ~QQuickScrollIndicatorAttached();
 
     QQuickScrollIndicator *horizontal() const;
     void setHorizontal(QQuickScrollIndicator *horizontal);