QProperty: Handle eager binding calling setBinding

When an eager binding  triggers a setBinding call, we end up with a
special kind of binding loop:
setBinding() -> evaluate -> notifyObserver
      ^                           |
      |                           /
      ----------------------------
We now catch set condition, and set the binding status to BindingLoop
(with a distinct description).

Task-number: QTBUG-87153
Task-number: QTBUG-87733
Pick-to: 6.0
Change-Id: I9f9915797d82eab820fc279baceaf89d7e5a3f4a
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
This commit is contained in:
Fabian Kosmale 2020-12-08 16:03:36 +01:00
parent 55245c769b
commit ddc585b7c7
3 changed files with 25 additions and 5 deletions

View File

@ -250,6 +250,10 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
if (auto *existingBinding = d.bindingPtr()) { if (auto *existingBinding = d.bindingPtr()) {
if (existingBinding == newBinding.data()) if (existingBinding == newBinding.data())
return QUntypedPropertyBinding(static_cast<QPropertyBindingPrivate *>(oldBinding.data())); return QUntypedPropertyBinding(static_cast<QPropertyBindingPrivate *>(oldBinding.data()));
if (existingBinding->isEagerlyUpdating()) {
existingBinding->setError({QPropertyBindingError::BindingLoop, QStringLiteral("Binding set during binding evaluation!")});
return QUntypedPropertyBinding(static_cast<QPropertyBindingPrivate *>(oldBinding.data()));
}
oldBinding = QPropertyBindingPrivatePtr(existingBinding); oldBinding = QPropertyBindingPrivatePtr(existingBinding);
observer = static_cast<QPropertyBindingPrivate *>(oldBinding.data())->takeObservers(); observer = static_cast<QPropertyBindingPrivate *>(oldBinding.data())->takeObservers();
static_cast<QPropertyBindingPrivate *>(oldBinding.data())->unlinkAndDeref(); static_cast<QPropertyBindingPrivate *>(oldBinding.data())->unlinkAndDeref();
@ -269,9 +273,11 @@ QUntypedPropertyBinding QPropertyBindingData::setBinding(const QUntypedPropertyB
newBindingRaw->prependObserver(observer); newBindingRaw->prependObserver(observer);
newBindingRaw->setStaticObserver(staticObserverCallback, guardCallback); newBindingRaw->setStaticObserver(staticObserverCallback, guardCallback);
if (newBindingRaw->requiresEagerEvaluation()) { if (newBindingRaw->requiresEagerEvaluation()) {
newBindingRaw->setEagerlyUpdating(true);
auto changed = newBindingRaw->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr); auto changed = newBindingRaw->evaluateIfDirtyAndReturnTrueIfValueChanged(propertyDataPtr);
if (changed) if (changed)
observer.notify(newBindingRaw, propertyDataPtr, /*alreadyKnownToHaveChanged=*/true); observer.notify(newBindingRaw, propertyDataPtr, /*alreadyKnownToHaveChanged=*/true);
newBindingRaw->setEagerlyUpdating(false);
} }
} else if (observer) { } else if (observer) {
d.setObservers(observer.ptr); d.setObservers(observer.ptr);

View File

@ -226,6 +226,9 @@ public:
// public because the auto-tests access it, too. // public because the auto-tests access it, too.
size_t dependencyObserverCount = 0; size_t dependencyObserverCount = 0;
bool isEagerlyUpdating() {return eagerlyUpdating;}
void setEagerlyUpdating(bool b) {eagerlyUpdating = b;}
QPropertyBindingPrivate(QMetaType metaType, const QtPrivate::BindingFunctionVTable *vtable, QPropertyBindingPrivate(QMetaType metaType, const QtPrivate::BindingFunctionVTable *vtable,
const QPropertyBindingSourceLocation &location, bool isQQmlPropertyBinding=false) const QPropertyBindingSourceLocation &location, bool isQQmlPropertyBinding=false)
: hasBindingWrapper(false) : hasBindingWrapper(false)

View File

@ -462,6 +462,7 @@ class BindingLoopTester : public QObject
eagerData2.setBinding(Qt::makePropertyBinding([&](){ return eagerData.value() + 1; } ) ); eagerData2.setBinding(Qt::makePropertyBinding([&](){ return eagerData.value() + 1; } ) );
i->setValue(42); i->setValue(42);
} }
BindingLoopTester() {}
int eagerProp() {return eagerData.value();} int eagerProp() {return eagerData.value();}
void setEagerProp(int i) { eagerData = i; } void setEagerProp(int i) { eagerData = i; }
@ -495,11 +496,21 @@ void tst_QProperty::bindingLoop()
QCOMPARE(secondProp.binding().error().type(), QPropertyBindingError::BindingLoop); QCOMPARE(secondProp.binding().error().type(), QPropertyBindingError::BindingLoop);
QProperty<int> i; {
BindingLoopTester tester(&i); QProperty<int> i;
QCOMPARE(tester.bindableEagerProp().binding().error().type(), QPropertyBindingError::BindingLoop); BindingLoopTester tester(&i);
QEXPECT_FAIL("", "Only the first property in a dependency cycle is set to the error state", Continue); QCOMPARE(tester.bindableEagerProp().binding().error().type(), QPropertyBindingError::BindingLoop);
QCOMPARE(tester.bindableEagerProp2().binding().error().type(), QPropertyBindingError::BindingLoop); QCOMPARE(tester.bindableEagerProp2().binding().error().type(), QPropertyBindingError::BindingLoop);
}
{
BindingLoopTester tester;
auto handler = tester.bindableEagerProp().onValueChanged([&]() {
tester.bindableEagerProp().setBinding([](){return 42;});
});
tester.bindableEagerProp().setBinding([]() {return 42;});
QCOMPARE(tester.bindableEagerProp().binding().error().type(), QPropertyBindingError::BindingLoop);
QCOMPARE(tester.bindableEagerProp().binding().error().description(), "Binding set during binding evaluation!");
}
} }
class ReallocTester : public QObject class ReallocTester : public QObject