Make property interceptors work on alias properties again

Fixes a regression introduced by change
01c0c0963794f4dd8c3601e8340cc3dc4dec41bd, where interceptors
wouldn't work correctly on alias properties.

This required some refactoring and splitting out the interceptor
handling from the VMEMO into it's own class, as we are now
installing bindings directly on the target property of an alias and
not on the alias anymore.

We now resolve the target property inside the QML object creator
and install a interceptor metaobject on the target if required where
we can then register the interceptor.

Change-Id: I3ebc8f492ce5dcab7acf901711b47336d5182ffa
Task-number: QTBUG-49072
Reviewed-by: Simon Hausmann <simon.hausmann@theqtcompany.com>

6 files changed, 192 insertions, 114 deletions

diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h
index 3d2a76693a..ef05dd1fd7 100644
--- a/src/qml/qml/qqmldata_p.h
+++ b/src/qml/qml/qqmldata_p.h
@@ -112,9 +112,10 @@ public:
      * v8 GC will check this flag, only deletes the objects when rootObjectInCreation is false.
      */
     quint32 rootObjectInCreation:1;
+    quint32 hasInterceptorMetaObject:1;
     quint32 hasVMEMetaObject:1;
     quint32 parentFrozen:1;
-    quint32 dummy:22;
+    quint32 dummy:21;
 
     // When bindingBitsSize < 32, we store the binding bit flags inside
     // bindingBitsValue. When we need more than 32 bits, we allocated
diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp
index 93e66a1b12..e8422474d3 100644
--- a/src/qml/qml/qqmlengine.cpp
+++ b/src/qml/qml/qqmlengine.cpp
@@ -667,7 +667,7 @@ void QQmlPrivate::qdeclarativeelement_destructor(QObject *o)
 QQmlData::QQmlData()
     : ownedByQml1(false), ownMemory(true), ownContext(false), indestructible(true), explicitIndestructibleSet(false),
       hasTaintedV4Object(false), isQueuedForDeletion(false), rootObjectInCreation(false),
-      hasVMEMetaObject(false), parentFrozen(false), bindingBitsSize(0), bindingBits(0), notifyList(0), context(0), outerContext(0),
+      hasInterceptorMetaObject(false), hasVMEMetaObject(false), parentFrozen(false), bindingBitsSize(0), bindingBits(0), notifyList(0), context(0), outerContext(0),
       bindings(0), signalHandlers(0), nextContextObject(0), prevContextObject(0),
       lineNumber(0), columnNumber(0), jsEngineId(0), compiledData(0), deferredData(0),
       propertyCache(0), guards(0), extendedData(0)
diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp
index c03a463c83..21e6d5f6de 100644
--- a/src/qml/qml/qqmlobjectcreator.cpp
+++ b/src/qml/qml/qqmlobjectcreator.cpp
@@ -859,11 +859,24 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *property, con
                 QQmlPropertyValueInterceptor *vi = reinterpret_cast<QQmlPropertyValueInterceptor *>(reinterpret_cast<char *>(createdSubObject) + valueInterceptorCast);
                 QObject *target = createdSubObject->parent();
 
+                if (targetCorePropertyData.isAlias()) {
+                    int propIndex;
+                    QQmlPropertyPrivate::findAliasTarget(target, targetCorePropertyData.coreIndex, &target, &propIndex);
+                    QQmlData *data = QQmlData::get(target);
+                    if (!data || !data->propertyCache) {
+                        qWarning() << "can't resolve property alias for 'on' assignment";
+                        return false;
+                    }
+                    targetCorePropertyData = *data->propertyCache->property(propIndex);
+                }
+
                 QQmlProperty prop =
                     QQmlPropertyPrivate::restore(target, targetCorePropertyData, context);
+
                 vi->setTarget(prop);
-                QQmlVMEMetaObject *mo = QQmlVMEMetaObject::get(target);
-                Q_ASSERT(mo);
+                QQmlInterceptorMetaObject *mo = QQmlInterceptorMetaObject::get(target);
+                if (!mo)
+                    mo = new QQmlInterceptorMetaObject(target, QQmlData::get(target)->propertyCache);
                 mo->registerInterceptor(prop.index(), QQmlPropertyPrivate::valueTypeCoreIndex(prop), vi);
                 return true;
             }
