From af53fb0e00d694b7cb115b0162ee75d883adff67 Mon Sep 17 00:00:00 2001
From: Fabian Kosmale <fabian.kosmale@qt.io>
Date: Mon, 25 Jan 2021 15:11:53 +0100
Subject: [PATCH] QProperty: Treat change listener modifying its source
 property as a loop

This is in line with QML where

import QtQuick 2.15
Rectangle {
  width: 100
  height: 100
  color: "red"
  Rectangle {
    id:  inner
    x: 10
    y: x
    width: 50
    height: 50
    onYChanged: { console.log("hey"); inner.x = 10}
    TapHandler {
      onTapped: inner.x = 20
    }
  }
}

results in a binding loop warning when the tap handler triggers. While
the change handler would only run once, we cannot statically determine
if we need to loop once, twice, or if there actually is a diverging
loop. Thus we unconditionally warn about the binding loop and stop
executing the binding.

As a drive-by, verify in the related test that a change handler which
overwrites its properties binding itself removes the binding.

Change-Id: I5372019c2389ab724c49cd7489ecbd3ebced1c69
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
---
 src/corelib/kernel/qproperty.cpp                      | 6 +++---
 tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp | 4 +++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/corelib/kernel/qproperty.cpp b/src/corelib/kernel/qproperty.cpp
index 5ad0fabd068..fbe582304b9 100644
--- a/src/corelib/kernel/qproperty.cpp
+++ b/src/corelib/kernel/qproperty.cpp
@@ -100,15 +100,15 @@ void QPropertyBindingPrivate::unlinkAndDeref()
 
 void QPropertyBindingPrivate::markDirtyAndNotifyObservers()
 {
-    if (dirty)
-        return;
-    dirty = true;
     if (eagerlyUpdating) {
         error = QPropertyBindingError(QPropertyBindingError::BindingLoop);
         if (isQQmlPropertyBinding)
             errorCallBack(this);
         return;
     }
+    if (dirty)
+        return;
+    dirty = true;
 
     eagerlyUpdating = true;
     QScopeGuard guard([&](){eagerlyUpdating = false;});
diff --git a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
index 31ed610b099..630eee9e9cf 100644
--- a/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
+++ b/tests/auto/corelib/kernel/qproperty/tst_qproperty.cpp
@@ -619,7 +619,8 @@ void tst_QProperty::changePropertyFromWithinChangeHandlerThroughDependency()
 
     resetPropertyOnChange = true;
     sourceProperty = 42;
-    QCOMPARE(property.value(), 100);
+    QVERIFY(property.value() == 100 || property.value() == 42);
+    QVERIFY(property.binding().error().type() == QPropertyBindingError::BindingLoop);
     // changing the property value inside the change handler won't result in the change
     // handler being called again.
     QCOMPARE(changeHandlerCallCount, 1);
@@ -640,6 +641,7 @@ void tst_QProperty::changePropertyFromWithinChangeHandler2()
 
     property = 42;
     QCOMPARE(property.value(), 43);
+    QVERIFY(!property.hasBinding()); // setting the value in the change handler removed the binding
 }
 
 void tst_QProperty::settingPropertyValueDoesRemoveBinding()