From 7433cc2f91d1aba045ab4920d6732c85d2ee33b9 Mon Sep 17 00:00:00 2001
From: Mitch Curtis <mitch.curtis@qt.io>
Date: Thu, 29 Sep 2016 16:34:48 +0200
Subject: Fix crash when flicking a ListView-based Tumbler

This only happens in 5.7, and seemingly only when the delegate uses
attached properties (like displacement).

Other fixes required and/or relevant to the patch:
- Fixed some instances of displacementChanged possibly not being
emitted.
- Use QPointer for the attached object's Tumbler pointer, as it
was being accessed while null after this patch.

Change-Id: I7d881d815faf57f1a8bc0ea8e73a7331523d2f9b
Reviewed-by: J-P Nurmi <jpnurmi@qt.io>
---
 src/quicktemplates2/qquicktumbler.cpp    | 35 ++++++++++++++++++++++---
 tests/auto/controls/data/tst_tumbler.qml | 45 +++++++++++++++++++++++++++++++-
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/src/quicktemplates2/qquicktumbler.cpp b/src/quicktemplates2/qquicktumbler.cpp
index 580859ef..8c9c1d10 100644
--- a/src/quicktemplates2/qquicktumbler.cpp
+++ b/src/quicktemplates2/qquicktumbler.cpp
@@ -436,9 +436,10 @@ public:
     void itemChildRemoved(QQuickItem *, QQuickItem *) override;
 
     void _q_calculateDisplacement();
+    void emitIfDisplacementChanged(qreal oldDisplacement, qreal newDisplacement);
 
     // The Tumbler that contains the delegate. Required to calculated the displacement.
-    QQuickTumbler *tumbler;
+    QPointer<QQuickTumbler> tumbler;
     // The index of the delegate. Used to calculate the displacement.
     int index;
     // The displacement for our delegate.
@@ -476,14 +477,23 @@ void QQuickTumblerAttachedPrivate::_q_calculateDisplacement()
     const int previousDisplacement = displacement;
     displacement = 0;
 
+    if (!tumbler) {
+        emitIfDisplacementChanged(previousDisplacement, displacement);
+        return;
+    }
+
     const int count = tumbler->count();
     // This can happen in tests, so it may happen in normal usage too.
-    if (count == 0)
+    if (count == 0) {
+        emitIfDisplacementChanged(previousDisplacement, displacement);
         return;
+    }
 
     ContentItemType contentType = contentItemType(tumbler->contentItem());
-    if (contentType == UnsupportedContentItemType)
+    if (contentType == UnsupportedContentItemType) {
+        emitIfDisplacementChanged(previousDisplacement, displacement);
         return;
+    }
 
     qreal offset = 0;
 
@@ -507,8 +517,13 @@ void QQuickTumblerAttachedPrivate::_q_calculateDisplacement()
         displacement = reverseDisplacement - index;
     }
 
+    emitIfDisplacementChanged(previousDisplacement, displacement);
+}
+
+void QQuickTumblerAttachedPrivate::emitIfDisplacementChanged(qreal oldDisplacement, qreal newDisplacement)
+{
     Q_Q(QQuickTumblerAttached);
-    if (displacement != previousDisplacement)
+    if (newDisplacement != oldDisplacement)
         emit q->displacementChanged();
 }
 
@@ -532,6 +547,18 @@ QQuickTumblerAttached::QQuickTumblerAttached(QQuickItem *delegateItem) :
 
 QQuickTumblerAttached::~QQuickTumblerAttached()
 {
+    Q_D(QQuickTumblerAttached);
+    if (!d->tumbler || !d->tumbler->contentItem())
+        return;
+
+    QQuickItem *rootContentItem = d->tumbler->contentItem();
+    const ContentItemType contentType = contentItemType(rootContentItem);
+    QQuickItem *actualItem = actualContentItem(rootContentItem, contentType);
+    if (!actualItem)
+        return;
+
+    QQuickItemPrivate *p = QQuickItemPrivate::get(actualItem);
+    p->removeItemChangeListener(d, QQuickItemPrivate::Geometry | QQuickItemPrivate::Children);
 }
 
 /*!
diff --git a/tests/auto/controls/data/tst_tumbler.qml b/tests/auto/controls/data/tst_tumbler.qml
index 22cde065..401f7537 100644
--- a/tests/auto/controls/data/tst_tumbler.qml
+++ b/tests/auto/controls/data/tst_tumbler.qml
@@ -63,7 +63,8 @@ TestCase {
     }
 
     function cleanup() {
-        tumbler.destroy();
+        if (tumbler)
+            tumbler.destroy();
     }
 
     function tumblerXCenter() {
@@ -234,6 +235,48 @@ TestCase {
         tryCompare(tumbler.dayTumbler, "currentIndex", 27);
     }
 
+    Component {
+        id: timePickerComponent
+
+        Row {
+            property alias minuteTumbler: minuteTumbler
+            property alias amPmTumbler: amPmTumbler
+
+            Tumbler {
+                id: minuteTumbler
+                currentIndex: 6
+                model: 60
+                width: 50
+                height: 150
+            }
+
+            Tumbler {
+                id: amPmTumbler
+                model: ["AM", "PM"]
+                width: 50
+                height: 150
+                contentItem: ListView {
+                    anchors.fill: parent
+                    model: amPmTumbler.model
+                    delegate: amPmTumbler.delegate
+                }
+            }
+        }
+    }
+
+    function test_listViewTimePicker() {
+        tumbler.destroy();
+
+        var root = timePickerComponent.createObject(testCase);
+        verify(root);
+
+        mouseDrag(root.minuteTumbler, root.minuteTumbler.width / 2, root.minuteTumbler.height / 2, 0, 50);
+        // Shouldn't crash.
+        mouseDrag(root.amPmTumbler, root.amPmTumbler.width / 2, root.amPmTumbler.height / 2, 0, 50);
+
+        root.destroy();
+    }
+
     function test_displacement_data() {
         var data = [
             // At 0 offset, the first item is current.
-- 
cgit v1.2.3