Revise QQmlProxyMetaObject and extension chain creation

Clone the full-ish meta object hierarchy when dealing with extensions:
that way we guarantee that shadowing works correctly both ways -
extension properties can shadow type properties AND type properties
can shadow extension properties (the latter happens when extension
belongs to a base type). This was impossible before since we would
always put extension proxies on top of the meta object chain,
regardless of where in the chain extensions are located. Consider:
"C -> B + ExtensionB -> A + ExtensionA" is interpreted as
(old) ExtensionB -> ExtensionA -> C -> B -> A
      ^^^^^^^^^^^^^^^^^^^^^^^^    ^^^^^^^^^^^
        cloned meta objects     static meta objects

(new) C -> ExtensionB -> B -> ExtensionA -> A -> C -> B -> A
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^    ^^^^^^^^^^^
               cloned meta objects             static meta objects

One can notice that we now clone _more_ meta objects: this is to
be able to to create a chain with correctly ordered properties

Unify the proxy data creation under QQmlMetaType::proxyData()

Update QQmlProxyMetaObject which now has to deal with different types
of proxies: not only extensions but also "cloned" self. CustomCall
metaCall() of QQmlProxyMetaObject is also adjusted to ignore
non-extensions

Change-Id: I40e1547a9c05a8504ab98bc06a6bc412a2460783
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>

7 files changed, 166 insertions, 80 deletions

