From cbd02e02bb8ed5373bfd856fcc7f6965ab5cd17d Mon Sep 17 00:00:00 2001
From: Ivan Solovev <ivan.solovev@qt.io>
Date: Thu, 5 Oct 2023 11:47:23 +0200
Subject: Fix QScxmlStateMachineLoader::source binding loop

The QScxmlStateMachineLoader::source setter assumes that the loader is
managed by a QML engine, so use a helper QML file and a QQmlComponent
to create a helper object and run the test for binding loops.

The binding loop here is a bit tricky, as the source setter modifies
two properties: source and stateMachine.
The fix for the source property is trivial.

The stateMachine property is read-only, so modify the
setStateMachine() internal setter in such way that it does not create
bindings. After that, manually call notify() if the state machine
has changed.

Use simpler state machines in the test, because the original ones
were generating the ASAN memleak warnings from scxml compiler.

Task-number: QTBUG-116542
Pick-to: 6.5
Change-Id: Ia8427ed07c1e63e5740b2f0817fe6b2cb48726b8
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
(cherry picked from commit 2d14e9a88497b1aad3b75304d5dfa7d37900e9c3)
Reviewed-by: Qt Cherry-pick Bot <cherrypick_bot@qt-project.org>
---
 src/scxmlqml/statemachineloader.cpp            | 22 ++++++++++++++--------
 tests/auto/qml/scxmlqmlcpp/data/smlHelper.qml  |  6 ++++++
 tests/auto/qml/scxmlqmlcpp/tst_scxmlqmlcpp.cpp | 12 +++++++++---
 3 files changed, 29 insertions(+), 11 deletions(-)
 create mode 100644 tests/auto/qml/scxmlqmlcpp/data/smlHelper.qml

diff --git a/src/scxmlqml/statemachineloader.cpp b/src/scxmlqml/statemachineloader.cpp
index c67b175..33e3172 100644
--- a/src/scxmlqml/statemachineloader.cpp
+++ b/src/scxmlqml/statemachineloader.cpp
@@ -38,9 +38,9 @@ QT_PREPEND_NAMESPACE(QScxmlStateMachine) *QScxmlStateMachineLoader::stateMachine
 
 void QScxmlStateMachineLoader::setStateMachine(QScxmlStateMachine* stateMachine)
 {
-    if (m_stateMachine.value() != stateMachine) {
-        delete m_stateMachine.value();
-        m_stateMachine = stateMachine;
+    if (m_stateMachine.valueBypassingBindings() != stateMachine) {
+        delete m_stateMachine.valueBypassingBindings();
+        m_stateMachine.setValueBypassingBindings(stateMachine);
     }
 }
 
@@ -65,17 +65,23 @@ void QScxmlStateMachineLoader::setSource(const QUrl &source)
     if (!source.isValid())
         return;
 
-    const QUrl oldSource = m_source;
+    m_source.removeBindingUnlessInWrapper();
+
+    const QUrl oldSource = m_source.valueBypassingBindings();
+    const auto *oldStateMachine = m_stateMachine.valueBypassingBindings();
     setStateMachine(nullptr);
     m_implicitDataModel = nullptr;
 
     if (parse(source))
-        m_source = source;
+        m_source.setValueBypassingBindings(source);
     else
-        m_source = QUrl();
+        m_source.setValueBypassingBindings(QUrl());
 
-    if (oldSource != m_source)
+    if (oldSource != m_source.valueBypassingBindings())
         m_source.notify();
+
+    if (oldStateMachine != m_stateMachine.valueBypassingBindings())
+        m_stateMachine.notify();
 }
 
 QBindable<QUrl> QScxmlStateMachineLoader::bindableSource()
@@ -177,7 +183,7 @@ bool QScxmlStateMachineLoader::parse(const QUrl &source)
         setStateMachine(stateMachine);
         // as this is deferred any pending property updates to m_dataModel and m_initialValues
         // should still occur before start().
-        QMetaObject::invokeMethod(m_stateMachine.value(), "start", Qt::QueuedConnection);
+        QMetaObject::invokeMethod(m_stateMachine.valueBypassingBindings(), "start", Qt::QueuedConnection);
         return true;
     } else {
         qmlWarning(this) << QStringLiteral("Something went wrong while parsing '%1':")
diff --git a/tests/auto/qml/scxmlqmlcpp/data/smlHelper.qml b/tests/auto/qml/scxmlqmlcpp/data/smlHelper.qml
new file mode 100644
index 0000000..8deec6e
--- /dev/null
+++ b/tests/auto/qml/scxmlqmlcpp/data/smlHelper.qml
@@ -0,0 +1,6 @@
+import QtQuick
+import QtScxml
+
+StateMachineLoader {
+    objectName: 'helper'
+}
diff --git a/tests/auto/qml/scxmlqmlcpp/tst_scxmlqmlcpp.cpp b/tests/auto/qml/scxmlqmlcpp/tst_scxmlqmlcpp.cpp
index c3c3501..5d10fa5 100644
--- a/tests/auto/qml/scxmlqmlcpp/tst_scxmlqmlcpp.cpp
+++ b/tests/auto/qml/scxmlqmlcpp/tst_scxmlqmlcpp.cpp
@@ -112,8 +112,8 @@ void tst_scxmlqmlcpp::stateMachineLoaderSourceStateMachineBinding()
 {
     // Test source and stateMachine together as they interact with each other
 
-    QUrl source1(testFileUrl("statemachine.scxml"));
-    QUrl source2(testFileUrl("topmachine.scxml"));
+    QUrl source1(testFileUrl("submachineA.scxml"));
+    QUrl source2(testFileUrl("submachineB.scxml"));
     // The 'setSource' of the statemachineloader assumes a valid qml context
     QQmlEngine engine;
     const QUrl smlUrl = testFileUrl("stateMachineLoader.qml");
@@ -124,9 +124,15 @@ void tst_scxmlqmlcpp::stateMachineLoaderSourceStateMachineBinding()
             qobject_cast<QScxmlStateMachineLoader*>(root->findChild<QObject*>("sml"));
     QVERIFY(sml != nullptr);
 
+    QQmlComponent otherComponent(&engine, testFileUrl("smlHelper.qml"));
+
     // -- StateMachineLoader::source
     QTestPrivate::testReadWritePropertyBasics<QScxmlStateMachineLoader, QUrl>(
-                *sml, source1, source2, "source");
+                *sml, source1, source2, "source",
+                [&otherComponent]() {
+                    return std::unique_ptr<QScxmlStateMachineLoader>(
+                            qobject_cast<QScxmlStateMachineLoader*>(otherComponent.create()));
+                });
     if (QTest::currentTestFailed()) {
         qWarning() << "QScxmlStateMachineLoader::source property testing failed";
         return;
-- 
cgit v1.2.3