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 845bbb99a41a3e4f05c2b3d05d6db748c825dca0 (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>

5 files changed, 74 insertions, 6 deletions

diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index b30d88359b..ee3be6fcdc 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/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()) {
diff --git a/src/qml/qml/qqmlbinding.cpp b/src/qml/qml/qqmlbinding.cpp
index 3253c547ab..6336b2cedc 100644
--- a/src/qml/qml/qqmlbinding.cpp
+++ b/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
diff --git a/src/qml/qml/qqmlengine_p.h b/src/qml/qml/qqmlengine_p.h
index f8b8b187a5..1f513ef8cc 100644
--- a/src/qml/qml/qqmlengine_p.h
+++ b/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;
 
diff --git a/src/qml/qml/qqmljavascriptexpression.cpp b/src/qml/qml/qqmljavascriptexpression.cpp
index 0f83baa893..88a9932f53 100644
--- a/src/qml/qml/qqmljavascriptexpression.cpp
+++ b/src/qml/qml/qqmljavascriptexpression.cpp
@@ -38,6 +38,7 @@
 ****************************************************************************/
 
 #include "qqmljavascriptexpression_p.h"
+#include "qqmljavascriptexpression_p.h"
 
 #include <private/qqmlexpression_p.h>
 #include <private/qv4context_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())
diff --git a/src/qml/qml/qqmljavascriptexpression_p.h b/src/qml/qml/qqmljavascriptexpression_p.h
index c2df5004d1..a1e06e34da 100644
--- a/src/qml/qml/qqmljavascriptexpression_p.h
+++ b/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