QtStateMachine QML-facing properties' bindable support part 2

This commit covers these QML-facing classes:
SignalTransition

Task-number: QTBUG-91375
Change-Id: I58bca7b4599b420aa99492233f9d88ea4af7e877
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

7 files changed, 137 insertions, 24 deletions

diff --git a/src/statemachineqml/CMakeLists.txt b/src/statemachineqml/CMakeLists.txt
index cd3331f..a30d76a 100644
--- a/src/statemachineqml/CMakeLists.txt
+++ b/src/statemachineqml/CMakeLists.txt
@@ -14,6 +14,8 @@ qt_internal_add_module(StateMachineQml
         Qt::Core
         Qt::Qml
         Qt::StateMachine
+    LIBRARIES
+        Qt::QmlPrivate
 )
 
 set_target_properties(StateMachineQml PROPERTIES
diff --git a/src/statemachineqml/signaltransition.cpp b/src/statemachineqml/signaltransition.cpp
index 16c2e0c..f082b0a 100644
--- a/src/statemachineqml/signaltransition.cpp
+++ b/src/statemachineqml/signaltransition.cpp
@@ -54,7 +54,7 @@
 SignalTransition::SignalTransition(QState *parent)
     : QSignalTransition(this, SIGNAL(invokeYourself()), parent), m_complete(false), m_signalExpression(nullptr)
 {
-    connect(this, SIGNAL(signalChanged()), SIGNAL(qmlSignalChanged()));
+    connect(this, &SignalTransition::signalChanged, [this](){ m_signal.notify(); });
 }
 
 bool SignalTransition::eventTest(QEvent *event)
@@ -63,7 +63,7 @@ bool SignalTransition::eventTest(QEvent *event)
     if (!QSignalTransition::eventTest(event))
         return false;
 
-    if (m_guard.isEmpty())
+    if (m_guard.value().isEmpty())
         return true;
 
     QQmlContext *outerContext = QQmlEngine::contextForObject(this);
@@ -79,7 +79,7 @@ bool SignalTransition::eventTest(QEvent *event)
     for (int i = 0; i < count; i++)
         context.setContextProperty(QString::fromUtf8(parameterNames[i]), QVariant::fromValue(e->arguments().at(i)));
 
-    QQmlExpression expr(m_guard, &context, this);
+    QQmlExpression expr(m_guard.value(), &context, this);
     QVariant result = expr.evaluate();
 
     return result.toBool();
@@ -101,8 +101,10 @@ const QJSValue& SignalTransition::signal()
 
 void SignalTransition::setSignal(const QJSValue &signal)
 {
-    if (m_signal.strictlyEquals(signal))
+    if (m_signal.value().strictlyEquals(signal)) {
+        m_signal.removeBindingUnlessInWrapper();
         return;
+    }
 
     QV4::ExecutionEngine *jsEngine = QQmlEngine::contextForObject(this)->engine()->handle();
     QV4::Scope scope(jsEngine);
@@ -111,16 +113,15 @@ void SignalTransition::setSignal(const QJSValue &signal)
     QMetaMethod signalMethod;
 
     m_signal = signal;
-    QJSValuePrivate::manageStringOnV4Heap(jsEngine, &m_signal);
-
-    QV4::ScopedValue value(scope, QJSValuePrivate::asReturnedValue(&m_signal));
+    QV4::ScopedValue value(scope, QJSValuePrivate::asReturnedValue(&signal));
 
     // Did we get the "slot" that can be used to invoke the signal?
     if (QV4::QObjectMethod *signalSlot = value->as<QV4::QObjectMethod>()) {
         sender = signalSlot->object();
         Q_ASSERT(sender);
         signalMethod = sender->metaObject()->method(signalSlot->methodIndex());
-    } else if (QV4::QmlSignalHandler *signalObject = value->as<QV4::QmlSignalHandler>()) { // or did we get the signal object (the one with the connect()/disconnect() functions) ?
+    } else if (QV4::QmlSignalHandler *signalObject = value->as<QV4::QmlSignalHandler>()) {
+        // or did we get the signal object (the one with the connect()/disconnect() functions) ?
         sender = signalObject->object();
         Q_ASSERT(sender);
         signalMethod = sender->metaObject()->method(signalObject->signalIndex());
@@ -130,11 +131,17 @@ void SignalTransition::setSignal(const QJSValue &signal)
     }
 
     QSignalTransition::setSenderObject(sender);
+    // the call below will emit change signal, and the interceptor lambda in ctor will notify()
     QSignalTransition::setSignal(signalMethod.methodSignature());
 
     connectTriggered();
 }
 
+QBindable<QJSValue> SignalTransition::bindableSignal()
+{
+    return &m_signal;
+}
+
 QQmlScriptString SignalTransition::guard() const
 {
     return m_guard;
@@ -142,11 +149,12 @@ QQmlScriptString SignalTransition::guard() const
 
 void SignalTransition::setGuard(const QQmlScriptString &guard)
 {
-    if (m_guard == guard)
-        return;
-
     m_guard = guard;
-    emit guardChanged();
+}
+
+QBindable<QQmlScriptString> SignalTransition::bindableGuard()
+{
+    return &m_guard;
 }
 
 void SignalTransition::invoke()
@@ -169,7 +177,8 @@ void SignalTransition::connectTriggered()
 
     QV4::ExecutionEngine *jsEngine = QQmlEngine::contextForObject(this)->engine()->handle();
     QV4::Scope scope(jsEngine);
-    QV4::Scoped<QV4::QObjectMethod> qobjectSignal(scope, QJSValuePrivate::asReturnedValue(&m_signal));
+    QV4::Scoped<QV4::QObjectMethod> qobjectSignal(
+                scope, QJSValuePrivate::asReturnedValue(&m_signal.value()));
     Q_ASSERT(qobjectSignal);
     QMetaMethod metaMethod = target->metaObject()->method(qobjectSignal->methodIndex());
     int signalIndex = QMetaObjectPrivate::signalIndex(metaMethod);
diff --git a/src/statemachineqml/signaltransition_p.h b/src/statemachineqml/signaltransition_p.h
index b8b3346..03e3b1b 100644
--- a/src/statemachineqml/signaltransition_p.h
+++ b/src/statemachineqml/signaltransition_p.h
@@ -62,6 +62,7 @@
 #include <private/qqmlcustomparser_p.h>
 #include <private/qqmlrefcount_p.h>
 #include <private/qqmlboundsignal_p.h>
+#include <QtCore/private/qproperty_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -69,8 +70,10 @@ class Q_STATEMACHINEQML_PRIVATE_EXPORT SignalTransition : public QSignalTransiti
 {
     Q_OBJECT
     Q_INTERFACES(QQmlParserStatus)
-    Q_PROPERTY(QJSValue signal READ signal WRITE setSignal NOTIFY qmlSignalChanged)
-    Q_PROPERTY(QQmlScriptString guard READ guard WRITE setGuard NOTIFY guardChanged)
+    Q_PROPERTY(QJSValue signal READ signal WRITE setSignal
+               NOTIFY qmlSignalChanged BINDABLE bindableSignal)
+    Q_PROPERTY(QQmlScriptString guard READ guard WRITE setGuard
+               NOTIFY guardChanged BINDABLE bindableGuard)
     QML_ELEMENT
     QML_ADDED_IN_VERSION(1, 0)
     QML_CUSTOMPARSER
@@ -80,12 +83,14 @@ public:
 
     QQmlScriptString guard() const;
     void setGuard(const QQmlScriptString &guard);
+    QBindable<QQmlScriptString> bindableGuard();
 
     bool eventTest(QEvent *event) override;
     void onTransition(QEvent *event) override;
 
     const QJSValue &signal();
     void setSignal(const QJSValue &signal);
+    QBindable<QJSValue> bindableSignal();
 
     Q_INVOKABLE void invoke();
 
@@ -103,8 +108,11 @@ private:
     void connectTriggered();
 
     friend class SignalTransitionParser;
-    QJSValue m_signal;
-    QQmlScriptString m_guard;
+
+    Q_OBJECT_COMPAT_PROPERTY(SignalTransition, QJSValue, m_signal, &SignalTransition::setSignal,
+                             &SignalTransition::qmlSignalChanged);
+    Q_OBJECT_BINDABLE_PROPERTY(SignalTransition, QQmlScriptString,
+                               m_guard, &SignalTransition::guardChanged);
     bool m_complete;
     QQmlRefPointer<QV4::ExecutableCompilationUnit> m_compilationUnit;
     QList<const QV4::CompiledData::Binding *> m_bindings;
diff --git a/tests/auto/qml/qqmlstatemachine/CMakeLists.txt b/tests/auto/qml/qqmlstatemachine/CMakeLists.txt
index 5772a55..fd14ad7 100644
--- a/tests/auto/qml/qqmlstatemachine/CMakeLists.txt
+++ b/tests/auto/qml/qqmlstatemachine/CMakeLists.txt
@@ -9,11 +9,12 @@ qt_internal_add_test(tst_qqmlstatemachine
         tst_qqmlstatemachine.cpp
     INCLUDE_DIRECTORIES
         ../../shared
-    PUBLIC_LIBRARIES
+    LIBRARIES
         Qt::CorePrivate
         Qt::Gui
         Qt::GuiPrivate
         Qt::QmlPrivate
+        Qt::StateMachineQmlPrivate
 )
 
 ## Scopes:
@@ -28,3 +29,10 @@ qt_extend_target(tst_qqmlstatemachine CONDITION NOT ANDROID AND NOT IOS
     DEFINES
         QT_QMLTEST_DATADIR=\\\"${CMAKE_CURRENT_SOURCE_DIR}/data\\\"
 )
+
+qt_internal_add_resource(tst_qqmlstatemachine "tst_qqmlstatemachine"
+    PREFIX
+        "/"
+    FILES
+        "data/signaltransition.qml"
+)
diff --git a/tests/auto/qml/qqmlstatemachine/data/signaltransition.qml b/tests/auto/qml/qqmlstatemachine/data/signaltransition.qml
new file mode 100644
index 0000000..52c62a7
--- /dev/null
+++ b/tests/auto/qml/qqmlstatemachine/data/signaltransition.qml
@@ -0,0 +1,51 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 The Qt Company
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+import QtQuick
+import QtQml.StateMachine
+
+Item {
+    id: root
+
+    signal signal1()
+    signal signal2()
+
+    function getSignal1() { return root.signal1 }
+    function getSignal2() { return root.signal2 }
+
+    SignalTransition {
+        objectName: "st1"
+        guard: 1 + 1
+    }
+
+    SignalTransition {
+        objectName: "st2"
+        guard: 2 + 2
+    }
+
+}
diff --git a/tests/auto/qml/qqmlstatemachine/tst_qqmlstatemachine.cpp b/tests/auto/qml/qqmlstatemachine/tst_qqmlstatemachine.cpp
index a6f0d65..80d75a8 100644
--- a/tests/auto/qml/qqmlstatemachine/tst_qqmlstatemachine.cpp
+++ b/tests/auto/qml/qqmlstatemachine/tst_qqmlstatemachine.cpp
@@ -28,8 +28,13 @@
 #include <QQmlComponent>
 #include <QQmlContext>
 #include <QQmlEngine>
+#include <QtQuick/QQuickItem>
+#include <QtStateMachineQml/private/signaltransition_p.h>
+#include <QtQml/qqmlscriptstring.h>
+
 #include <QTest>
 #include "../../shared/util.h"
+#include "../../shared/bindableutils.h"
 
 class tst_qqmlstatemachine : public QQmlDataTest
 {
@@ -39,6 +44,7 @@ public:
 
 private slots:
     void tst_cppObjectSignal();
+    void tst_bindings();
 };
 
 
@@ -104,6 +110,33 @@ void tst_qqmlstatemachine::tst_cppObjectSignal()
     QTRY_VERIFY(!rootObject->property("running").toBool());
 }
 
+void tst_qqmlstatemachine::tst_bindings()
+{
+    // -- SignalTransition::guard
+    SignalTransition signalTransition;
+    // Generating QQmlScriptString requires proper qml context setup, and here we
+    // use same the element that we are testing to create the testing material
+    QQmlEngine engine;
+    QQmlComponent component(&engine, QUrl(QLatin1String("qrc:///data/signaltransition.qml")));
+
+    std::unique_ptr<QObject> obj(component.create());
+    SignalTransition *st1 = qobject_cast<SignalTransition*>(obj->findChild<QObject*>("st1"));
+    SignalTransition *st2 = qobject_cast<SignalTransition*>(obj->findChild<QObject*>("st2"));
+    QVERIFY(st1 && st2 && (st1->guard() != st2->guard()));
+    testWritableBindableBasics<SignalTransition, QQmlScriptString>(
+        signalTransition, st1->guard(), st2->guard(), "guard");
+
+    // -- SignalTransition::signal
+    // We use QML to create the test material (QJSValues that contain valid methods)
+    QVariant signal1;
+    QVariant signal2;
+    QMetaObject::invokeMethod(obj.get(), "getSignal1", Q_RETURN_ARG(QVariant, signal1));
+    QMetaObject::invokeMethod(obj.get(), "getSignal2", Q_RETURN_ARG(QVariant, signal2));
+    // QJSValue does not implement operator== so we supply own comparator
+    testWritableBindableBasics<SignalTransition, QJSValue>(
+                *st1, signal1.value<QJSValue>(), signal2.value<QJSValue>(), "signal",
+                [](QJSValue d1, QJSValue d2) { return d1.strictlyEquals(d2); });
+}
 
 QTEST_MAIN(tst_qqmlstatemachine)
 
diff --git a/tests/auto/shared/bindableutils.h b/tests/auto/shared/bindableutils.h
index 3e93752..6e9f657 100644
--- a/tests/auto/shared/bindableutils.h
+++ b/tests/auto/shared/bindableutils.h
@@ -46,7 +46,8 @@
 // "propertyName" is the name of the property we are interested in testing
 template<typename TestedClass, typename TestedData>
 void testWritableBindableBasics(TestedClass& testedClass, TestedData data1,
-                        TestedData data2, const char* propertyName)
+                        TestedData data2, const char* propertyName,
+                        std::function<bool(TestedData,TestedData)> dataComparator = [](TestedData d1, TestedData d2) { return d1 == d2; })
 {
     // Get the property we are testing
     const QMetaObject *metaObject = testedClass.metaObject();
@@ -66,7 +67,8 @@ void testWritableBindableBasics(TestedClass& testedClass, TestedData data1,
 
     // Test basic property read and write
     testedClass.setProperty(propertyName, QVariant::fromValue(data1));
-    QVERIFY2(testedClass.property(propertyName).template value<TestedData>() == data1, qPrintable(id));
+
+    QVERIFY2(dataComparator(testedClass.property(propertyName).template value<TestedData>(), data1), qPrintable(id));
     QVERIFY2(spy.count() == 1, qPrintable(id + ", actual: " + QString::number(spy.count())));
 
     // Test setting a binding as a source for the property
@@ -76,25 +78,25 @@ void testWritableBindableBasics(TestedClass& testedClass, TestedData data1,
     bindable.setBinding(Qt::makePropertyBinding(property2));
     QVERIFY2(bindable.hasBinding(), qPrintable(id));
     // Check that the value also changed
-    QVERIFY2(testedClass.property(propertyName).template value<TestedData>() == data2, qPrintable(id));
+    QVERIFY2(dataComparator(testedClass.property(propertyName).template value<TestedData>(), data2), qPrintable(id));
     QVERIFY2(spy.count() == 2, qPrintable(id + ", actual: " + QString::number(spy.count())));
     // Same test but with a lambda binding (cast to be able to set the lambda directly)
     QBindable<TestedData> *typedBindable = static_cast<QBindable<TestedData>*>(&bindable);
     typedBindable->setBinding([&](){ return property1.value(); });
     QVERIFY2(typedBindable->hasBinding(), qPrintable(id));
-    QVERIFY2(testedClass.property(propertyName).template value<TestedData>() == data1, qPrintable(id));
+    QVERIFY2(dataComparator(testedClass.property(propertyName).template value<TestedData>(), data1), qPrintable(id));
     QVERIFY2(spy.count() == 3, qPrintable(id + ", actual: " + QString::number(spy.count())));
 
     // Remove binding by setting a value directly
     QVERIFY2(bindable.hasBinding(), qPrintable(id));
     testedClass.setProperty(propertyName, QVariant::fromValue(data2));
-    QVERIFY2(testedClass.property(propertyName).template value<TestedData>() == data2, qPrintable(id));
+    QVERIFY2(dataComparator(testedClass.property(propertyName).template value<TestedData>(), data2), qPrintable(id));
     QVERIFY2(!bindable.hasBinding(), qPrintable(id));
     QVERIFY2(spy.count() == 4, qPrintable(id + ", actual: " + QString::number(spy.count())));
 
     // Test using the property as the source in a binding
     QProperty<bool> data1Used([&](){
-        return testedClass.property(propertyName).template value<TestedData>() == data1;
+        return dataComparator(testedClass.property(propertyName).template value<TestedData>(), data1);
     });
     QVERIFY2(data1Used == false, qPrintable(id));
     testedClass.setProperty(propertyName, QVariant::fromValue(data1));