diff --git a/src/qml/qml/qqmlpropertyvalueinterceptor_p.h b/src/qml/qml/qqmlpropertyvalueinterceptor_p.h
index ea267f9c30..6403e85f2a 100644
--- a/src/qml/qml/qqmlpropertyvalueinterceptor_p.h
+++ b/src/qml/qml/qqmlpropertyvalueinterceptor_p.h
@@ -60,7 +60,7 @@ public:
     virtual void write(const QVariant &value) = 0;
 
 private:
-    friend class QQmlVMEMetaObject;
+    friend class QQmlInterceptorMetaObject;
 
     int m_coreIndex;
     int m_valueTypeCoreIndex;
diff --git a/src/qml/qml/qqmlvmemetaobject.cpp b/src/qml/qml/qqmlvmemetaobject.cpp
index 67c4295d95..37dbfec2a3 100644
--- a/src/qml/qml/qqmlvmemetaobject.cpp
+++ b/src/qml/qml/qqmlvmemetaobject.cpp
@@ -130,7 +130,131 @@ void QQmlVMEMetaObjectEndpoint::tryConnect()
     }
 }
 
-QAbstractDynamicMetaObject *QQmlVMEMetaObject::toDynamicMetaObject(QObject *o)
+
+QQmlInterceptorMetaObject::QQmlInterceptorMetaObject(QObject *obj, QQmlPropertyCache *cache)
+    : object(obj),
+      cache(cache),
+      interceptors(0),
+      hasAssignedMetaObjectData(false)
+{
+    QObjectPrivate *op = QObjectPrivate::get(obj);
+
+    if (op->metaObject) {
+        parent = op->metaObject;
+        // Use the extra flag in QBiPointer to know if we can safely cast parent.asT1() to QQmlVMEMetaObject*
+        parent.setFlagValue(QQmlData::get(obj)->hasVMEMetaObject);
+    } else {
+        parent = obj->metaObject();
+    }
+
+    op->metaObject = this;
+    QQmlData::get(obj)->hasInterceptorMetaObject = true;
+}
+
+QQmlInterceptorMetaObject::~QQmlInterceptorMetaObject()
+{
+
+}
+
+void QQmlInterceptorMetaObject::registerInterceptor(int index, int valueIndex, QQmlPropertyValueInterceptor *interceptor)
+{
+    interceptor->m_coreIndex = index;
+    interceptor->m_valueTypeCoreIndex = valueIndex;
+    interceptor->m_next = interceptors;
+    interceptors = interceptor;
+}
+
+int QQmlInterceptorMetaObject::metaCall(QObject *o, QMetaObject::Call c, int id, void **a)
+{
+    Q_ASSERT(o == object);
+    Q_UNUSED(o);
+
+    if (intercept(c, id, a))
+        return -1;
+    return object->qt_metacall(c, id, a);
+}
+
+bool QQmlInterceptorMetaObject::intercept(QMetaObject::Call c, int id, void **a)
+{
+    if (c == QMetaObject::WriteProperty && interceptors &&
+       !(*reinterpret_cast<int*>(a[3]) & QQmlPropertyPrivate::BypassInterceptor)) {
+
+        for (QQmlPropertyValueInterceptor *vi = interceptors; vi; vi = vi->m_next) {
+            if (vi->m_coreIndex != id)
+                continue;
+
+            int valueIndex = vi->m_valueTypeCoreIndex;
+            int type = QQmlData::get(object)->propertyCache->property(id)->propType;
+
+            if (type != QVariant::Invalid) {
+                if (valueIndex != -1) {
+                    QQmlValueType *valueType = QQmlValueTypeFactory::valueType(type);
+                    Q_ASSERT(valueType);
+
+                    //
+                    // Consider the following case:
+                    //  color c = { 0.1, 0.2, 0.3 }
+                    //  interceptor exists on c.r
+                    //  write { 0.2, 0.4, 0.6 }
+                    //
+                    // The interceptor may choose not to update the r component at this
+                    // point (for example, a behavior that creates an animation). But we
+                    // need to ensure that the g and b components are updated correctly.
+                    //
+                    // So we need to perform a full write where the value type is:
+                    //    r = old value, g = new value, b = new value
+                    //
+                    // And then call the interceptor which may or may not write the
+                    // new value to the r component.
+                    //
+                    // This will ensure that the other components don't contain stale data
+                    // and any relevant signals are emitted.
+                    //
+                    // To achieve this:
+                    //   (1) Store the new value type as a whole (needed due to
+                    //       aliasing between a[0] and static storage in value type).
+                    //   (2) Read the entire existing value type from object -> valueType temp.
+                    //   (3) Read the previous value of the component being changed
+                    //       from the valueType temp.
+                    //   (4) Write the entire new value type into the temp.
+                    //   (5) Overwrite the component being changed with the old value.
+                    //   (6) Perform a full write to the value type (which may emit signals etc).
+                    //   (7) Issue the interceptor call with the new component value.
+                    //
+
+                    QMetaProperty valueProp = valueType->metaObject()->property(valueIndex);
+                    QVariant newValue(type, a[0]);
+
+                    valueType->read(object, id);
+                    QVariant prevComponentValue = valueProp.read(valueType);
+
+                    valueType->setValue(newValue);
+                    QVariant newComponentValue = valueProp.read(valueType);
+
+                    // Don't apply the interceptor if the intercepted value has not changed
+                    bool updated = false;
+                    if (newComponentValue != prevComponentValue) {
+                        valueProp.write(valueType, prevComponentValue);
+                        valueType->write(object, id, QQmlPropertyPrivate::DontRemoveBinding | QQmlPropertyPrivate::BypassInterceptor);
+
+                        vi->write(newComponentValue);
+                        updated = true;
+                    }
+
+                    if (updated)
+                        return true;
+                } else {
+                    vi->write(QVariant(type, a[0]));
+                    return true;
+                }
+            }
+        }
+    }
+    return false;
+}
+
+
+QAbstractDynamicMetaObject *QQmlInterceptorMetaObject::toDynamicMetaObject(QObject *o)
 {
     if (!hasAssignedMetaObjectData) {
         *static_cast<QMetaObject *>(this) = *cache->createMetaObject();
@@ -149,23 +273,13 @@ QAbstractDynamicMetaObject *QQmlVMEMetaObject::toDynamicMetaObject(QObject *o)
 QQmlVMEMetaObject::QQmlVMEMetaObject(QObject *obj,
                                      QQmlPropertyCache *cache,
                                      const QQmlVMEMetaData *meta)
-    : object(obj),
-      ctxt(QQmlData::get(obj, true)->outerContext), cache(cache), metaData(meta),
-      hasAssignedMetaObjectData(false), aliasEndpoints(0),
-      interceptors(0), methods(0)
+    : QQmlInterceptorMetaObject(obj, cache),
+      ctxt(QQmlData::get(obj, true)->outerContext), metaData(meta),
+      aliasEndpoints(0),
+      methods(0)
 {
     cache->addref();
 
-    QObjectPrivate *op = QObjectPrivate::get(obj);
-
-    if (op->metaObject) {
-        parent = op->metaObject;
-        // Use the extra flag in QBiPointer to know if we can safely cast parent.asT1() to QQmlVMEMetaObject*
-        parent.setFlagValue(QQmlData::get(obj)->hasVMEMetaObject);
-    } else
-        parent = obj->metaObject();
-
-    op->metaObject = this;
     QQmlData::get(obj)->hasVMEMetaObject = true;
 
     int qobject_type = qMetaTypeId<QObject*>();
@@ -471,80 +585,10 @@ int QQmlVMEMetaObject::metaCall(QObject *o, QMetaObject::Call c, int _id, void *
     Q_UNUSED(o);
 
     int id = _id;
-    if (c == QMetaObject::WriteProperty && interceptors &&
-       !(*reinterpret_cast<int*>(a[3]) & QQmlPropertyPrivate::BypassInterceptor)) {
-
-        for (QQmlPropertyValueInterceptor *vi = interceptors; vi; vi = vi->m_next) {
-            if (vi->m_coreIndex != id)
-                continue;
-
-            int valueIndex = vi->m_valueTypeCoreIndex;
-            int type = QQmlData::get(object)->propertyCache->property(id)->propType;
-
-            if (type != QVariant::Invalid) {
-                if (valueIndex != -1) {
-                    QQmlValueType *valueType = QQmlValueTypeFactory::valueType(type);
-                    Q_ASSERT(valueType);
-
-                    //
-                    // Consider the following case:
-                    //  color c = { 0.1, 0.2, 0.3 }
-                    //  interceptor exists on c.r
-                    //  write { 0.2, 0.4, 0.6 }
-                    //
-                    // The interceptor may choose not to update the r component at this
-                    // point (for example, a behavior that creates an animation). But we
-                    // need to ensure that the g and b components are updated correctly.
-                    //
-                    // So we need to perform a full write where the value type is:
-                    //    r = old value, g = new value, b = new value
-                    //
-                    // And then call the interceptor which may or may not write the
-                    // new value to the r component.
-                    //
-                    // This will ensure that the other components don't contain stale data
-                    // and any relevant signals are emitted.
-                    //
-                    // To achieve this:
-                    //   (1) Store the new value type as a whole (needed due to
-                    //       aliasing between a[0] and static storage in value type).
-                    //   (2) Read the entire existing value type from object -> valueType temp.
-                    //   (3) Read the previous value of the component being changed
-                    //       from the valueType temp.
-                    //   (4) Write the entire new value type into the temp.
-                    //   (5) Overwrite the component being changed with the old value.
-                    //   (6) Perform a full write to the value type (which may emit signals etc).
-                    //   (7) Issue the interceptor call with the new component value.
-                    //
-
-                    QMetaProperty valueProp = valueType->metaObject()->property(valueIndex);
-                    QVariant newValue(type, a[0]);
 
-                    valueType->read(object, id);
-                    QVariant prevComponentValue = valueProp.read(valueType);
-
-                    valueType->setValue(newValue);
-                    QVariant newComponentValue = valueProp.read(valueType);
+    if (intercept(c, _id, a))
+        return -1;
 
-                    // Don't apply the interceptor if the intercepted value has not changed
-                    bool updated = false;
-                    if (newComponentValue != prevComponentValue) {
-                        valueProp.write(valueType, prevComponentValue);
-                        valueType->write(object, id, QQmlPropertyPrivate::DontRemoveBinding | QQmlPropertyPrivate::BypassInterceptor);
-
-                        vi->write(newComponentValue);
-                        updated = true;
-                    }
-
-                    if (updated)
-                        return -1;
-                } else {
-                    vi->write(QVariant(type, a[0]));
-                    return -1;
-                }
-            }
-        }
-    }
     if (c == QMetaObject::ReadProperty || c == QMetaObject::WriteProperty || c == QMetaObject::ResetProperty) {
         if (id >= propOffset()) {
             id -= propOffset();
@@ -992,14 +1036,6 @@ void QQmlVMEMetaObject::list_clear(QQmlListProperty<QObject> *prop)
     static_cast<QQmlVMEMetaObject *>(prop->dummy1)->activate(prop->object, reinterpret_cast<quintptr>(prop->dummy2), 0);
 }
 
-void QQmlVMEMetaObject::registerInterceptor(int index, int valueIndex, QQmlPropertyValueInterceptor *interceptor)
-{
-    interceptor->m_coreIndex = index;
-    interceptor->m_valueTypeCoreIndex = valueIndex;
-    interceptor->m_next = interceptors;
-    interceptors = interceptor;
-}
-
 quint16 QQmlVMEMetaObject::vmeMethodLineNumber(int index)
 {
     if (index < methodOffset()) {
diff --git a/src/qml/qml/qqmlvmemetaobject_p.h b/src/qml/qml/qqmlvmemetaobject_p.h
index af0742d2c6..7da44e3b82 100644
--- a/src/qml/qml/qqmlvmemetaobject_p.h
+++ b/src/qml/qml/qqmlvmemetaobject_p.h
@@ -148,17 +148,57 @@ public:
     int m_index;
 };
 
+
+class Q_QML_PRIVATE_EXPORT QQmlInterceptorMetaObject : public QAbstractDynamicMetaObject
+{
+public:
+    QQmlInterceptorMetaObject(QObject *obj, QQmlPropertyCache *cache);
+    ~QQmlInterceptorMetaObject();
+
+    void registerInterceptor(int index, int valueIndex, QQmlPropertyValueInterceptor *interceptor);
+
+    static QQmlInterceptorMetaObject *get(QObject *obj);
+
+    virtual QAbstractDynamicMetaObject *toDynamicMetaObject(QObject *o);
+
+    // Used by auto-tests for inspection
+    QQmlPropertyCache *propertyCache() const { return cache; }
+
+protected:
+    virtual int metaCall(QObject *o, QMetaObject::Call c, int id, void **a);
+    bool intercept(QMetaObject::Call c, int id, void **a);
+
+public:
+    QObject *object;
+    QQmlPropertyCache *cache;
+    QBiPointer<QDynamicMetaObjectData, const QMetaObject> parent;
+
+    QQmlPropertyValueInterceptor *interceptors;
+    bool hasAssignedMetaObjectData;
+};
+
+inline QQmlInterceptorMetaObject *QQmlInterceptorMetaObject::get(QObject *obj)
+{
+    if (obj) {
+        if (QQmlData *data = QQmlData::get(obj)) {
+            if (data->hasInterceptorMetaObject)
+                return static_cast<QQmlInterceptorMetaObject *>(QObjectPrivate::get(obj)->metaObject);
+        }
+    }
+
+    return 0;
+}
+
 class QQmlVMEVariant;
 class QQmlRefCount;
 class QQmlVMEMetaObjectEndpoint;
-class Q_QML_PRIVATE_EXPORT QQmlVMEMetaObject : public QAbstractDynamicMetaObject
+class Q_QML_PRIVATE_EXPORT QQmlVMEMetaObject : public QQmlInterceptorMetaObject
 {
 public:
     QQmlVMEMetaObject(QObject *obj, QQmlPropertyCache *cache, const QQmlVMEMetaData *data);
     ~QQmlVMEMetaObject();
 
     bool aliasTarget(int index, QObject **target, int *coreIndex, int *valueTypeIndex) const;
-    void registerInterceptor(int index, int valueIndex, QQmlPropertyValueInterceptor *interceptor);
     QV4::ReturnedValue vmeMethod(int index);
     quint16 vmeMethodLineNumber(int index);
     void setVmeMethod(int index, const QV4::Value &function);
@@ -167,11 +207,6 @@ public:
 
     void connectAliasSignal(int index, bool indexInSignalRange);
 
-    virtual QAbstractDynamicMetaObject *toDynamicMetaObject(QObject *o);
-
-    // Used by auto-tests for inspection
-    QQmlPropertyCache *propertyCache() const { return cache; }
-
     static inline QQmlVMEMetaObject *get(QObject *o);
     static QQmlVMEMetaObject *getForProperty(QObject *o, int coreIndex);
     static QQmlVMEMetaObject *getForMethod(QObject *o, int coreIndex);
@@ -185,9 +220,7 @@ public:
     friend class QQmlVMEVariantQObjectPtr;
     friend class QQmlPropertyCache;
 
-    QObject *object;
     QQmlGuardedContextData ctxt;
-    QQmlPropertyCache *cache;
 
     const QQmlVMEMetaData *metaData;
     inline int propOffset() const;
@@ -195,7 +228,6 @@ public:
     inline int signalOffset() const;
     inline int signalCount() const;
 
-    bool hasAssignedMetaObjectData;
     QQmlVMEMetaObjectEndpoint *aliasEndpoints;
 
     QV4::WeakValue properties;
@@ -233,8 +265,6 @@ public:
 
     void connectAlias(int aliasId);
 
-    QQmlPropertyValueInterceptor *interceptors;
-
     QV4::PersistentValue *methods;
     QV4::ReturnedValue method(int);
 
@@ -243,8 +273,6 @@ public:
     QVariant readPropertyAsVariant(int);
     void writeProperty(int, const QVariant &);
 
-    QBiPointer<QDynamicMetaObjectData, const QMetaObject> parent;
-
     inline QQmlVMEMetaObject *parentVMEMetaObject() const;
 
     void listChanged(int);