Fix proxy-data handling

This addresses two different issues:
- Firstly, we were casting the resolved binding data pointer to
  QPropertyProxyBindingData, instead of the d_ptr of
  QPropertyBindingData. Fix this by introducing a helper function,
  and consistently using it to access the proxy data.
- Secondly, we were not resetting the originalBindingData when the
  pointed to object was destoyed. Fix that, too.

Pick-to: 6.5 6.4 6.2
Task-number: QTBUG-110899
Change-Id: I7691c9df5cc26e761f6b0e5f16d152f7f2183208
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Fabian Kosmale 2023-02-03 14:35:55 +01:00
parent efce30bb43
commit 55ca636180
4 changed files with 57 additions and 4 deletions

View File

@ -446,6 +446,8 @@ QMetaType QUntypedPropertyBinding::valueMetaType() const
QPropertyBindingData::~QPropertyBindingData() QPropertyBindingData::~QPropertyBindingData()
{ {
QPropertyBindingDataPointer d{this}; QPropertyBindingDataPointer d{this};
if (isNotificationDelayed())
proxyData()->originalBindingData = nullptr;
for (auto observer = d.firstObserver(); observer;) { for (auto observer = d.firstObserver(); observer;) {
auto next = observer.nextObserver(); auto next = observer.nextObserver();
observer.unlink(); observer.unlink();

View File

@ -83,6 +83,7 @@ struct QPropertyBindingDataPointer
void Q_ALWAYS_INLINE addObserver(QPropertyObserver *observer); void Q_ALWAYS_INLINE addObserver(QPropertyObserver *observer);
inline void setFirstObserver(QPropertyObserver *observer); inline void setFirstObserver(QPropertyObserver *observer);
inline QPropertyObserverPointer firstObserver() const; inline QPropertyObserverPointer firstObserver() const;
static QPropertyProxyBindingData *proxyData(QtPrivate::QPropertyBindingData *ptr);
inline int observerCount() const; inline int observerCount() const;
@ -418,9 +419,9 @@ inline void QPropertyBindingDataPointer::fixupAfterMove(QtPrivate::QPropertyBind
{ {
auto &d = ptr->d_ref(); auto &d = ptr->d_ref();
if (ptr->isNotificationDelayed()) { if (ptr->isNotificationDelayed()) {
QPropertyProxyBindingData *proxyData QPropertyProxyBindingData *proxy = ptr->proxyData();
= reinterpret_cast<QPropertyProxyBindingData*>(d & ~QtPrivate::QPropertyBindingData::BindingBit); Q_ASSERT(proxy);
proxyData->originalBindingData = ptr; proxy->originalBindingData = ptr;
} }
// If QPropertyBindingData has been moved, and it has an observer // If QPropertyBindingData has been moved, and it has an observer
// we have to adjust the firstObserver's prev pointer to point to // we have to adjust the firstObserver's prev pointer to point to
@ -438,6 +439,17 @@ inline QPropertyObserverPointer QPropertyBindingDataPointer::firstObserver() con
return { reinterpret_cast<QPropertyObserver *>(ptr->d()) }; return { reinterpret_cast<QPropertyObserver *>(ptr->d()) };
} }
/*!
\internal
Returns the proxy data of \a ptr, or \c nullptr if \a ptr has no delayed notification
*/
inline QPropertyProxyBindingData *QPropertyBindingDataPointer::proxyData(QtPrivate::QPropertyBindingData *ptr)
{
if (!ptr->isNotificationDelayed())
return nullptr;
return ptr->proxyData();
}
inline int QPropertyBindingDataPointer::observerCount() const inline int QPropertyBindingDataPointer::observerCount() const
{ {
int count = 0; int count = 0;

View File

@ -303,10 +303,15 @@ private:
{ {
quintptr &d = d_ptr; quintptr &d = d_ptr;
if (isNotificationDelayed()) if (isNotificationDelayed())
return reinterpret_cast<QPropertyProxyBindingData *>(d_ptr & ~(BindingBit|DelayedNotificationBit))->d_ptr; return proxyData()->d_ptr;
return d; return d;
} }
quintptr d() const { return d_ref(); } quintptr d() const { return d_ref(); }
QPropertyProxyBindingData *proxyData() const
{
Q_ASSERT(isNotificationDelayed());
return reinterpret_cast<QPropertyProxyBindingData *>(d_ptr & ~(BindingBit|DelayedNotificationBit));
}
void registerWithCurrentlyEvaluatingBinding_helper(BindingEvaluationState *currentBinding) const; void registerWithCurrentlyEvaluatingBinding_helper(BindingEvaluationState *currentBinding) const;
void removeBinding_helper(); void removeBinding_helper();

View File

@ -6,6 +6,7 @@
#include <qtest.h> #include <qtest.h>
#include <qproperty.h> #include <qproperty.h>
#include <private/qproperty_p.h> #include <private/qproperty_p.h>
#include <private/qobject_p.h>
#if __has_include(<source_location>) && __cplusplus >= 202002L && !defined(Q_QDOC) #if __has_include(<source_location>) && __cplusplus >= 202002L && !defined(Q_QDOC)
#include <source_location> #include <source_location>
@ -87,6 +88,7 @@ private slots:
void noDoubleNotification(); void noDoubleNotification();
void groupedNotifications(); void groupedNotifications();
void groupedNotificationConsistency(); void groupedNotificationConsistency();
void bindingGroupMovingBindingData();
void uninstalledBindingDoesNotEvaluate(); void uninstalledBindingDoesNotEvaluate();
void notify(); void notify();
@ -2132,6 +2134,38 @@ void tst_QProperty::groupedNotificationConsistency()
QVERIFY(areEqual); // value changed runs after everything has been evaluated QVERIFY(areEqual); // value changed runs after everything has been evaluated
} }
void tst_QProperty::bindingGroupMovingBindingData()
{
auto tester = std::make_unique<ClassWithNotifiedProperty>();
auto testerPriv = QObjectPrivate::get(tester.get());
auto dummyNotifier = tester->property.addNotifier([](){});
auto bindingData = testerPriv->bindingStorage.bindingData(&tester->property);
QVERIFY(bindingData); // we have a notifier, so there should be binding data
Qt::beginPropertyUpdateGroup();
auto cleanup = qScopeGuard([](){ Qt::endPropertyUpdateGroup(); });
tester->property = 42;
QCOMPARE(testerPriv->bindingStorage.bindingData(&tester->property), bindingData);
auto proxyData = QPropertyBindingDataPointer::proxyData(bindingData);
// as we've modified the property, we now should have a proxy for the delayed notification
QVERIFY(proxyData);
// trigger binding data reallocation
std::array<QUntypedPropertyData, 10> propertyDataArray;
for (auto&& data: propertyDataArray)
testerPriv->bindingStorage.bindingData(&data, true);
// binding data has moved
QVERIFY(testerPriv->bindingStorage.bindingData(&tester->property) != bindingData);
bindingData = testerPriv->bindingStorage.bindingData(&tester->property);
// the proxy data has been updated
QCOMPARE(proxyData->originalBindingData, bindingData);
tester.reset();
// the property data is gone, proxyData should have been informed
QCOMPARE(proxyData->originalBindingData, nullptr);
QVERIFY(proxyData);
}
void tst_QProperty::uninstalledBindingDoesNotEvaluate() void tst_QProperty::uninstalledBindingDoesNotEvaluate()
{ {
QProperty<int> i; QProperty<int> i;