Unify PropertyCache refcounting

We should not keep plain QQmlPropertyCache pointers around. Also
optimize self-assignment of QQmlRefPointer.

Change-Id: I0e30b4ce29bb6b7acf288a9dc7b515d0e8f4ddfe
Reviewed-by: Andrei Golubev <andrei.golubev@qt.io>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

9 files changed, 64 insertions, 49 deletions

diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 9e30225d3c..34994bd0e6 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -764,7 +764,7 @@ bool QObjectWrapper::virtualIsEqualTo(Managed *a, Managed *b)
 ReturnedValue QObjectWrapper::create(ExecutionEngine *engine, QObject *object)
 {
     if (QJSEngine *jsEngine = engine->jsEngine()) {
-        if (QQmlPropertyCache *cache = QQmlData::ensurePropertyCache(jsEngine, object).data()) {
+        if (QQmlRefPointer<QQmlPropertyCache> cache = QQmlData::ensurePropertyCache(jsEngine, object)) {
             ReturnedValue result = QV4::Encode::null();
             void *args[] = { &result, &engine };
             if (cache->callJSFactoryMethod(object, args))
diff --git a/src/qml/qml/ftw/qqmlrefcount_p.h b/src/qml/qml/ftw/qqmlrefcount_p.h
index 6c8bf20279..06314ef5c5 100644
--- a/src/qml/qml/ftw/qqmlrefcount_p.h
+++ b/src/qml/qml/ftw/qqmlrefcount_p.h
@@ -173,8 +173,12 @@ QQmlRefPointer<T>::~QQmlRefPointer()
 template<class T>
 QQmlRefPointer<T> &QQmlRefPointer<T>::operator=(const QQmlRefPointer<T> &other)
 {
-    if (other.o) other.o->addref();
-    if (o) o->release();
+    if (o == other.o)
+        return *this;
+    if (other.o)
+        other.o->addref();
+    if (o)
+        o->release();
     o = other.o;
     return *this;
 }
diff --git a/src/qml/qml/qqmlpropertycachecreator.cpp b/src/qml/qml/qqmlpropertycachecreator.cpp
index d542175647..6d3b8ca030 100644
--- a/src/qml/qml/qqmlpropertycachecreator.cpp
+++ b/src/qml/qml/qqmlpropertycachecreator.cpp
@@ -99,8 +99,11 @@ QByteArray QQmlPropertyCacheCreatorBase::createClassNameForInlineComponent(const
     return baseName;
 }
 
-QQmlBindingInstantiationContext::QQmlBindingInstantiationContext(int referencingObjectIndex, const QV4::CompiledData::Binding *instantiatingBinding,
-                                                                 const QString &instantiatingPropertyName, QQmlPropertyCache *referencingObjectPropertyCache)
+QQmlBindingInstantiationContext::QQmlBindingInstantiationContext(
+        int referencingObjectIndex,
+        const QV4::CompiledData::Binding *instantiatingBinding,
+        const QString &instantiatingPropertyName,
+        const QQmlRefPointer<QQmlPropertyCache> &referencingObjectPropertyCache)
     : referencingObjectIndex(referencingObjectIndex)
     , instantiatingBinding(instantiatingBinding)
     , instantiatingPropertyName(instantiatingPropertyName)
diff --git a/src/qml/qml/qqmlpropertycachecreator_p.h b/src/qml/qml/qqmlpropertycachecreator_p.h
index 78db3b243b..fc30b88889 100644
--- a/src/qml/qml/qqmlpropertycachecreator_p.h
+++ b/src/qml/qml/qqmlpropertycachecreator_p.h
@@ -75,10 +75,10 @@ inline QQmlError qQmlCompileError(const QV4::CompiledData::Location &location,
 
 struct QQmlBindingInstantiationContext {
     QQmlBindingInstantiationContext() {}
-    QQmlBindingInstantiationContext(int referencingObjectIndex,
-                                    const QV4::CompiledData::Binding *instantiatingBinding,
-                                    const QString &instantiatingPropertyName,
-                                    QQmlPropertyCache *referencingObjectPropertyCache);
+    QQmlBindingInstantiationContext(
+            int referencingObjectIndex, const QV4::CompiledData::Binding *instantiatingBinding,
+            const QString &instantiatingPropertyName,
+            const QQmlRefPointer<QQmlPropertyCache> &referencingObjectPropertyCache);
 
     bool resolveInstantiatingProperty();
     QQmlRefPointer<QQmlPropertyCache> instantiatingPropertyCache(QQmlEnginePrivate *enginePrivate) const;
@@ -340,7 +340,7 @@ inline QQmlError QQmlPropertyCacheCreator<ObjectContainer>::buildMetaObjectRecur
         }
     }
 
-    QQmlPropertyCache *thisCache = propertyCaches->at(objectIndex);
+    QQmlRefPointer<QQmlPropertyCache> thisCache = propertyCaches->at(objectIndex);
     auto binding = obj->bindingsBegin();
     auto end = obj->bindingsEnd();
     for (; binding != end; ++binding) {
@@ -837,7 +837,8 @@ inline void QQmlPropertyCacheAliasCreator<ObjectContainer>::appendAliasPropertie
             if (alias->encodedMetaPropertyIndex == -1)
                 continue;
 
-            const QQmlPropertyCache *targetCache = propertyCaches->at(targetObjectIndex);
+            const QQmlRefPointer<QQmlPropertyCache> targetCache
+                    = propertyCaches->at(targetObjectIndex);
             Q_ASSERT(targetCache);
 
             int coreIndex = QQmlPropertyIndex::fromEncoded(alias->encodedMetaPropertyIndex).coreIndex();
@@ -956,7 +957,7 @@ inline QQmlError QQmlPropertyCacheAliasCreator<ObjectContainer>::propertyDataFor
         int coreIndex = QQmlPropertyIndex::fromEncoded(alias.encodedMetaPropertyIndex).coreIndex();
         int valueTypeIndex = QQmlPropertyIndex::fromEncoded(alias.encodedMetaPropertyIndex).valueTypeIndex();
 
-        QQmlPropertyCache *targetCache = propertyCaches->at(targetObjectIndex);
+        QQmlRefPointer<QQmlPropertyCache> targetCache = propertyCaches->at(targetObjectIndex);
         Q_ASSERT(targetCache);
 
         QQmlPropertyData *targetProperty = targetCache->property(coreIndex);
@@ -1021,7 +1022,7 @@ inline QQmlError QQmlPropertyCacheAliasCreator<ObjectContainer>::appendAliasesTo
     if (!object.aliasCount())
         return QQmlError();
 
-    QQmlPropertyCache *propertyCache = propertyCaches->at(objectIndex);
+    QQmlRefPointer<QQmlPropertyCache> propertyCache = propertyCaches->at(objectIndex);
     Q_ASSERT(propertyCache);
 
     int effectiveSignalIndex = propertyCache->signalHandlerIndexCacheStart + propertyCache->propertyIndexCache.count();
diff --git a/src/qml/qml/qqmlpropertycachevector_p.h b/src/qml/qml/qqmlpropertycachevector_p.h
index 45fc2364df..5e4b84aec0 100644
--- a/src/qml/qml/qqmlpropertycachevector_p.h
+++ b/src/qml/qml/qqmlpropertycachevector_p.h
@@ -65,17 +65,17 @@ public:
         CacheNeedsVMEMetaObject
     };
 
-    QQmlPropertyCacheVector() {}
-    QQmlPropertyCacheVector(QQmlPropertyCacheVector &&other)
-        : data(std::move(other.data)) {}
-    QQmlPropertyCacheVector &operator=(QQmlPropertyCacheVector &&other) {
-        QVector<QTaggedPointer<QQmlPropertyCache, Tag>> moved(std::move(other.data));
-        data.swap(moved);
-        return *this;
-    }
+    QQmlPropertyCacheVector() = default;
+    QQmlPropertyCacheVector(QQmlPropertyCacheVector &&) = default;
+    QQmlPropertyCacheVector &operator=(QQmlPropertyCacheVector &&) = default;
 
     ~QQmlPropertyCacheVector() { clear(); }
-    void resize(int size) { return data.resize(size); }
+    void resize(int size)
+    {
+        Q_ASSERT(size >= data.size());
+        return data.resize(size);
+    }
+
     int count() const { return data.count(); }
     void clear()
     {
@@ -86,8 +86,11 @@ public:
         data.clear();
     }
 
-    void append(QQmlPropertyCache *cache) { cache->addref(); data.append(QTaggedPointer<QQmlPropertyCache, Tag>(cache)); }
-    QQmlPropertyCache *at(int index) const { return data.at(index).data(); }
+    void append(const QQmlRefPointer<QQmlPropertyCache> &cache) {
+        cache->addref();
+        data.append(QTaggedPointer<QQmlPropertyCache, Tag>(cache.data()));
+    }
+    QQmlRefPointer<QQmlPropertyCache> at(int index) const { return data.at(index).data(); }
     void set(int index, const QQmlRefPointer<QQmlPropertyCache> &replacement) {
         if (QQmlPropertyCache *oldCache = data.at(index).data()) {
             if (replacement.data() == oldCache)
diff --git a/src/qml/qml/qqmlpropertyvalidator.cpp b/src/qml/qml/qqmlpropertyvalidator.cpp
index 8e60afb57d..daaec2e3aa 100644
--- a/src/qml/qml/qqmlpropertyvalidator.cpp
+++ b/src/qml/qml/qqmlpropertyvalidator.cpp
@@ -114,7 +114,7 @@ QVector<QQmlError> QQmlPropertyValidator::validateObject(
         return validateObject(componentBinding->value.objectIndex, componentBinding);
     }
 
-    QQmlPropertyCache *propertyCache = propertyCaches.at(objectIndex);
+    QQmlRefPointer<QQmlPropertyCache> propertyCache = propertyCaches.at(objectIndex);
     if (!propertyCache)
         return QVector<QQmlError>();
 
@@ -659,7 +659,7 @@ QQmlError QQmlPropertyValidator::validateLiteralBinding(
     Returns true if from can be assigned to a (QObject) property of type
     to.
 */
-bool QQmlPropertyValidator::canCoerce(QMetaType to, QQmlPropertyCache *fromMo) const
+bool QQmlPropertyValidator::canCoerce(QMetaType to, QQmlRefPointer<QQmlPropertyCache> fromMo) const
 {
     QQmlRefPointer<QQmlPropertyCache> toMo = enginePrivate->rawPropertyCacheForType(to);
 
@@ -679,7 +679,7 @@ bool QQmlPropertyValidator::canCoerce(QMetaType to, QQmlPropertyCache *fromMo) c
     while (fromMo) {
         if (fromMo == toMo)
             return true;
-        fromMo = fromMo->parent().data();
+        fromMo = fromMo->parent();
     }
     return false;
 }
@@ -747,7 +747,7 @@ QQmlError QQmlPropertyValidator::validateObjectBinding(QQmlPropertyData *propert
     } else if (property->isQList()) {
         const QMetaType listType = QQmlMetaType::listType(property->propType());
         if (!QQmlMetaType::isInterface(listType)) {
-            QQmlPropertyCache *source = propertyCaches.at(binding->value.objectIndex);
+            QQmlRefPointer<QQmlPropertyCache> source = propertyCaches.at(binding->value.objectIndex);
             if (!canCoerce(listType, source)) {
                 return qQmlCompileError(binding->valueLocation, tr("Cannot assign object to list property \"%1\"").arg(propertyName));
             }
@@ -790,7 +790,7 @@ QQmlError QQmlPropertyValidator::validateObjectBinding(QQmlPropertyData *propert
             // Will be true if the assigned type inherits propertyMetaObject
             // Determine isAssignable value
             bool isAssignable = false;
-            QQmlPropertyCache *c = propertyCaches.at(binding->value.objectIndex);
+            QQmlRefPointer<QQmlPropertyCache> c = propertyCaches.at(binding->value.objectIndex);
             while (c && !isAssignable) {
                 isAssignable |= c == propertyMetaObject;
                 c = c->parent().data();
diff --git a/src/qml/qml/qqmlpropertyvalidator_p.h b/src/qml/qml/qqmlpropertyvalidator_p.h
index 554a7faff3..52b7699f6c 100644
--- a/src/qml/qml/qqmlpropertyvalidator_p.h
+++ b/src/qml/qml/qqmlpropertyvalidator_p.h
@@ -79,7 +79,7 @@ private:
             QQmlPropertyData *property, const QString &propertyName,
             const QV4::CompiledData::Binding *binding) const;
 
-    bool canCoerce(QMetaType to, QQmlPropertyCache *fromMo) const;
+    bool canCoerce(QMetaType to, QQmlRefPointer<QQmlPropertyCache> fromMo) const;
 
     Q_REQUIRED_RESULT QVector<QQmlError> recordError(
             const QV4::CompiledData::Location &location, const QString &description) const;
diff --git a/src/qml/qml/qqmltypecompiler.cpp b/src/qml/qml/qqmltypecompiler.cpp
index a9992d45ba..6c4e65aa57 100644
--- a/src/qml/qml/qqmltypecompiler.cpp
+++ b/src/qml/qml/qqmltypecompiler.cpp
@@ -316,7 +316,7 @@ bool SignalHandlerResolver::resolveSignalHandlerExpressions()
 {
     for (int objectIndex = 0; objectIndex < qmlObjects.count(); ++objectIndex) {
         const QmlIR::Object * const obj = qmlObjects.at(objectIndex);
-        QQmlPropertyCache *cache = propertyCaches->at(objectIndex);
+        QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches->at(objectIndex);
         if (!cache)
             continue;
         if (QQmlCustomParser *customParser = customParsers.value(obj->inheritedTypeNameIndex)) {
@@ -330,9 +330,9 @@ bool SignalHandlerResolver::resolveSignalHandlerExpressions()
     return true;
 }
 
-bool SignalHandlerResolver::resolveSignalHandlerExpressions(const QmlIR::Object *obj,
-                                                            const QString &typeName,
-                                                            QQmlPropertyCache *propertyCache)
+bool SignalHandlerResolver::resolveSignalHandlerExpressions(
+        const QmlIR::Object *obj, const QString &typeName,
+        const QQmlRefPointer<QQmlPropertyCache> &propertyCache)
 {
     // map from signal name defined in qml itself to list of parameters
     QHash<QString, QStringList> customSignals;
@@ -473,7 +473,7 @@ QQmlEnumTypeResolver::QQmlEnumTypeResolver(QQmlTypeCompiler *typeCompiler)
 bool QQmlEnumTypeResolver::resolveEnumBindings()
 {
     for (int i = 0; i < qmlObjects.count(); ++i) {
-        QQmlPropertyCache *propertyCache = propertyCaches->at(i);
+        QQmlRefPointer<QQmlPropertyCache> propertyCache = propertyCaches->at(i);
         if (!propertyCache)
             continue;
         const QmlIR::Object *obj = qmlObjects.at(i);
@@ -515,7 +515,9 @@ bool QQmlEnumTypeResolver::assignEnumToBinding(QmlIR::Binding *binding, QStringV
     return true;
 }
 
-bool QQmlEnumTypeResolver::tryQualifiedEnumAssignment(const QmlIR::Object *obj, const QQmlPropertyCache *propertyCache, const QQmlPropertyData *prop, QmlIR::Binding *binding)
+bool QQmlEnumTypeResolver::tryQualifiedEnumAssignment(
+        const QmlIR::Object *obj, const QQmlRefPointer<QQmlPropertyCache> &propertyCache,
+        const QQmlPropertyData *prop, QmlIR::Binding *binding)
 {
     bool isIntProp = (prop->propType().id() == QMetaType::Int) && !prop->isEnum();
     if (!prop->isEnum() && !isIntProp)
@@ -693,7 +695,7 @@ QQmlAliasAnnotator::QQmlAliasAnnotator(QQmlTypeCompiler *typeCompiler)
 void QQmlAliasAnnotator::annotateBindingsToAliases()
 {
     for (int i = 0; i < qmlObjects.count(); ++i) {
-        QQmlPropertyCache *propertyCache = propertyCaches->at(i);
+        QQmlRefPointer<QQmlPropertyCache> propertyCache = propertyCaches->at(i);
         if (!propertyCache)
             continue;
 
@@ -725,7 +727,7 @@ void QQmlScriptStringScanner::scan()
 {
     const QMetaType scriptStringMetaType = QMetaType::fromType<QQmlScriptString>();
     for (int i = 0; i < qmlObjects.count(); ++i) {
-        QQmlPropertyCache *propertyCache = propertyCaches->at(i);
+        QQmlRefPointer<QQmlPropertyCache> propertyCache = propertyCaches->at(i);
         if (!propertyCache)
             continue;
 
@@ -774,7 +776,8 @@ static bool isUsableComponent(const QMetaObject *metaObject)
     return false;
 }
 
-void QQmlComponentAndAliasResolver::findAndRegisterImplicitComponents(const QmlIR::Object *obj, QQmlPropertyCache *propertyCache)
+void QQmlComponentAndAliasResolver::findAndRegisterImplicitComponents(
+        const QmlIR::Object *obj, const QQmlRefPointer<QQmlPropertyCache> &propertyCache)
 {
     QQmlPropertyResolver propertyResolver(propertyCache);
 
@@ -854,7 +857,7 @@ void QQmlComponentAndAliasResolver::findAndRegisterImplicitComponents(const QmlI
         // Keep property caches symmetric
         QQmlRefPointer<QQmlPropertyCache> componentCache
                 = enginePrivate->cache(&QQmlComponent::staticMetaObject);
-        propertyCaches.append(componentCache.data());
+        propertyCaches.append(componentCache);
 
         QmlIR::Binding *syntheticBinding = pool->New<QmlIR::Binding>();
         *syntheticBinding = *binding;
@@ -891,7 +894,7 @@ bool QQmlComponentAndAliasResolver::resolve(int root)
                     obj->flags & QV4::CompiledData::Object::IsInlineComponentRoot)
                 break; // left current inline component (potentially entered a new one)
         }
-        QQmlPropertyCache *cache = propertyCaches.at(i);
+        QQmlRefPointer<QQmlPropertyCache> cache = propertyCaches.at(i);
         if (obj->inheritedTypeNameIndex == 0 && !cache)
             continue;
 
@@ -1108,7 +1111,7 @@ QQmlComponentAndAliasResolver::resolveAliasesInObject(int objectIndex,
         if (property.isEmpty()) {
             alias->flags |= QV4::CompiledData::Alias::AliasPointsToPointerObject;
         } else {
-            QQmlPropertyCache *targetCache = propertyCaches.at(targetObjectIndex);
+            QQmlRefPointer<QQmlPropertyCache> targetCache = propertyCaches.at(targetObjectIndex);
             if (!targetCache) {
                 *error = qQmlCompileError(
                         alias->referenceLocation,
@@ -1248,7 +1251,7 @@ bool QQmlDeferredAndCustomParserBindingScanner::scanObject(
         return scanObject(componentBinding->value.objectIndex, ScopeDeferred::False);
     }
 
-    QQmlPropertyCache *propertyCache = propertyCaches->at(objectIndex);
+    QQmlRefPointer<QQmlPropertyCache> propertyCache = propertyCaches->at(objectIndex);
     if (!propertyCache)
         return true;
 
@@ -1421,7 +1424,7 @@ void QQmlDefaultPropertyMerger::mergeDefaultProperties()
 
 void QQmlDefaultPropertyMerger::mergeDefaultProperties(int objectIndex)
 {
-    QQmlPropertyCache *propertyCache = propertyCaches->at(objectIndex);
+    QQmlRefPointer<QQmlPropertyCache> propertyCache = propertyCaches->at(objectIndex);
     if (!propertyCache)
         return;
 
diff --git a/src/qml/qml/qqmltypecompiler_p.h b/src/qml/qml/qqmltypecompiler_p.h
index e5d62f69de..557ec07aa4 100644
--- a/src/qml/qml/qqmltypecompiler_p.h
+++ b/src/qml/qml/qqmltypecompiler_p.h
@@ -195,7 +195,7 @@ public:
 
 private:
     bool resolveSignalHandlerExpressions(const QmlIR::Object *obj, const QString &typeName,
-                                         QQmlPropertyCache *propertyCache);
+                                         const QQmlRefPointer<QQmlPropertyCache> &propertyCache);
 
     QQmlEnginePrivate *enginePrivate;
     const QVector<QmlIR::Object*> &qmlObjects;
@@ -222,9 +222,9 @@ private:
     {
         return assignEnumToBinding(binding, QStringView(enumName), enumValue, isQtObject);
     }
-    bool tryQualifiedEnumAssignment(const QmlIR::Object *obj, const QQmlPropertyCache *propertyCache,
-                                    const QQmlPropertyData *prop,
-                                    QmlIR::Binding *binding);
+    bool tryQualifiedEnumAssignment(
+            const QmlIR::Object *obj, const QQmlRefPointer<QQmlPropertyCache> &propertyCache,
+            const QQmlPropertyData *prop, QmlIR::Binding *binding);
     int evaluateEnum(const QString &scope, QStringView enumName, QStringView enumValue, bool *ok) const;
 
 
@@ -280,7 +280,8 @@ public:
     bool resolve(int root = 0);
 
 protected:
-    void findAndRegisterImplicitComponents(const QmlIR::Object *obj, QQmlPropertyCache *propertyCache);
+    void findAndRegisterImplicitComponents(
+            const QmlIR::Object *obj, const QQmlRefPointer<QQmlPropertyCache> &propertyCache);
     bool collectIdsAndAliases(int objectIndex);
     bool resolveAliases(int componentIndex);
     void propertyDataForAlias(QmlIR::Alias *alias, int *type, quint32 *propertyFlags);