Implement support for property observers

Syntactically we call them signal handler expressions :-), now also
working when the underlying property doesn't emit an old-style signal
but is just a QProperty.

Change-Id: I719a3e428f44af0fd48036434aefa682a02f7de1
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

8 files changed, 75 insertions, 26 deletions

diff --git a/src/qml/common/qv4compileddata_p.h b/src/qml/common/qv4compileddata_p.h
index 7d6ef14854..f0c4c2aa3e 100644
--- a/src/qml/common/qv4compileddata_p.h
+++ b/src/qml/common/qv4compileddata_p.h
@@ -449,7 +449,8 @@ struct Binding
         IsBindingToAlias = 0x40,
         IsDeferredBinding = 0x80,
         IsCustomParserBinding = 0x100,
-        IsFunctionExpression = 0x200
+        IsFunctionExpression = 0x200,
+        IsPropertyObserver = 0x400
     };
 
     union {
@@ -475,7 +476,8 @@ struct Binding
             || type == Type_GroupProperty)
             return false;
         if (flags & IsSignalHandlerExpression
-            || flags & IsSignalHandlerObject)
+            || flags & IsSignalHandlerObject
+            || flags & IsPropertyObserver)
             return false;
         return true;
     }
@@ -485,7 +487,7 @@ struct Binding
 
     bool isSignalHandler() const
     {
-        if (flags & IsSignalHandlerExpression || flags & IsSignalHandlerObject) {
+        if (flags & IsSignalHandlerExpression || flags & IsSignalHandlerObject || flags & IsPropertyObserver) {
             Q_ASSERT(!isValueBinding());
             Q_ASSERT(!isAttachedProperty());
             Q_ASSERT(!isGroupProperty());
diff --git a/src/qml/qml/qqmlboundsignal.cpp b/src/qml/qml/qqmlboundsignal.cpp
index 6272979e92..d47b4aa5a2 100644
--- a/src/qml/qml/qqmlboundsignal.cpp
+++ b/src/qml/qml/qqmlboundsignal.cpp
@@ -408,4 +408,13 @@ QQmlBoundSignalExpressionPointer &QQmlBoundSignalExpressionPointer::take(QQmlBou
     return *this;
 }
 
+QQmlPropertyObserver::QQmlPropertyObserver(QQmlBoundSignalExpression *expr)
+    : QPropertyObserver([](QPropertyObserver *self) {
+                           auto This = static_cast<QQmlPropertyObserver*>(self);
+                           This->expression->evaluate(QList<QVariant>());
+                       })
+{
+    expression.take(expr);
+}
+
 QT_END_NAMESPACE
diff --git a/src/qml/qml/qqmlboundsignal_p.h b/src/qml/qml/qqmlboundsignal_p.h
index 4a0fa6d4e7..71a3e90b7f 100644
--- a/src/qml/qml/qqmlboundsignal_p.h
+++ b/src/qml/qml/qqmlboundsignal_p.h
@@ -126,6 +126,15 @@ private:
     QQmlBoundSignalExpressionPointer m_expression;
 };
 
+class QQmlPropertyObserver : public QPropertyObserver
+{
+public:
+    QQmlPropertyObserver(QQmlBoundSignalExpression *expr);
+
+private:
+    QQmlBoundSignalExpressionPointer expression;
+};
+
 QT_END_NAMESPACE
 
 #endif // QQMLBOUNDSIGNAL_P_H
diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index 1c238767d8..f850746d92 100644
--- a/src/qml/qml/qqmldata_p.h
+++ b/src/qml/qml/qqmldata_p.h
@@ -74,6 +74,7 @@ class QQmlContextData;
 class QQmlNotifier;
 class QQmlDataExtended;
 class QQmlNotifierEndpoint;
+class QQmlPropertyObserver;
 
 namespace QV4 {
 class ExecutableCompilationUnit;
@@ -175,6 +176,7 @@ public:
 
     QQmlAbstractBinding *bindings;
     QQmlBoundSignal *signalHandlers;
+    std::vector<QQmlPropertyObserver> propertyObservers;
 
     // Linked list for QQmlContext::contextObjects
     QQmlData *nextContextObject;
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index 564835ff8b..c8564ba294 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -909,15 +909,22 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper
         QQmlPropertyPrivate::removeBinding(_bindingTarget, QQmlPropertyIndex(bindingProperty->coreIndex()));
 
     if (binding->type == QV4::CompiledData::Binding::Type_Script || binding->isTranslationBinding()) {
-        if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression) {
+        if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression
+            || binding->flags & QV4::CompiledData::Binding::IsPropertyObserver) {
             QV4::Function *runtimeFunction = compilationUnit->runtimeFunctions[binding->value.compiledScriptIndex];
             int signalIndex = _propertyCache->methodIndexToSignalIndex(bindingProperty->coreIndex());
-            QQmlBoundSignal *bs = new QQmlBoundSignal(_bindingTarget, signalIndex, _scopeObject, engine);
             QQmlBoundSignalExpression *expr = new QQmlBoundSignalExpression(
                         _bindingTarget, signalIndex, context,
                         _scopeObject, runtimeFunction, currentQmlContext());
 
-            bs->takeExpression(expr);
+            if (bindingProperty->isQProperty()) {
+                auto &observer = QQmlData::get(_scopeObject)->propertyObservers.emplace_back(expr);
+                void *argv[] = { &observer };
+                _bindingTarget->qt_metacall(QMetaObject::RegisterQPropertyObserver, bindingProperty->coreIndex(), argv);
+            } else {
+                QQmlBoundSignal *bs = new QQmlBoundSignal(_bindingTarget, signalIndex, _scopeObject, engine);
+                bs->takeExpression(expr);
+            }
         } else if (bindingProperty->isQProperty()) {
             QUntypedPropertyBinding qmlBinding;
             if (binding->isTranslationBinding()) {
diff --git a/src/qml/qml/qqmltypecompiler.cpp b/src/qml/qml/qqmltypecompiler.cpp
index 957f64e367..65b9213954 100644
--- a/src/qml/qml/qqmltypecompiler.cpp
+++ b/src/qml/qml/qqmltypecompiler.cpp
@@ -332,45 +332,55 @@ bool SignalHandlerConverter::convertSignalHandlerExpressionsToFunctionDeclaratio
     QHash<QString, QStringList> customSignals;
 
     for (QmlIR::Binding *binding = obj->firstBinding(); binding; binding = binding->next) {
-        QString propertyName = stringAt(binding->propertyNameIndex);
+        const QString bindingPropertyName = stringAt(binding->propertyNameIndex);
         // Attached property?
         if (binding->type == QV4::CompiledData::Binding::Type_AttachedProperty) {
             const QmlIR::Object *attachedObj = qmlObjects.at(binding->value.objectIndex);
             auto *typeRef = resolvedType(binding->propertyNameIndex);
             QQmlType type = typeRef ? typeRef->type() : QQmlType();
             if (!type.isValid())
-                imports->resolveType(propertyName, &type, nullptr, nullptr, nullptr);
+                imports->resolveType(bindingPropertyName, &type, nullptr, nullptr, nullptr);
 
             const QMetaObject *attachedType = type.attachedPropertiesType(enginePrivate);
             if (!attachedType)
                 COMPILE_EXCEPTION(binding, tr("Non-existent attached object"));
             QQmlPropertyCache *cache = compiler->enginePrivate()->cache(attachedType);
-            if (!convertSignalHandlerExpressionsToFunctionDeclarations(attachedObj, propertyName, cache))
+            if (!convertSignalHandlerExpressionsToFunctionDeclarations(attachedObj, bindingPropertyName, cache))
                 return false;
             continue;
         }
 
-        if (!QmlIR::IRBuilder::isSignalPropertyName(propertyName))
+        if (!QmlIR::IRBuilder::isSignalPropertyName(bindingPropertyName))
             continue;
 
         QQmlPropertyResolver resolver(propertyCache);
 
-        Q_ASSERT(propertyName.startsWith(QLatin1String("on")));
-        propertyName.remove(0, 2);
+        Q_ASSERT(bindingPropertyName.startsWith(QLatin1String("on")));
+        QString signalNameCandidate = bindingPropertyName;
+        signalNameCandidate.remove(0, 2);
 
         // Note that the property name could start with any alpha or '_' or '$' character,
         // so we need to do the lower-casing of the first alpha character.
-        for (int firstAlphaIndex = 0; firstAlphaIndex < propertyName.size(); ++firstAlphaIndex) {
-            if (propertyName.at(firstAlphaIndex).isUpper()) {
-                propertyName[firstAlphaIndex] = propertyName.at(firstAlphaIndex).toLower();
+        for (int firstAlphaIndex = 0; firstAlphaIndex < signalNameCandidate.size(); ++firstAlphaIndex) {
+            if (signalNameCandidate.at(firstAlphaIndex).isUpper()) {
+                signalNameCandidate[firstAlphaIndex] = signalNameCandidate.at(firstAlphaIndex).toLower();
                 break;
             }
         }
 
+        QString qPropertyName;
+        if (signalNameCandidate.endsWith(QLatin1String("Changed")))
+            qPropertyName = signalNameCandidate.mid(0, signalNameCandidate.length() - static_cast<int>(strlen("Changed")));
+
         QList<QString> parameters;
 
         bool notInRevision = false;
-        QQmlPropertyData *signal = resolver.signal(propertyName, &notInRevision);
+        QQmlPropertyData * const signal = resolver.signal(signalNameCandidate, &notInRevision);
+        QQmlPropertyData * const signalPropertyData = resolver.property(signalNameCandidate, /*notInRevision ptr*/nullptr);
+        QQmlPropertyData * const qPropertyData = !qPropertyName.isEmpty() ? resolver.property(qPropertyName) : nullptr;
+        QString finalSignalHandlerPropertyName = signalNameCandidate;
+        uint flags = QV4::CompiledData::Binding::IsSignalHandlerExpression;
+
         if (signal) {
             int sigIndex = propertyCache->methodIndexToSignalIndex(signal->coreIndex());
             sigIndex = propertyCache->originalClone(sigIndex);
@@ -389,10 +399,13 @@ bool SignalHandlerConverter::convertSignalHandlerExpressionsToFunctionDeclaratio
                 }
                 parameters += param;
             }
+        } else if (!signalPropertyData && qPropertyData && qPropertyData->isQProperty()) {
+            finalSignalHandlerPropertyName = qPropertyName;
+            flags = QV4::CompiledData::Binding::IsPropertyObserver;
         } else {
             if (notInRevision) {
                 // Try assinging it as a property later
-                if (resolver.property(propertyName, /*notInRevision ptr*/nullptr))
+                if (signalPropertyData)
                     continue;
 
                 const QString &originalPropertyName = stringAt(binding->propertyNameIndex);
@@ -424,11 +437,9 @@ bool SignalHandlerConverter::convertSignalHandlerExpressionsToFunctionDeclaratio
                 }
             }
 
-            QHash<QString, QStringList>::ConstIterator entry = customSignals.constFind(propertyName);
-            if (entry == customSignals.constEnd() && propertyName.endsWith(QLatin1String("Changed"))) {
-                QString alternateName = propertyName.mid(0, propertyName.length() - static_cast<int>(strlen("Changed")));
-                entry = customSignals.constFind(alternateName);
-            }
+            QHash<QString, QStringList>::ConstIterator entry = customSignals.constFind(signalNameCandidate);
+            if (entry == customSignals.constEnd() && !qPropertyName.isEmpty())
+                entry = customSignals.constFind(qPropertyName);
 
             if (entry == customSignals.constEnd()) {
                 // Can't find even a custom signal, then just don't do anything and try
@@ -490,8 +501,8 @@ bool SignalHandlerConverter::convertSignalHandlerExpressionsToFunctionDeclaratio
             functionDeclaration->rbraceToken = foe->node->lastSourceLocation();
         }
         foe->node = functionDeclaration;
-        binding->propertyNameIndex = compiler->registerString(propertyName);
-        binding->flags |= QV4::CompiledData::Binding::IsSignalHandlerExpression;
+        binding->propertyNameIndex = compiler->registerString(finalSignalHandlerPropertyName);
+        binding->flags |= flags;
     }
     return true;
 }
@@ -516,7 +527,8 @@ bool QQmlEnumTypeResolver::resolveEnumBindings()
 
         for (QmlIR::Binding *binding = obj->firstBinding(); binding; binding = binding->next) {
             if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression
-                || binding->flags & QV4::CompiledData::Binding::IsSignalHandlerObject)
+                || binding->flags & QV4::CompiledData::Binding::IsSignalHandlerObject
+                || binding->flags & QV4::CompiledData::Binding::IsPropertyObserver)
                 continue;
 
             if (binding->type != QV4::CompiledData::Binding::Type_Script)
@@ -1319,7 +1331,8 @@ bool QQmlDeferredAndCustomParserBindingScanner::scanObject(int objectIndex)
         }
 
         if (binding->flags & QV4::CompiledData::Binding::IsSignalHandlerExpression
-            || binding->flags & QV4::CompiledData::Binding::IsSignalHandlerObject)
+            || binding->flags & QV4::CompiledData::Binding::IsSignalHandlerObject
+            || binding->flags & QV4::CompiledData::Binding::IsPropertyObserver)
             continue;
 
         if (!pd) {
diff --git a/tests/auto/qml/qqmlecmascript/data/bindingOnQProperty.qml b/tests/auto/qml/qqmlecmascript/data/bindingOnQProperty.qml
index 1bd043d9df..2be07f35ae 100644
--- a/tests/auto/qml/qqmlecmascript/data/bindingOnQProperty.qml
+++ b/tests/auto/qml/qqmlecmascript/data/bindingOnQProperty.qml
@@ -4,4 +4,8 @@ ClassWithQProperty {
     value: {
         return externalValue
     }
+    property int changeHandlerCount: 0
+    onValueChanged: {
+        changeHandlerCount++;
+    }
 }
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 2a23d7ec60..fc50c2a09f 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -9288,8 +9288,11 @@ void tst_qqmlecmascript::bindingOnQProperty()
     QScopedPointer<QObject> test(component.create());
     test->setProperty("externalValue", 42);
     QCOMPARE(test->property("value").toInt(), 42);
+    // Value hasn't changed yet...
+    QCOMPARE(test->property("changeHandlerCount").toInt(), 0);
     test->setProperty("externalValue", 100);
     QCOMPARE(test->property("value").toInt(), 100);
+    QCOMPARE(test->property("changeHandlerCount").toInt(), 1);
 
     QVERIFY(qobject_cast<ClassWithQProperty*>(test.data()));
     QProperty<int> &qprop = static_cast<ClassWithQProperty*>(test.data())->value;