From 5b681bea902c9b9556ea8325df17338e78039d20 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Mon, 10 May 2021 15:50:12 +0200
Subject: [PATCH] QPropertyBinding: Do not reevaluate if not installed on
 property

Since we changed binding evaluation to be always eager, we notify and
evaluate all bindings as soon as any dependency changes. This includes
bindings which have been initially installed on a property, but which
were later removed.
With lazy evaluation, we would only notify those bindings and mark them
as dirty, which is unproblematic. With eager evalution, we attempt to
evaluate the binding, though. While that part is still fine, afterwards
we would attempt to write the new value into the property. However,
there is no property at that point, as the binding is not installed.
Instead of adding a check whether the propertydataptr is null, we skip
the reevaluation completely by removing the bindings observers - and
thus the cause for the binding function's reevaluation. As soon as the
binding is set, we reevaluate the function anyway, at which point we
also capture the observers again.

Task-number: QTBUG-89505
Change-Id: Ie1885ccd8be519fb96f6fde658275810b54f445a
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
---
 src/corelib/kernel/qproperty.cpp              |  1 +
 .../kernel/qproperty/tst_qproperty.cpp        | 27 +++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index d8176a845a..ace90fa672 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -273,6 +273,7 @@ QPropertyBindingPrivate::~QPropertyBindingPrivate()
 
 void QPropertyBindingPrivate::unlinkAndDeref()
 {
+    clearDependencyObservers();
     propertyDataPtr = nullptr;
     if (--ref == 0)
         destroyAndFreeMemory(this);
diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
index 7a214acbc4..4195459af5 100644
--- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
+++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
@@ -107,6 +107,7 @@ private slots:
     void noDoubleNotification();
     void groupedNotifications();
     void groupedNotificationConsistency();
+    void uninstalledBindingDoesNotEvaluate();
 };
 
 void tst_QProperty::functorBinding()
@@ -1747,6 +1748,32 @@ void tst_QProperty::groupedNotificationConsistency()
     QVERIFY(areEqual); // value changed runs after everything has been evaluated
 }
 
+void tst_QProperty::uninstalledBindingDoesNotEvaluate()
+{
+    QProperty<int> i;
+    QProperty<int> j;
+    int bindingEvaluationCounter = 0;
+    i.setBinding([&](){
+        bindingEvaluationCounter++;
+        return j.value();
+    });
+    QCOMPARE(bindingEvaluationCounter, 1);
+    // Sanity check: if we force a binding reevaluation,
+    j = 42;
+    // the binding function will be called again.
+    QCOMPARE(bindingEvaluationCounter, 2);
+    // If we keep referencing the binding
+    auto keptBinding = i.binding();
+    // but have it not installed on a property
+    i = 10;
+    QVERIFY(!i.hasBinding());
+    QVERIFY(!keptBinding.isNull());
+    // then changing a dependency
+    j = 12;
+    // does not lead to the binding being reevaluated.
+    QCOMPARE(bindingEvaluationCounter, 2);
+}
+
 QTEST_MAIN(tst_QProperty);
 
 #undef QT_SOURCE_LOCATION_NAMESPACE