diff --git a/src/qml/qml/qqmlmetatype.cpp b/src/qml/qml/qqmlmetatype.cpp
index 110363ae3b..46e53739ff 100644
--- a/src/qml/qml/qqmlmetatype.cpp
+++ b/src/qml/qml/qqmlmetatype.cpp
@@ -237,7 +237,7 @@ static QQmlTypePrivate *createQQmlType(QQmlMetaTypeData *data, const QString &el
 
 void QQmlMetaType::clone(QMetaObjectBuilder &builder, const QMetaObject *mo,
                          const QMetaObject *ignoreStart, const QMetaObject *ignoreEnd,
-                         QQmlMetaType::ClonePolicy policy)
+                         QQmlMetaType::ClonePolicy policy, QQmlMetaType::CloneTarget target)
 {
     // Set classname
     builder.setClassName(mo->className());
@@ -272,6 +272,19 @@ void QQmlMetaType::clone(QMetaObjectBuilder &builder, const QMetaObject *mo,
                 found = name == other.name();
             }
 
+            // do not clone signals from the type itself: they are hard to do
+            // right from both QML and C++. instead, just ignore they exist and
+            // put a dummy into the slot instead. the disadvantage is that
+            // extension signals would always shadow type signals regardless of
+            // where in the chain an extension exists. however, this behavior is
+            // how it used to work, so there is no regression
+            const bool ignoreSignal = (target == QQmlMetaType::CloneTargetObject
+                                       && method.methodType() == QMetaMethod::Signal);
+            if (ignoreSignal) {
+                builder.addSignal(QByteArray("__qml_ignore__") + method.methodSignature());
+                continue;
+            }
+
             QMetaMethodBuilder m = builder.addMethod(method);
             if (found) // SKIP
                 m.setAccess(QMetaMethod::Private);
@@ -281,8 +294,16 @@ void QQmlMetaType::clone(QMetaObjectBuilder &builder, const QMetaObject *mo,
         for (int ii = mo->propertyOffset(); ii < mo->propertyCount(); ++ii) {
             QMetaProperty property = mo->property(ii);
 
+            // do not clone final properties from the type itself, otherwise
+            // they would be considered overridden (error). following the same
+            // logic, extension properties must not override type's final
+            // properties, so this is somewhat safe proxy-model-wise
+            const bool ignoreProperty =
+                    (target == QQmlMetaType::CloneTargetObject && property.isFinal());
+
             int otherIndex = ignoreEnd->indexOfProperty(property.name());
-            if (otherIndex >= ignoreStart->propertyOffset() + ignoreStart->propertyCount()) {
+            if (ignoreProperty
+                || (otherIndex >= ignoreStart->propertyOffset() + ignoreStart->propertyCount())) {
                 builder.addProperty(QByteArray("__qml_ignore__") + property.name(),
                                     QByteArray("void"));
                 // Skip
@@ -1634,51 +1655,106 @@ QString QQmlMetaType::prettyTypeName(const QObject *object)
     return typeName;
 }
 
-QList<QQmlProxyMetaObject::ProxyData> QQmlMetaType::proxyData(const QMetaObject *mo,
-                                                              const QMetaObject *baseMetaObject,
-                                                              QMetaObject *lastMetaObject)
+template<typename Action>
+static void iterateExtensions(const QQmlMetaTypeDataPtr &typeData, QQmlTypePrivate *t,
+                              const QMetaObject *mo, Action visit)
 {
-    QList<QQmlProxyMetaObject::ProxyData> metaObjects;
-    mo = mo->d.superdata;
-
-    const QQmlMetaTypeDataPtr data;
+    Q_ASSERT(t);
+    Q_ASSERT(mo);
 
-    auto createProxyMetaObject = [&](QQmlTypePrivate *This,
-                                     const QMetaObject *superdataBaseMetaObject,
-                                     const QMetaObject *extMetaObject,
-                                     QObject *(*extFunc)(QObject *)) {
-        if (!extMetaObject)
-            return;
-
-        QMetaObjectBuilder builder;
-        clone(builder, extMetaObject, superdataBaseMetaObject, baseMetaObject,
-              extFunc ? QQmlMetaType::CloneAll : QQmlMetaType::CloneEnumsOnly);
-        QMetaObject *mmo = builder.toMetaObject();
-        mmo->d.superdata = baseMetaObject;
-        if (!metaObjects.isEmpty())
-            metaObjects.constLast().metaObject->d.superdata = mmo;
-        else if (lastMetaObject)
-            lastMetaObject->d.superdata = mmo;
-        QQmlProxyMetaObject::ProxyData data = { mmo, extFunc, 0, 0 };
-        metaObjects << data;
-        registerMetaObjectForType(mmo, This);
-    };
-
-    while (mo) {
-        QQmlTypePrivate *t = data->metaObjectToType.value(mo);
+    bool finish = false;
+    while (mo && !finish) {
         if (t) {
             if (t->regType == QQmlType::CppType) {
-                createProxyMetaObject(t, t->baseMetaObject, t->extraData.cd->extMetaObject,
-                                      t->extraData.cd->extFunc);
+                finish = visit(t, t->baseMetaObject, t->extraData.cd->extMetaObject,
+                               t->extraData.cd->extFunc);
             } else if (t->regType == QQmlType::SingletonType) {
-                createProxyMetaObject(t, t->baseMetaObject, t->extraData.sd->extMetaObject,
-                                      t->extraData.sd->extFunc);
+                finish = visit(t, t->baseMetaObject, t->extraData.sd->extMetaObject,
+                               t->extraData.sd->extFunc);
             }
         }
         mo = mo->d.superdata;
+        t = typeData->metaObjectToType.value(mo);
+    }
+}
+
+QList<QQmlProxyMetaObject::ProxyData> QQmlMetaType::proxyData(QQmlTypePrivate *type,
+                                                              const QMetaObject *mo,
+                                                              const QMetaObject *baseMetaObject)
+{
+    const QQmlMetaTypeDataPtr data;
+
+    qsizetype extensionCount = 0;
+    iterateExtensions(data, type, mo,
+                      [&](QQmlTypePrivate *, const QMetaObject *, const QMetaObject *extMetaObject,
+                          QObject *(*)(QObject *)) {
+                          if (extMetaObject)
+                              ++extensionCount;
+                          return false;
+                      });
+
+    // don't generate proxies if there are no extensions
+    if (extensionCount == 0)
+        return {};
+
+    QList<QQmlProxyMetaObject::ProxyData> proxies;
+    QList<const QMetaObject *> proxyOrigins;
+
+    qsizetype proxyCount = 0;
+    auto createProxy = [&](QQmlTypePrivate *This, const QMetaObject *superdataBaseMetaObject,
+                           const QMetaObject *extMetaObject, QObject *(*extFunc)(QObject *)) {
+        if (extMetaObject) {
+            QMetaObjectBuilder builder;
+            clone(builder, extMetaObject, superdataBaseMetaObject, baseMetaObject,
+                  extFunc ? QQmlMetaType::CloneAll : QQmlMetaType::CloneEnumsOnly,
+                  QQmlMetaType::CloneTargetExtension);
+            QMetaObject *mmo = builder.toMetaObject();
+            proxies << QQmlProxyMetaObject::ProxyData { mmo, extFunc };
+            registerMetaObjectForType(mmo, This);
+            proxyOrigins << extMetaObject;
+
+            ++proxyCount;
+        }
+
+        // even if current QQmlType is not extended, its parent could be, in
+        // which case this type's meta object should come before the extension
+        // meta object of the parent
+        QMetaObjectBuilder builder;
+        clone(builder, superdataBaseMetaObject, superdataBaseMetaObject, superdataBaseMetaObject,
+              QQmlMetaType::CloneAll, QQmlMetaType::CloneTargetObject);
+        QMetaObject *baseClone = builder.toMetaObject();
+
+        const auto identity = [](QObject *object) { return object; };
+        proxies << QQmlProxyMetaObject::ProxyData {
+            baseClone, identity, 0, 0, 0, 0, QQmlProxyMetaObject::ProxyIsObject
+        };
+        registerMetaObjectForType(baseClone, This);
+        proxyOrigins << superdataBaseMetaObject;
+
+        return proxyCount == extensionCount; // early exit
+    };
+    iterateExtensions(data, type, mo, createProxy);
+
+    // bind the meta objects into a chain
+    for (qsizetype i = 0; i < proxies.size() - 1; ++i)
+        proxies[i].metaObject->d.superdata = proxies[i + 1].metaObject;
+    proxies.constLast().metaObject->d.superdata = baseMetaObject;
+
+    // once there is a proper meta object chain, populate the rest of the
+    // ProxyData structure
+    Q_ASSERT(proxies.size() == proxyOrigins.size());
+    for (int ii = 0; ii < proxies.count(); ++ii) {
+        auto &proxyData = proxies[ii];
+        proxyData.propertyOffset = proxyData.metaObject->propertyOffset();
+        proxyData.methodOffset = proxyData.metaObject->methodOffset();
+
+        const QMetaObject *origin = proxyOrigins[ii];
+        Q_ASSERT(origin);
+        proxyData.originPropertyOffset = origin->propertyOffset();
+        proxyData.originMethodOffset = origin->methodOffset();
     }
 
-    return metaObjects;
+    return proxies;
 }
 
 static bool isInternalType(int idx)
diff --git a/src/qml/qml/qqmlmetatype_p.h b/src/qml/qml/qqmlmetatype_p.h
index b9bf78b0a7..11e397da2a 100644
--- a/src/qml/qml/qqmlmetatype_p.h
+++ b/src/qml/qml/qqmlmetatype_p.h
@@ -256,17 +256,20 @@ public:
     static int registerUnitCacheHook(const QQmlPrivate::RegisterQmlUnitCacheHook &hookRegistration);
     static void clearTypeRegistrations();
 
-    static QList<QQmlProxyMetaObject::ProxyData> proxyData(const QMetaObject *mo,
-                                                           const QMetaObject *baseMetaObject,
-                                                           QMetaObject *lastMetaObject);
+    static QList<QQmlProxyMetaObject::ProxyData>
+    proxyData(QQmlTypePrivate *type, const QMetaObject *mo, const QMetaObject *baseMetaObject);
 
     enum ClonePolicy {
         CloneAll, // default
         CloneEnumsOnly, // skip properties and methods
     };
+    enum CloneTarget {
+        CloneTargetExtension,
+        CloneTargetObject,
+    };
     static void clone(QMetaObjectBuilder &builder, const QMetaObject *mo,
                       const QMetaObject *ignoreStart, const QMetaObject *ignoreEnd,
-                      ClonePolicy policy);
+                      ClonePolicy policy, CloneTarget target);
 
     static void qmlInsertModuleRegistration(const QString &uri, void (*registerFunction)());
     static void qmlRemoveModuleRegistration(const QString &uri);
diff --git a/src/qml/qml/qqmlproxymetaobject.cpp b/src/qml/qml/qqmlproxymetaobject.cpp
index 4844ca3720..81ef3c3976 100644
--- a/src/qml/qml/qqmlproxymetaobject.cpp
+++ b/src/qml/qml/qqmlproxymetaobject.cpp
@@ -78,9 +78,11 @@ QObject *QQmlProxyMetaObject::getProxy(int index)
             return nullptr;
 
         QObject *proxy = data.createFunc(object);
-        const QMetaObject *metaObject = proxy->metaObject();
         proxies[index] = proxy;
+        if (data.type == ProxyIsObject)
+            return proxies[index];
 
+        const QMetaObject *metaObject = proxy->metaObject();
         int localOffset = data.metaObject->methodOffset();
         int methodOffset = metaObject->methodOffset();
         int methods = metaObject->methodCount() - methodOffset;
@@ -114,7 +116,7 @@ int QQmlProxyMetaObject::metaCall(QObject *o, QMetaObject::Call c, int id, void
 
             QObject *proxy = getProxy(ii);
             Q_ASSERT(proxy);
-            const int localProxyOffset = proxy->metaObject()->propertyOffset();
+            const int localProxyOffset = metaObjects->at(ii).originPropertyOffset;
             const int localProxyId = id - globalPropertyOffset + localProxyOffset;
             return proxy->qt_metacall(c, localProxyId, a);
         }
@@ -137,7 +139,7 @@ int QQmlProxyMetaObject::metaCall(QObject *o, QMetaObject::Call c, int id, void
 
             QObject *proxy = getProxy(ii);
             Q_ASSERT(proxy);
-            const int localMethodOffset = proxy->metaObject()->methodOffset();
+            const int localMethodOffset = metaObjects->at(ii).originMethodOffset;
             const int localMethodId = id - globalMethodOffset + localMethodOffset;
             return proxy->qt_metacall(c, localMethodId, a);
         }
@@ -148,6 +150,17 @@ int QQmlProxyMetaObject::metaCall(QObject *o, QMetaObject::Call c, int id, void
         if ((id & ~MaxExtensionCount) != ExtensionObjectId)
             break;
         int index = id & MaxExtensionCount;
+
+        // map extension index to proxy index
+        int absoluteIndex = 0;
+        for (int i = -1; absoluteIndex < metaObjects->size(); ++absoluteIndex) {
+            if (metaObjects->at(absoluteIndex).type == ProxyIsExtension)
+                ++i;
+            if (i == index)
+                break;
+        }
+        index = absoluteIndex;
+
         if (qsizetype(index) >= metaObjects->size())
             break;
         a[0] = getProxy(index);
diff --git a/src/qml/qml/qqmlproxymetaobject_p.h b/src/qml/qml/qqmlproxymetaobject_p.h
index 965d86c70f..2815ac8423 100644
--- a/src/qml/qml/qqmlproxymetaobject_p.h
+++ b/src/qml/qml/qqmlproxymetaobject_p.h
@@ -65,12 +65,26 @@ QT_BEGIN_NAMESPACE
 class QQmlProxyMetaObject : public QDynamicMetaObjectData
 {
 public:
+    enum ProxyType {
+        ProxyIsExtension,
+        ProxyIsObject,
+    };
     struct ProxyData {
         typedef QObject *(*CreateFunc)(QObject *);
-        QMetaObject *metaObject;
-        CreateFunc createFunc;
-        int propertyOffset;
-        int methodOffset;
+        QMetaObject *metaObject = nullptr;
+        CreateFunc createFunc = nullptr; // function to create the proxy object
+
+        // precalculated offsets of metaObject member
+        int propertyOffset = 0;
+        int methodOffset = 0;
+
+        // cached offsets of metaObject's origin (metaObject is a clone of
+        // origin). unlike offsets above, these might not be available later
+        // during proxy calls since we lose the origin of a non-extension proxy
+        int originPropertyOffset = 0;
+        int originMethodOffset = 0;
+
+        ProxyType type = ProxyIsExtension;
     };
 
     QQmlProxyMetaObject(QObject *, QList<ProxyData> *);
diff --git a/src/qml/qml/qqmltype.cpp b/src/qml/qml/qqmltype.cpp
index 0bc817845d..fcf6e57d31 100644
--- a/src/qml/qml/qqmltype.cpp
+++ b/src/qml/qml/qqmltype.cpp
@@ -219,37 +219,8 @@ void QQmlTypePrivate::init() const
         return;
     }
 
-    auto setupExtendedMetaObject = [&](const QMetaObject *extMetaObject,
-                                       QObject *(*extFunc)(QObject *)) {
-        if (!extMetaObject)
-            return;
-
-        // XXX - very inefficient
-        QMetaObjectBuilder builder;
-        QQmlMetaType::clone(builder, extMetaObject, extMetaObject, extMetaObject,
-                            extFunc ? QQmlMetaType::CloneAll : QQmlMetaType::CloneEnumsOnly);
-        QMetaObject *mmo = builder.toMetaObject();
-        mmo->d.superdata = mo;
-        QQmlProxyMetaObject::ProxyData data = { mmo, extFunc, 0, 0 };
-        metaObjects << data;
-        QQmlMetaType::registerMetaObjectForType(mmo, const_cast<QQmlTypePrivate *>(this));
-    };
-
-    if (regType == QQmlType::SingletonType)
-        setupExtendedMetaObject(extraData.sd->extMetaObject, extraData.sd->extFunc);
-    else if (regType == QQmlType::CppType)
-        setupExtendedMetaObject(extraData.cd->extMetaObject, extraData.cd->extFunc);
-
-    metaObjects.append(QQmlMetaType::proxyData(
-            mo, baseMetaObject, metaObjects.isEmpty() ? nullptr
-                                                      : metaObjects.constLast().metaObject));
-
-    for (int ii = 0; ii < metaObjects.count(); ++ii) {
-        metaObjects[ii].propertyOffset =
-                metaObjects.at(ii).metaObject->propertyOffset();
-        metaObjects[ii].methodOffset =
-                metaObjects.at(ii).metaObject->methodOffset();
-    }
+    // init() is a const function, so const_cast `this`:
+    metaObjects = QQmlMetaType::proxyData(const_cast<QQmlTypePrivate *>(this), mo, baseMetaObject);
 
     // Check for revisioned details
     {
diff --git a/tests/auto/qml/qqmllanguage/testtypes.h b/tests/auto/qml/qqmllanguage/testtypes.h
index bcc6a9d9eb..f92548efb7 100644
--- a/tests/auto/qml/qqmllanguage/testtypes.h
+++ b/tests/auto/qml/qqmllanguage/testtypes.h
@@ -1836,8 +1836,12 @@ class ExtendedInParent : public MultiExtensionParent
     QML_ELEMENT
     // properties from base type: p, c, f
     // properties from base type's extension: a, c, d, f, g
+
+    Q_PROPERTY(int c READ c CONSTANT) // overwrite base type extension's property
 public:
     ExtendedInParent(QObject *parent = nullptr) : MultiExtensionParent(parent) { }
+
+    int c() { return 1111; }
 };
 
 class ExtendedByIndirect : public QObject
diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 73f886e217..83dd6a55c6 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -6509,9 +6509,14 @@ void tst_qqmllanguage::extensionSpecial()
         QVERIFY2(c.isReady(), qPrintable(c.errorString()));
         QScopedPointer<QObject> o(c.create());
         QVERIFY(o);
-
         // property a exists only in the extension type
         QCOMPARE(o->property("a").toInt(), int('a'));
+        // property c exists on the leaf type and that should be prioritized
+        // over the base type extension's one
+        QCOMPARE(o->property("c").toInt(), 1111);
+
+        o->setProperty("objectName", u"foobar"_s);
+        QCOMPARE(o->property("objectName").toString(), u"foobar"_s);
     }
 
     {