From 0942e5cd6f1e4c8eac45a77c5deece671a6f513b Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Fri, 27 Jan 2017 09:45:22 +0100 Subject: QQuickScrollBarAttached: fix change listener removal Before bb2a6c6 QQuickScrollBarAttached called flickable->updateOrAddGeometryChangeListener(Size) when attaching to a Flickable, and flickable->removeItemChangeListener(Geometry) when detaching from a Flickable. In bb2a6c6 the latter was changed to flickable->updateOrRemoveGeometryChangeListener(listener, Size) to make the attach and detach operations nice and symmetric. Now the problem is that updateOrRemoveGeometryChangeListener(Size) doesn't actually remove the listener, but just resets the geometry types it listens to. Thus, upon destruction of QQuickScrollBarAttached, it leaves behind a dangling pointer in Flickable's list of change listeners. We can call either of these to fix the problem: flickable->updateOrRemoveGeometryChangeListener(Nothing) flickable->removeItemChangeListener(Geometry) The former does essentially the latter with some extra overhead, so we'll just revert back to how it was before bb2a6c6. I added a warning note, also to ScrollIndicator since it's using the same approach, to avoid the same pitfall in the future. Change-Id: Ibdce15b22edf549491426d769b74b18daf0500ca Reviewed-by: Mitch Curtis --- src/quicktemplates2/qquickscrollbar.cpp | 5 ++++- src/quicktemplates2/qquickscrollindicator.cpp | 3 +++ 2 files changed, 7 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/quicktemplates2/qquickscrollbar.cpp b/src/quicktemplates2/qquickscrollbar.cpp index f0dd640b..bb29ebd6 100644 --- a/src/quicktemplates2/qquickscrollbar.cpp +++ b/src/quicktemplates2/qquickscrollbar.cpp @@ -630,7 +630,10 @@ QQuickScrollBarAttachedPrivate::QQuickScrollBarAttachedPrivate() void QQuickScrollBarAttachedPrivate::setFlickable(QQuickFlickable *item) { if (flickable) { - QQuickItemPrivate::get(flickable)->updateOrRemoveGeometryChangeListener(this, QQuickItemPrivate::Size); + // NOTE: Use removeItemChangeListener(Geometry) instead of updateOrRemoveGeometryChangeListener(Size). + // The latter doesn't remove the listener but only resets its types. Thus, it leaves behind a dangling + // pointer on destruction. + QQuickItemPrivate::get(flickable)->removeItemChangeListener(this, QQuickItemPrivate::Geometry); if (horizontal) cleanupHorizontal(); if (vertical) diff --git a/src/quicktemplates2/qquickscrollindicator.cpp b/src/quicktemplates2/qquickscrollindicator.cpp index c98a1956..a57cfbb6 100644 --- a/src/quicktemplates2/qquickscrollindicator.cpp +++ b/src/quicktemplates2/qquickscrollindicator.cpp @@ -412,6 +412,9 @@ QQuickScrollIndicatorAttached::~QQuickScrollIndicatorAttached() QQuickItemPrivate::get(d->horizontal)->removeItemChangeListener(d, horizontalChangeTypes); if (d->vertical) QQuickItemPrivate::get(d->vertical)->removeItemChangeListener(d,verticalChangeTypes); + // NOTE: Use removeItemChangeListener(Geometry) instead of updateOrRemoveGeometryChangeListener(Size). + // The latter doesn't remove the listener but only resets its types. Thus, it leaves behind a dangling + // pointer on destruction. QQuickItemPrivate::get(d->flickable)->removeItemChangeListener(d, QQuickItemPrivate::Geometry); } } -- cgit v1.2.3