QQmlPropertyCache: Guarantee 1:1 relationship to meta object

The QQmlPropertyCache ctor that just takes a QMetaObject is really
dangerous. It misbehaves for anything but plain QObject. Remove it.

Also, realize that we never want to update a property cache
"recursively". That is, each property cache maps exactly one metaobject.
We cannot cover multiple metaobjects with the same property cache.

Finally, any property caches constructed dynamically must not be
recorded in the type registry. These caches are not comparable to
anything else. Introduce a special method to create them.

Fixes: QTBUG-102454
Change-Id: I47a1ff0f467e9444ff9f581ffcdf0a8b5730b0b8
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
(cherry picked from commit f7f6e140947582026d08a68421052e6ac7b997e4)
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>

10 files changed, 102 insertions, 51 deletions

diff --git a/src/qml/qml/qqmlmetatypedata.cpp b/src/qml/qml/qqmlmetatypedata.cpp
index 6bac4ca8c5..b2cae077cf 100644
--- a/src/qml/qml/qqmlmetatypedata.cpp
+++ b/src/qml/qml/qqmlmetatypedata.cpp
@@ -147,7 +147,7 @@ QQmlRefPointer<QQmlPropertyCache> QQmlMetaTypeData::propertyCache(const QMetaObj
     if (const QMetaObject *superMeta = metaObject->superClass())
         rv = propertyCache(superMeta, version)->copyAndAppend(metaObject, version);
     else
-        rv.adopt(new QQmlPropertyCache(metaObject));
+        rv = QQmlPropertyCache::createStandalone(metaObject);
 
     const auto *mop = reinterpret_cast<const QMetaObjectPrivate *>(metaObject->d.data);
     if (!(mop->flags & DynamicMetaObject))
diff --git a/src/qml/qml/qqmlopenmetaobject.cpp b/src/qml/qml/qqmlopenmetaobject.cpp
index 8b0e0c66d5..d40a2beaa7 100644
--- a/src/qml/qml/qqmlopenmetaobject.cpp
+++ b/src/qml/qml/qqmlopenmetaobject.cpp
@@ -40,6 +40,7 @@
 #include "qqmlopenmetaobject_p.h"
 #include <private/qqmlpropertycache_p.h>
 #include <private/qqmldata_p.h>
+#include <private/qqmlmetatype_p.h>
 #include <private/qmetaobjectbuilder_p.h>
 #include <qdebug.h>
 
@@ -423,8 +424,11 @@ void QQmlOpenMetaObject::setCached(bool c)
 
     QQmlData *qmldata = QQmlData::get(d->object, true);
     if (d->cacheProperties) {
+        // As the propertyCache is not saved in QQmlMetaType (due to it being dynamic)
+        // we cannot leak it to other places before we're done with it. Yes, it's still
+        // terrible.
         if (!d->type->d->cache)
-            d->type->d->cache = new QQmlPropertyCache(this);
+            d->type->d->cache = QQmlPropertyCache::createStandalone(this).take();
         qmldata->propertyCache = d->type->d->cache;
     } else {
         if (d->type->d->cache)
diff --git a/src/qml/qml/qqmlpropertycache.cpp b/src/qml/qml/qqmlpropertycache.cpp
index bb305549e0..69fa317495 100644
--- a/src/qml/qml/qqmlpropertycache.cpp
+++ b/src/qml/qml/qqmlpropertycache.cpp
@@ -149,14 +149,23 @@ void QQmlPropertyData::load(const QMetaMethod &m)
 }
 
 /*!
-Creates a new QQmlPropertyCache of \a metaObject.
+    Creates a standalone QQmlPropertyCache of \a metaObject. It is separate from the usual
+    QQmlPropertyCache hierarchy. Its parent is not equal to any other QQmlPropertyCache
+    created from QObject::staticMetaObject, for example.
 */
-QQmlPropertyCache::QQmlPropertyCache(const QMetaObject *metaObject, QTypeRevision metaObjectRevision)
-    : _metaObject(metaObject)
+QQmlRefPointer<QQmlPropertyCache> QQmlPropertyCache::createStandalone(
+        const QMetaObject *metaObject, QTypeRevision metaObjectRevision)
 {
     Q_ASSERT(metaObject);
 
-    update(metaObject);
+    QQmlRefPointer<QQmlPropertyCache> result;
+    if (const QMetaObject *super = metaObject->superClass()) {
+        result = createStandalone(
+                    super, metaObjectRevision)->copyAndAppend(metaObject, metaObjectRevision);
+    } else {
+        result.adopt(new QQmlPropertyCache(metaObject));
+        result->update(metaObject);
+    }
 
     if (metaObjectRevision.isValid() && metaObjectRevision != QTypeRevision::zero()) {
         // Set the revision of the meta object that this cache describes to be
@@ -164,9 +173,13 @@ QQmlPropertyCache::QQmlPropertyCache(const QMetaObject *metaObject, QTypeRevisio
         // from a type that was created directly in C++, and not through QML. For such
         // types, the revision for each recorded QMetaObject would normally be zero, which
         // would exclude any revisioned properties.
-        for (int metaObjectOffset = 0; metaObjectOffset < allowedRevisionCache.size(); ++metaObjectOffset)
-            allowedRevisionCache[metaObjectOffset] = metaObjectRevision;
+        for (int metaObjectOffset = 0; metaObjectOffset < result->allowedRevisionCache.size();
+             ++metaObjectOffset) {
+            result->allowedRevisionCache[metaObjectOffset] = metaObjectRevision;
+        }
     }
+
+    return result;
 }
 
 QQmlPropertyCache::~QQmlPropertyCache()
@@ -573,16 +586,6 @@ void QQmlPropertyCache::append(const QMetaObject *metaObject,
     }
 }
 
-void QQmlPropertyCache::updateRecur(const QMetaObject *metaObject)
-{
-    if (!metaObject)
-        return;
-
-    updateRecur(metaObject->superClass());
-
-    append(metaObject, QTypeRevision());
-}
-
 void QQmlPropertyCache::update(const QMetaObject *metaObject)
 {
     Q_ASSERT(metaObject);
@@ -602,7 +605,8 @@ void QQmlPropertyCache::update(const QMetaObject *metaObject)
     // cached in a parent cache.
     stringCache.reserve(pc + mc + sc);
 
-    updateRecur(metaObject);
+    if (metaObject)
+        append(metaObject, QTypeRevision());
 }
 
 /*! \internal
diff --git a/src/qml/qml/qqmlpropertycache_p.h b/src/qml/qml/qqmlpropertycache_p.h
index 60f3e1fa44..7be8bb93a6 100644
--- a/src/qml/qml/qqmlpropertycache_p.h
+++ b/src/qml/qml/qqmlpropertycache_p.h
@@ -151,8 +151,10 @@ private:
 class Q_QML_PRIVATE_EXPORT QQmlPropertyCache : public QQmlRefCount
 {
 public:
+    static QQmlRefPointer<QQmlPropertyCache> createStandalone(
+            const QMetaObject *, QTypeRevision metaObjectRevision = QTypeRevision::zero());
+
     QQmlPropertyCache() = default;
-    QQmlPropertyCache(const QMetaObject *, QTypeRevision metaObjectRevision = QTypeRevision::zero());
     ~QQmlPropertyCache() override;
 
     void update(const QMetaObject *);
@@ -272,8 +274,6 @@ private:
     QQmlPropertyData *findProperty(StringCache::ConstIterator it, const QQmlVMEMetaObject *,
                                    const QQmlRefPointer<QQmlContextData> &) const;
 
-    void updateRecur(const QMetaObject *);
-
     template<typename K>
     QQmlPropertyData *findNamedProperty(const K &key) const
     {
diff --git a/src/qmlmodels/qqmladaptormodel.cpp b/src/qmlmodels/qqmladaptormodel.cpp
index fe0ab6bcbc..e55845d3e2 100644
--- a/src/qmlmodels/qqmladaptormodel.cpp
+++ b/src/qmlmodels/qqmladaptormodel.cpp
@@ -549,7 +549,8 @@ public:
 
         metaObject.reset(builder.toMetaObject());
         *static_cast<QMetaObject *>(this) = *metaObject;
-        propertyCache.adopt(new QQmlPropertyCache(metaObject.data(), model.modelItemRevision));
+        propertyCache = QQmlPropertyCache::createStandalone(
+                    metaObject.data(), model.modelItemRevision);
     }
 };
 
@@ -683,8 +684,8 @@ public:
     {
         VDMListDelegateDataType *dataType = const_cast<VDMListDelegateDataType *>(this);
         if (!propertyCache) {
-            dataType->propertyCache.adopt(new QQmlPropertyCache(
-                        &QQmlDMListAccessorData::staticMetaObject, model.modelItemRevision));
+            dataType->propertyCache = QQmlPropertyCache::createStandalone(
+                        &QQmlDMListAccessorData::staticMetaObject, model.modelItemRevision);
         }
 
         return new QQmlDMListAccessorData(
diff --git a/src/quick/designer/qqmldesignermetaobject.cpp b/src/quick/designer/qqmldesignermetaobject.cpp
index 90fdbddabf..87234478aa 100644
--- a/src/quick/designer/qqmldesignermetaobject.cpp
+++ b/src/quick/designer/qqmldesignermetaobject.cpp
@@ -107,7 +107,7 @@ QQmlDesignerMetaObject* QQmlDesignerMetaObject::getNodeInstanceMetaObject(QObjec
     return mo;
 }
 
-void QQmlDesignerMetaObject::init(QObject *object, QQmlEngine *engine)
+void QQmlDesignerMetaObject::init(QObject *object, QQmlEngine *)
 {
     //Creating QQmlOpenMetaObjectType
     m_openMetaObject = std::make_unique<QQmlOpenMetaObject>(object, metaObjectParent());
@@ -118,7 +118,7 @@ void QQmlDesignerMetaObject::init(QObject *object, QQmlEngine *engine)
     QObjectPrivate *op = QObjectPrivate::get(object);
     op->metaObject = this;
 
-    cache = QQmlEnginePrivate::get(engine)->cache(metaObject);
+    cache = QQmlPropertyCache::createStandalone(metaObject);
 
     nodeInstanceMetaObjectList.insert(this, true);
 }
diff --git a/tests/auto/qml/qqmlopenmetaobject/tst_qqmlopenmetaobject.cpp b/tests/auto/qml/qqmlopenmetaobject/tst_qqmlopenmetaobject.cpp
index 6265c6ca67..27ecf24f26 100644
--- a/tests/auto/qml/qqmlopenmetaobject/tst_qqmlopenmetaobject.cpp
+++ b/tests/auto/qml/qqmlopenmetaobject/tst_qqmlopenmetaobject.cpp
@@ -52,7 +52,9 @@ void tst_qqmlopenmetaobject::createProperties()
 {
     QQmlEngine engine;
     CustomObject object;
-    const QQmlRefPointer<QQmlOpenMetaObjectType> mot = new QQmlOpenMetaObjectType(object.metaObject());
+    const QQmlRefPointer<QQmlOpenMetaObjectType> mot(
+                new QQmlOpenMetaObjectType(object.metaObject()),
+                QQmlRefPointer<QQmlOpenMetaObjectType>::Adopt);
     QQmlOpenMetaObject *const mo = new QQmlOpenMetaObject(&object, mot.data());
     mo->setCached(true);
     mot->createProperty("customProperty");
diff --git a/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp b/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp
index 912b34805e..7e1dc2f3e5 100644
--- a/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp
+++ b/tests/auto/qml/qqmlpropertycache/tst_qqmlpropertycache.cpp
@@ -200,9 +200,8 @@ void tst_qqmlpropertycache::properties()
     DerivedObject object;
     const QMetaObject *metaObject = object.metaObject();
 
-    QQmlRefPointer<QQmlPropertyCache> cache(new QQmlPropertyCache(metaObject),
-                                            QQmlRefPointer<QQmlPropertyCache>::Adopt);
-    QQmlPropertyData *data;
+    QQmlRefPointer<QQmlPropertyCache> cache = QQmlPropertyCache::createStandalone(metaObject);
+    const QQmlPropertyData *data;
 
     QVERIFY((data = cacheProperty(cache, "propertyA")));
     QCOMPARE(data->coreIndex(), metaObject->indexOfProperty("propertyA"));
@@ -223,12 +222,11 @@ void tst_qqmlpropertycache::propertiesDerived()
     DerivedObject object;
     const QMetaObject *metaObject = object.metaObject();
 
-    QQmlRefPointer<QQmlPropertyCache> parentCache(
-                new QQmlPropertyCache(&BaseObject::staticMetaObject),
-                QQmlRefPointer<QQmlPropertyCache>::Adopt);
-    QQmlRefPointer<QQmlPropertyCache> cache =
-            parentCache->copyAndAppend(object.metaObject(), QTypeRevision());
-    QQmlPropertyData *data;
+    QQmlRefPointer<QQmlPropertyCache> parentCache
+            = QQmlPropertyCache::createStandalone(&BaseObject::staticMetaObject);
+    QQmlRefPointer<QQmlPropertyCache> cache
+            = parentCache->copyAndAppend(object.metaObject(), QTypeRevision());
+    const QQmlPropertyData *data;
 
     QVERIFY((data = cacheProperty(cache, "propertyA")));
     QCOMPARE(data->coreIndex(), metaObject->indexOfProperty("propertyA"));
@@ -251,11 +249,11 @@ void tst_qqmlpropertycache::revisionedProperties()
     DerivedObject object;
     const QMetaObject *metaObject = object.metaObject();
 
-    QQmlRefPointer<QQmlPropertyCache> cacheWithoutVersion(new QQmlPropertyCache(metaObject),
-                                                          QQmlRefPointer<QQmlPropertyCache>::Adopt);
+    QQmlRefPointer<QQmlPropertyCache> cacheWithoutVersion(
+                QQmlPropertyCache::createStandalone(metaObject));
     QQmlRefPointer<QQmlPropertyCache> cacheWithVersion(
-                new QQmlPropertyCache(metaObject, QTypeRevision::fromMinorVersion(1)),
-                QQmlRefPointer<QQmlPropertyCache>::Adopt);
+                QQmlPropertyCache::createStandalone(
+                    metaObject, QTypeRevision::fromMinorVersion(1)));
     QQmlPropertyData *data;
 
     QVERIFY((data = cacheProperty(cacheWithoutVersion, "propertyE")));
@@ -269,9 +267,8 @@ void tst_qqmlpropertycache::methods()
     DerivedObject object;
     const QMetaObject *metaObject = object.metaObject();
 
-    QQmlRefPointer<QQmlPropertyCache> cache(new QQmlPropertyCache(metaObject),
-                                            QQmlRefPointer<QQmlPropertyCache>::Adopt);
-    QQmlPropertyData *data;
+    QQmlRefPointer<QQmlPropertyCache> cache(QQmlPropertyCache::createStandalone(metaObject));
+    const QQmlPropertyData *data;
 
     QVERIFY((data = cacheProperty(cache, "slotA")));
     QCOMPARE(data->coreIndex(), metaObject->indexOfMethod("slotA()"));
@@ -305,8 +302,7 @@ void tst_qqmlpropertycache::methodsDerived()
     const QMetaObject *metaObject = object.metaObject();
 
     QQmlRefPointer<QQmlPropertyCache> parentCache(
-                new QQmlPropertyCache(&BaseObject::staticMetaObject),
-                QQmlRefPointer<QQmlPropertyCache>::Adopt);
+                QQmlPropertyCache::createStandalone(&BaseObject::staticMetaObject));
     QQmlRefPointer<QQmlPropertyCache> cache
             = parentCache->copyAndAppend(object.metaObject(), QTypeRevision {});
     QQmlPropertyData *data;
@@ -342,9 +338,8 @@ void tst_qqmlpropertycache::signalHandlers()
     DerivedObject object;
     const QMetaObject *metaObject = object.metaObject();
 
-    QQmlRefPointer<QQmlPropertyCache> cache(new QQmlPropertyCache(metaObject),
-                                            QQmlRefPointer<QQmlPropertyCache>::Adopt);
-    QQmlPropertyData *data;
+    QQmlRefPointer<QQmlPropertyCache> cache(QQmlPropertyCache::createStandalone(metaObject));
+    const QQmlPropertyData *data;
 
     QVERIFY((data = cacheProperty(cache, "onSignalA")));
     QCOMPARE(data->coreIndex(), metaObject->indexOfMethod("signalA()"));
@@ -372,8 +367,7 @@ void tst_qqmlpropertycache::signalHandlersDerived()
     const QMetaObject *metaObject = object.metaObject();
 
     QQmlRefPointer<QQmlPropertyCache> parentCache(
-                new QQmlPropertyCache(&BaseObject::staticMetaObject),
-                QQmlRefPointer<QQmlPropertyCache>::Adopt);
+                QQmlPropertyCache::createStandalone(&BaseObject::staticMetaObject));
     QQmlRefPointer<QQmlPropertyCache> cache
             = parentCache->copyAndAppend(object.metaObject(), QTypeRevision{});
     QQmlPropertyData *data;
diff --git a/tests/auto/qml/qqmlpropertymap/data/cached.qml b/tests/auto/qml/qqmlpropertymap/data/cached.qml
new file mode 100644
index 0000000000..3222a672c3
--- /dev/null
+++ b/tests/auto/qml/qqmlpropertymap/data/cached.qml
@@ -0,0 +1,5 @@
+import QtQml
+
+QtObject {
+    property string text: map ? map.c : "yada"
+}
diff --git a/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp b/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp
index 508154e557..a6eca4825a 100644
--- a/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp
+++ b/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp
@@ -64,6 +64,7 @@ private slots:
     void QTBUG_48136();
     void lookupsInSubTypes();
     void freeze();
+    void cachedSignals();
 };
 
 class LazyPropertyMap : public QQmlPropertyMap, public QQmlParserStatus
@@ -595,6 +596,46 @@ void tst_QQmlPropertyMap::freeze()
     QCOMPARE(map.value("key1").toString(), QStringLiteral("Hello World"));
 }
 
+class Map: public QQmlPropertyMap
+{
+    Q_OBJECT
+public:
+    Map(QObject *parent = nullptr)
+        : QQmlPropertyMap(this, parent)
+    {
+        insert( "a", u"yayayaya"_qs );
+        insert( "b", u"yayayayb"_qs);
+        insert( "c", u"yayayayc"_qs);
+        insert( "d", u"yayayayd"_qs);
+
+        freeze();
+    }
+};
+
+void tst_QQmlPropertyMap::cachedSignals()
+{
+    Map foo;
+    QQmlEngine engine;
+    engine.rootContext()->setContextProperty("map", &foo);
+    const QUrl url = testFileUrl("cached.qml");
+    QQmlComponent c(&engine, url);
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+    QCOMPARE(o->property("text").toString(), u"yayayayc"_qs);
+    foo.setProperty("c", u"something"_qs);
+    QCOMPARE(o->property("text").toString(), u"something"_qs);
+    foo.insert("c", u"other"_qs);
+    QCOMPARE(o->property("text").toString(), u"other"_qs);
+    QTest::ignoreMessage(
+                QtWarningMsg,
+                qPrintable(url.toString() + u":4:5: Unable to assign [undefined] to QString"_qs));
+    foo.clear("c");
+    QCOMPARE(o->property("text").toString(), u"other"_qs);
+    foo.insert("c", u"final"_qs);
+    QCOMPARE(o->property("text").toString(), u"final"_qs);
+}
+
 QTEST_MAIN(tst_QQmlPropertyMap)
 
 #include "tst_qqmlpropertymap.moc"