From c018df5b4075ae962966d4df7653d476dab02840 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Wed, 17 Apr 2019 08:38:20 +0200 Subject: QML: Remove static attchedPropertyIds map If the same object is available under two different names it should still have the same attached properties no matter which name you use. This was achieved by having a static map of metaobjects to attached property IDs that would always hold the first attached property ID registered for a given metaobject. This attached property ID was then used as key in the map of attached properties for the actual objects. The obvious downside to that is that we need a global static which gives us thread safety and static initialization (and destruction) problems. It turns out, all the attached properties are created by attached properties functions, registered by the user. Those functions only get the object to be amended as parameter. Therefore, no attached properties function can be registered for multiple attached properties on the same object as it wouldn't know which one to create for a given call. Thus, the whole ID dance is unnecessary as we can as well index the attached property objects by the function that created them. This nicely avoids creating two attached property objects for the same object and function and still makes the global static unnecessary. Fixes: QTBUG-75176 Change-Id: Ie8d53ef0a6f41c9b3d6b9d611cde1603a557901c Reviewed-by: Erik Verbruggen --- src/qml/qml/qqml.h | 9 ++++-- src/qml/qml/qqmldata_p.h | 3 +- src/qml/qml/qqmlengine.cpp | 60 ++++++++++++++++++++++++++++++--------- src/qml/qml/qqmlmetatype.cpp | 35 ++++++++--------------- src/qml/qml/qqmlmetatype_p.h | 2 ++ src/qml/qml/qqmlobjectcreator.cpp | 4 +-- src/qml/qml/qqmlprivate.h | 5 ++++ src/qml/qml/qqmlproperty.cpp | 4 +-- src/qml/qml/qqmltypewrapper.cpp | 7 +++-- 9 files changed, 83 insertions(+), 46 deletions(-) (limited to 'src/qml') diff --git a/src/qml/qml/qqml.h b/src/qml/qml/qqml.h index 05a9f70247..9eacc5bc22 100644 --- a/src/qml/qml/qqml.h +++ b/src/qml/qml/qqml.h @@ -582,6 +582,10 @@ namespace QtQml { Q_QML_EXPORT QObject *qmlAttachedPropertiesObjectById(int, const QObject *, bool create = true); Q_QML_EXPORT QObject *qmlAttachedPropertiesObject(int *, const QObject *, const QMetaObject *, bool create); + Q_QML_EXPORT QQmlAttachedPropertiesFunc qmlAttachedPropertiesFunction(QObject *, + const QMetaObject *); + Q_QML_EXPORT QObject *qmlAttachedPropertiesObject(QObject *, QQmlAttachedPropertiesFunc func, + bool create = true); #ifndef Q_QDOC } #endif @@ -601,8 +605,9 @@ Q_QML_EXPORT void qmlRegisterModule(const char *uri, int versionMajor, int versi template QObject *qmlAttachedPropertiesObject(const QObject *obj, bool create = true) { - static int idx = -1; - return qmlAttachedPropertiesObject(&idx, obj, &T::staticMetaObject, create); + QObject *mutableObj = const_cast(obj); + return qmlAttachedPropertiesObject( + mutableObj, qmlAttachedPropertiesFunction(mutableObj, &T::staticMetaObject), create); } Q_QML_EXPORT void qmlRegisterBaseTypes(const char *uri, int versionMajor, int versionMinor); diff --git a/src/qml/qml/qqmldata_p.h b/src/qml/qml/qqmldata_p.h index 2468de6857..f4c03fc17c 100644 --- a/src/qml/qml/qqmldata_p.h +++ b/src/qml/qml/qqmldata_p.h @@ -57,6 +57,7 @@ #include #include #include +#include #include #include @@ -265,7 +266,7 @@ public: } bool hasExtendedData() const { return extendedData != nullptr; } - QHash *attachedProperties() const; + QHash *attachedProperties() const; static inline bool wasDeleted(const QObject *); diff --git a/src/qml/qml/qqmlengine.cpp b/src/qml/qml/qqmlengine.cpp index 5841a480fc..7cd4bf8579 100644 --- a/src/qml/qml/qqmlengine.cpp +++ b/src/qml/qml/qqmlengine.cpp @@ -1647,29 +1647,38 @@ QQmlEngine *qmlEngine(const QObject *obj) return data->context->engine; } -QObject *qmlAttachedPropertiesObjectById(int id, const QObject *object, bool create) +static QObject *resolveAttachedProperties(QQmlAttachedPropertiesFunc pf, QQmlData *data, + QObject *object, bool create) { - QQmlData *data = QQmlData::get(object, create); - if (!data) - return nullptr; // Attached properties are only on objects created by QML, unless explicitly requested (create==true) + if (!pf) + return nullptr; - QObject *rv = data->hasExtendedData()?data->attachedProperties()->value(id):0; + QObject *rv = data->hasExtendedData() ? data->attachedProperties()->value(pf) : 0; if (rv || !create) return rv; - QQmlEnginePrivate *engine = QQmlEnginePrivate::get(data->context); - QQmlAttachedPropertiesFunc pf = QQmlMetaType::attachedPropertiesFuncById(engine, id); - if (!pf) - return nullptr; - - rv = pf(const_cast(object)); + rv = pf(object); if (rv) - data->attachedProperties()->insert(id, rv); + data->attachedProperties()->insert(pf, rv); return rv; } +QObject *qmlAttachedPropertiesObjectById(int id, const QObject *object, bool create) +{ + QQmlData *data = QQmlData::get(object, create); + + // Attached properties are only on objects created by QML, + // unless explicitly requested (create==true) + if (!data) + return nullptr; + + QQmlEnginePrivate *engine = QQmlEnginePrivate::get(data->context); + return resolveAttachedProperties(QQmlMetaType::attachedPropertiesFuncById(engine, id), data, + const_cast(object), create); +} + QObject *qmlAttachedPropertiesObject(int *idCache, const QObject *object, const QMetaObject *attachedMetaObject, bool create) { @@ -1684,6 +1693,29 @@ QObject *qmlAttachedPropertiesObject(int *idCache, const QObject *object, return qmlAttachedPropertiesObjectById(*idCache, object, create); } +QQmlAttachedPropertiesFunc qmlAttachedPropertiesFunction(QObject *object, + const QMetaObject *attachedMetaObject) +{ + QQmlEngine *engine = object ? qmlEngine(object) : nullptr; + return QQmlMetaType::attachedPropertiesFunc(engine ? QQmlEnginePrivate::get(engine) : nullptr, + attachedMetaObject); +} + +QObject *qmlAttachedPropertiesObject(QObject *object, QQmlAttachedPropertiesFunc func, bool create) +{ + if (!object) + return nullptr; + + QQmlData *data = QQmlData::get(object, create); + + // Attached properties are only on objects created by QML, + // unless explicitly requested (create==true) + if (!data) + return nullptr; + + return resolveAttachedProperties(func, data, object, create); +} + } // namespace QtQml #if QT_DEPRECATED_SINCE(5, 1) @@ -1724,7 +1756,7 @@ public: QQmlDataExtended(); ~QQmlDataExtended(); - QHash attachedProperties; + QHash attachedProperties; }; QQmlDataExtended::QQmlDataExtended() @@ -1870,7 +1902,7 @@ void QQmlData::disconnectNotifiers() } } -QHash *QQmlData::attachedProperties() const +QHash *QQmlData::attachedProperties() const { if (!extendedData) extendedData = new QQmlDataExtended; return &extendedData->attachedProperties; diff --git a/src/qml/qml/qqmlmetatype.cpp b/src/qml/qml/qqmlmetatype.cpp index f801e9aeba..1873a99a71 100644 --- a/src/qml/qml/qqmlmetatype.cpp +++ b/src/qml/qml/qqmlmetatype.cpp @@ -224,7 +224,6 @@ public: QQmlCustomParser *customParser; QQmlAttachedPropertiesFunc attachedPropertiesFunc; const QMetaObject *attachedPropertiesType; - int attachedPropertiesId; int propertyValueSourceCast; int propertyValueInterceptorCast; bool registerEnumClassesUnscoped; @@ -269,8 +268,6 @@ public: mutable QStringHash scopedEnumIndex; // maps from enum name to index in scopedEnums mutable QList*> scopedEnums; - static QHash attachedPropertyIds; - struct PropertyCacheByMinorVersion { PropertyCacheByMinorVersion() : cache(nullptr), minorVersion(-1) {} @@ -363,8 +360,6 @@ QJSValue QQmlType::SingletonInstanceInfo::scriptApi(QQmlEngine *e) const return scriptApis.value(e); } -QHash QQmlTypePrivate::attachedPropertyIds; - QQmlTypePrivate::QQmlTypePrivate(QQmlType::RegistrationType type) : refCount(1), regType(type), iid(nullptr), typeId(0), listId(0), revision(0), containsRevisionedAttributes(false), baseMetaObject(nullptr), @@ -498,14 +493,6 @@ QQmlType::QQmlType(QQmlMetaTypeData *data, const QString &elementName, const QQm d->baseMetaObject = type.metaObject; d->extraData.cd->attachedPropertiesFunc = type.attachedPropertiesFunction; d->extraData.cd->attachedPropertiesType = type.attachedPropertiesMetaObject; - if (d->extraData.cd->attachedPropertiesType) { - auto iter = QQmlTypePrivate::attachedPropertyIds.find(d->baseMetaObject); - if (iter == QQmlTypePrivate::attachedPropertyIds.end()) - iter = QQmlTypePrivate::attachedPropertyIds.insert(d->baseMetaObject, d->index); - d->extraData.cd->attachedPropertiesId = *iter; - } else { - d->extraData.cd->attachedPropertiesId = -1; - } d->extraData.cd->parserStatusCast = type.parserStatusCast; d->extraData.cd->propertyValueSourceCast = type.valueSourceCast; d->extraData.cd->propertyValueInterceptorCast = type.valueInterceptorCast; @@ -571,16 +558,8 @@ QQmlType::QQmlType(QQmlTypePrivate *priv) QQmlType::~QQmlType() { - if (d && !d->refCount.deref()) { - // If attached properties were successfully registered, deregister them. - // (They may not have been registered if some other type used the same baseMetaObject) - if (d->regType == CppType && d->extraData.cd->attachedPropertiesType) { - auto it = QQmlTypePrivate::attachedPropertyIds.find(d->baseMetaObject); - if (it != QQmlTypePrivate::attachedPropertyIds.end() && *it == d->index) - QQmlTypePrivate::attachedPropertyIds.erase(it); - } + if (d && !d->refCount.deref()) delete d; - } } QHashedString QQmlType::module() const @@ -1228,7 +1207,7 @@ int QQmlType::attachedPropertiesId(QQmlEnginePrivate *engine) const if (!d) return -1; if (d->regType == CppType) - return d->extraData.cd->attachedPropertiesId; + return d->extraData.cd->attachedPropertiesType ? d->index : -1; QQmlType base; if (d->regType == CompositeType) @@ -2188,6 +2167,16 @@ QQmlAttachedPropertiesFunc QQmlMetaType::attachedPropertiesFuncById(QQmlEnginePr return data->types.at(id).attachedPropertiesFunction(engine); } +QQmlAttachedPropertiesFunc QQmlMetaType::attachedPropertiesFunc(QQmlEnginePrivate *engine, + const QMetaObject *mo) +{ + QMutexLocker lock(metaTypeDataLock()); + QQmlMetaTypeData *data = metaTypeData(); + + QQmlType type(data->metaObjectToType.value(mo)); + return type.attachedPropertiesFunction(engine); +} + QMetaProperty QQmlMetaType::defaultProperty(const QMetaObject *metaObject) { int idx = metaObject->indexOfClassInfo("DefaultProperty"); diff --git a/src/qml/qml/qqmlmetatype_p.h b/src/qml/qml/qqmlmetatype_p.h index 8256212207..3c1589d19b 100644 --- a/src/qml/qml/qqmlmetatype_p.h +++ b/src/qml/qml/qqmlmetatype_p.h @@ -119,6 +119,8 @@ public: static int listType(int); static int attachedPropertiesFuncId(QQmlEnginePrivate *engine, const QMetaObject *); static QQmlAttachedPropertiesFunc attachedPropertiesFuncById(QQmlEnginePrivate *, int); + static QQmlAttachedPropertiesFunc attachedPropertiesFunc(QQmlEnginePrivate *, + const QMetaObject *); enum TypeCategory { Unknown, Object, List }; static TypeCategory typeCategory(int); diff --git a/src/qml/qml/qqmlobjectcreator.cpp b/src/qml/qml/qqmlobjectcreator.cpp index 5af658194f..b62b07e39c 100644 --- a/src/qml/qml/qqmlobjectcreator.cpp +++ b/src/qml/qml/qqmlobjectcreator.cpp @@ -793,8 +793,8 @@ bool QQmlObjectCreator::setPropertyBinding(const QQmlPropertyData *bindingProper else return false; } - const int id = attachedType.attachedPropertiesId(QQmlEnginePrivate::get(engine)); - QObject *qmlObject = qmlAttachedPropertiesObjectById(id, _qobject); + QObject *qmlObject = qmlAttachedPropertiesObject( + _qobject, attachedType.attachedPropertiesFunction(QQmlEnginePrivate::get(engine))); if (!populateInstance(binding->value.objectIndex, qmlObject, qmlObject, /*value type property*/nullptr)) return false; return true; diff --git a/src/qml/qml/qqmlprivate.h b/src/qml/qml/qqmlprivate.h index c0232a7691..fa05b3fe19 100644 --- a/src/qml/qml/qqmlprivate.h +++ b/src/qml/qml/qqmlprivate.h @@ -77,6 +77,11 @@ typedef void (*IRLoaderFunction)(Document *, const QQmlPrivate::CachedQmlUnit *) typedef QObject *(*QQmlAttachedPropertiesFunc)(QObject *); +inline uint qHash(QQmlAttachedPropertiesFunc func, uint seed = 0) +{ + return qHash(quintptr(func), seed); +} + template class QQmlTypeInfo { diff --git a/src/qml/qml/qqmlproperty.cpp b/src/qml/qml/qqmlproperty.cpp index 000b88ebaa..c8166695ba 100644 --- a/src/qml/qml/qqmlproperty.cpp +++ b/src/qml/qml/qqmlproperty.cpp @@ -277,7 +277,7 @@ void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name) QQmlAttachedPropertiesFunc func = r.type.attachedPropertiesFunction(enginePrivate); if (!func) return; // Not an attachable type - currentObject = qmlAttachedPropertiesObjectById(r.type.attachedPropertiesId(enginePrivate), currentObject); + currentObject = qmlAttachedPropertiesObject(currentObject, func); if (!currentObject) return; // Something is broken with the attachable type } else if (r.importNamespace) { if ((ii + 1) == path.count()) return; // No type following the namespace @@ -289,7 +289,7 @@ void QQmlPropertyPrivate::initProperty(QObject *obj, const QString &name) QQmlAttachedPropertiesFunc func = r.type.attachedPropertiesFunction(enginePrivate); if (!func) return; // Not an attachable type - currentObject = qmlAttachedPropertiesObjectById(r.type.attachedPropertiesId(enginePrivate), currentObject); + currentObject = qmlAttachedPropertiesObject(currentObject, func); if (!currentObject) return; // Something is broken with the attachable type } else if (r.scriptIndex != -1) { diff --git a/src/qml/qml/qqmltypewrapper.cpp b/src/qml/qml/qqmltypewrapper.cpp index d30c225741..246de04316 100644 --- a/src/qml/qml/qqmltypewrapper.cpp +++ b/src/qml/qml/qqmltypewrapper.cpp @@ -263,7 +263,9 @@ ReturnedValue QQmlTypeWrapper::virtualGet(const Managed *m, PropertyKey id, cons // Fall through to base implementation } else if (w->d()->object) { - QObject *ao = qmlAttachedPropertiesObjectById(type.attachedPropertiesId(QQmlEnginePrivate::get(v4->qmlEngine())), object); + QObject *ao = qmlAttachedPropertiesObject( + object, + type.attachedPropertiesFunction(QQmlEnginePrivate::get(v4->qmlEngine()))); if (ao) return QV4::QObjectWrapper::getQmlProperty(v4, context, ao, name, QV4::QObjectWrapper::IgnoreRevision, hasProperty); @@ -335,7 +337,8 @@ bool QQmlTypeWrapper::virtualPut(Managed *m, PropertyKey id, const Value &value, if (type.isValid() && !type.isSingleton() && w->d()->object) { QObject *object = w->d()->object; QQmlEngine *e = scope.engine->qmlEngine(); - QObject *ao = qmlAttachedPropertiesObjectById(type.attachedPropertiesId(QQmlEnginePrivate::get(e)), object); + QObject *ao = qmlAttachedPropertiesObject( + object, type.attachedPropertiesFunction(QQmlEnginePrivate::get(e))); if (ao) return QV4::QObjectWrapper::setQmlProperty(scope.engine, context, ao, name, QV4::QObjectWrapper::IgnoreRevision, value); return false; -- cgit v1.2.3