Handle QProperty inside bindings

If we have a binding containing QProperties, and the binding target is
an old style binding, still we have to trigger an update if any of the
captured properties changes. We cannot reuse the
QQmlJavaScriptExpressionGuards as those depend on Qt's signals, and a
QProperty is not associated with a change signal in the general case.
Therefore, we introduce a new list of QPropertyChangeHandler, which when
triggered cause a reevaluation of the binding.
As an optimization, we skip the whole capturing process for
QQmlPropertyBinding, as that one already takes care of updating itself.

Reverts 845bbb99a4 (because skipping the
capture is only possible when _both_ the bindee and the property in the
binding are QProperty based.)

Change-Id: Iafed2a41dcd708bcc33912ce810d803949379c63
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

src/qml/jsruntime/qv4qobjectwrapper.cpp

@@ -261,7 +261,7 @@ ReturnedValue QObjectWrapper::getProperty(ExecutionEngine *engine, QObject *obje
 
     QQmlEnginePrivate *ep = engine->qmlEngine() ? QQmlEnginePrivate::get(engine->qmlEngine()) : nullptr;
 
-    if (ep && ep->propertyCapture && !property->isConstant() && !property->isBindable())
+    if (ep && ep->propertyCapture && !property->isConstant())
         ep->propertyCapture->captureProperty(object, property->coreIndex(), property->notifyIndex());
 
     if (property->isVarProperty()) {

src/qml/qml/qqmlbinding.cpp

@@ -670,7 +670,7 @@ QVector<QQmlProperty> QQmlBinding::dependencies() const
 
 bool QQmlBinding::hasDependencies() const
 {
-    return !activeGuards.isEmpty() || translationsCaptured();
+    return !activeGuards.isEmpty() || translationsCaptured() || qpropertyChangeTriggers;
 }
 
 class QObjectPointerBinding: public QQmlNonbindingBinding

src/qml/qml/qqmlengine_p.h

@@ -81,6 +81,8 @@
 #include <private/qjsengine_p.h>
 #include <private/qqmldirparser_p.h>
 
+#include <qproperty.h>
+
 QT_BEGIN_NAMESPACE
 
 class QQmlContext;
@@ -125,6 +127,18 @@ public:
     QQmlJavaScriptExpressionGuard *next;
 };
 
+struct QPropertyChangeTrigger {
+    QQmlJavaScriptExpression * m_expression;
+    void operator()();
+};
+
+struct TriggerList : QPropertyChangeHandler<QPropertyChangeTrigger> {
+    TriggerList(QPropertyChangeTrigger trigger) : QPropertyChangeHandler<QPropertyChangeTrigger>(trigger) {};
+    TriggerList *next = nullptr;
+    QObject *target = nullptr;
+    int propertyIndex = 0;
+};
+
 class Q_QML_PRIVATE_EXPORT QQmlEnginePrivate : public QJSEnginePrivate
 {
     Q_DECLARE_PUBLIC(QQmlEngine)
@@ -140,6 +154,7 @@ public:
     QQmlPropertyCapture *propertyCapture;
 
     QRecyclePool<QQmlJavaScriptExpressionGuard> jsExpressionGuardPool;
+    QRecyclePool<TriggerList> qPropertyTriggerPool;
 
     QQmlContext *rootContext;
 

src/qml/qml/qqmljavascriptexpression.cpp

@@ -37,6 +37,7 @@
 **
 ****************************************************************************/
 
+#include "qqmljavascriptexpression_p.h"
 #include "qqmljavascriptexpression_p.h"
 
 #include <private/qqmlexpression_p.h>
@@ -51,6 +52,9 @@
 #include <private/qv4qobjectwrapper_p.h>
 #include <private/qqmlbuiltinfunctions_p.h>
 #include <private/qqmlsourcecoordinate_p.h>
+#include <private/qqmlabstractbinding_p.h>
+#include <private/qqmlpropertybinding_p.h>
+#include <private/qproperty_p.h>
 
 QT_BEGIN_NAMESPACE
 
@@ -109,6 +113,12 @@ QQmlJavaScriptExpression::~QQmlJavaScriptExpression()
             m_nextExpression->m_prevExpression = m_prevExpression;
     }
 
+    while (qpropertyChangeTriggers) {
+        auto current = qpropertyChangeTriggers;
+        qpropertyChangeTriggers = current->next;
+        QRecyclePool<TriggerList>::Delete(current);
+    }
+
     clearActiveGuards();
     clearError();
     if (m_scopeObject.isT2()) // notify DeleteWatcher of our deletion.
@@ -289,7 +299,33 @@ void QQmlPropertyCapture::captureProperty(QObject *o, int c, int n, bool doNotif
         return;
 
     Q_ASSERT(expression);
+    const QQmlData *ddata = QQmlData::get(o, /*create=*/false);
+    bool isBindable = false;
+    if (auto const propCache = ddata ? ddata->propertyCache : nullptr; propCache) {
+        Q_ASSERT(propCache->property(c));
+        isBindable = propCache->property(c)->isBindable();
+    } else {
+        auto metaProp = o->staticMetaObject.property(c);
+        isBindable = metaProp.isBindable();
+    }
+    if (isBindable) {
+        // if the property is a QPropery, and we're binding to a QProperty
+        // the automatic capturing process already takes care of everything
+        if (typeid(QQmlPropertyBinding) == typeid(*expression))
+            return;
+        for (auto trigger = expression->qpropertyChangeTriggers; trigger; trigger = trigger->next) {
+            if (trigger->target == o && trigger->propertyIndex == c)
+                return; // already installed
+        }
+        auto trigger = expression->allocatePropertyChangeTrigger(o, c);
+        QUntypedBindable bindable;
+        void *argv[] = { &bindable };
+        o->qt_metacall(QMetaObject::BindableProperty, c, argv);
+        bindable.observe(trigger);
+        return;
+    }
     if (n == -1) {
+
         if (!errorString) {
             errorString = new QStringList;
             QString preamble = QLatin1String("QQmlExpression: Expression ") +
@@ -298,11 +334,9 @@ void QQmlPropertyCapture::captureProperty(QObject *o, int c, int n, bool doNotif
             errorString->append(preamble);
         }
 
-        const QMetaObject *metaObj = o->metaObject();
-        QMetaProperty metaProp = metaObj->property(c);
-
+        const QMetaProperty metaProp = o->metaObject()->property(c);
         QString error = QLatin1String("    ") +
-                QString::fromUtf8(metaObj->className()) +
+                QString::fromUtf8(o->metaObject()->className()) +
                 QLatin1String("::") +
                 QString::fromUtf8(metaProp.name());
         errorString->append(error);
@@ -413,6 +447,21 @@ void QQmlJavaScriptExpression::setCompilationUnit(const QQmlRefPointer<QV4::Exec
     m_compilationUnit = compilationUnit;
 }
 
+void QPropertyChangeTrigger::operator()() {
+    m_expression->expressionChanged();
+}
+
+QPropertyChangeHandler<QPropertyChangeTrigger> *QQmlJavaScriptExpression::allocatePropertyChangeTrigger(QObject *target, int propertyIndex)
+{
+    auto trigger = QQmlEnginePrivate::get(engine())->qPropertyTriggerPool.New(QPropertyChangeTrigger { this });
+    trigger->target = target;
+    trigger->propertyIndex = propertyIndex;
+    auto oldHead = qpropertyChangeTriggers;
+    trigger->next = oldHead;
+    qpropertyChangeTriggers = trigger;
+    return trigger;
+}
+
 void QQmlJavaScriptExpression::clearActiveGuards()
 {
     while (QQmlJavaScriptExpressionGuard *g = activeGuards.takeFirst())

src/qml/qml/qqmljavascriptexpression_p.h

@@ -164,6 +164,7 @@ public:
 
     QQmlEngine *engine() const { return m_context ? m_context->engine() : nullptr; }
     bool hasUnresolvedNames() const { return m_context && m_context->hasUnresolvedNames(); }
+    QPropertyChangeHandler<QPropertyChangeTrigger>* allocatePropertyChangeTrigger(QObject *target, int propertyIndex);
 
 protected:
     void createQmlBinding(const QQmlRefPointer<QQmlContextData> &ctxt, QObject *scope,
@@ -216,6 +217,9 @@ private:
     QV4::PersistentValue m_qmlScope;
     QQmlRefPointer<QV4::ExecutableCompilationUnit> m_compilationUnit;
     QV4::Function *m_v4Function;
+
+protected:
+    TriggerList *qpropertyChangeTriggers = nullptr;
 };
 
 class Q_QML_PRIVATE_EXPORT QQmlPropertyCapture

tests/auto/qml/qqmlecmascript/data/bindingContainingQProperty.qml

@@ -0,0 +1,5 @@
+import Qt.test 1.0
+
+ClassWithQProperty {
+    property real expected: value
+}

tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp

@@ -383,6 +383,7 @@ private slots:
     void hugeStack();
     void bindingOnQProperty();
     void bindingOnQPropertyContextProperty();
+    void bindingContainingQProperty();
     void urlConstruction();
     void urlPropertyInvalid();
     void urlPropertySet();
@@ -9189,6 +9190,17 @@ void tst_qqmlecmascript::bindingOnQPropertyContextProperty()
     // QCOMPARE(classWithQProperty->value.value(), 2);
 }
 
+void tst_qqmlecmascript::bindingContainingQProperty()
+{
+    QQmlEngine engine;
+    QQmlComponent component(&engine, testFileUrl("bindingContainingQProperty.qml"));
+    QVERIFY2(component.isReady(), qPrintable(component.errorString()));
+    QScopedPointer<QObject> test(component.create());
+    QVERIFY(!test.isNull());
+    test->setProperty("value", 42.0);
+    QCOMPARE(test->property("expected"), 42.0);
+}
+
 void tst_qqmlecmascript::urlConstruction()
 {
     QQmlEngine qmlengine;