diff options
author | Simon Hausmann <simon.hausmann@qt.io> | 2019-05-20 16:09:13 +0200 |
---|---|---|
committer | Simon Hausmann <simon.hausmann@qt.io> | 2019-05-21 12:42:45 +0200 |
commit | 2f4479857d2b68f8cd3267b2f2b3c652ada431ed (patch) | |
tree | 588c84d73801025dcc878bc5b51e04a66f1d1c19 | |
parent | c61a75cd6a4a9cd7b3594b205fcac8fc1208d493 (diff) |
Fix lookups of properties in QML singletons
An unqualified name that points to a QML singleton will evaluate to a
QQmlTypeWrapper JS object. A member lookup in such an object is not
guaranteed to always produce the same property. The property cache check
may protect us from that, but we must still retrieve the QObject
singleton for every lookup.
Task-number: QTBUG-75896
Change-Id: Ibd9bac6e5c2047f838758811790b299ace636446
Reviewed-by: Ulf Hermann <ulf.hermann@qt.io>
-rw-r--r-- | src/qml/jsruntime/qv4lookup_p.h | 2 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4qmlcontext.cpp | 2 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4qobjectwrapper.cpp | 1 | ||||
-rw-r--r-- | src/qml/jsruntime/qv4qobjectwrapper_p.h | 3 | ||||
-rw-r--r-- | src/qml/qml/qqmltypewrapper.cpp | 47 | ||||
-rw-r--r-- | src/qml/qml/qqmltypewrapper_p.h | 2 | ||||
-rw-r--r-- | tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml | 14 | ||||
-rw-r--r-- | tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp | 50 |
8 files changed, 110 insertions, 11 deletions
diff --git a/src/qml/jsruntime/qv4lookup_p.h b/src/qml/jsruntime/qv4lookup_p.h index 03dc5f6d3c..4fd5e133f7 100644 --- a/src/qml/jsruntime/qv4lookup_p.h +++ b/src/qml/jsruntime/qv4lookup_p.h @@ -120,7 +120,7 @@ struct Lookup { } indexedLookup; struct { Heap::InternalClass *ic; - Heap::QObjectWrapper *staticQObject; + Heap::InternalClass *qmlTypeIc; // only used when lookup goes through QQmlTypeWrapper QQmlPropertyCache *propertyCache; QQmlPropertyData *propertyData; } qobjectLookup; diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp index 0c5226d46c..6aa0130188 100644 --- a/src/qml/jsruntime/qv4qmlcontext.cpp +++ b/src/qml/jsruntime/qv4qmlcontext.cpp @@ -293,7 +293,6 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r ScopedValue val(scope, base ? *base : Value::fromReturnedValue(QV4::QObjectWrapper::wrap(v4, scopeObject))); const QObjectWrapper *That = static_cast<const QObjectWrapper *>(val->objectValue()); lookup->qobjectLookup.ic = That->internalClass(); - lookup->qobjectLookup.staticQObject = nullptr; lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = propertyData; @@ -326,7 +325,6 @@ ReturnedValue QQmlContextWrapper::getPropertyAndBase(const QQmlContextWrapper *r ScopedValue val(scope, base ? *base : Value::fromReturnedValue(QV4::QObjectWrapper::wrap(v4, context->contextObject))); const QObjectWrapper *That = static_cast<const QObjectWrapper *>(val->objectValue()); lookup->qobjectLookup.ic = That->internalClass(); - lookup->qobjectLookup.staticQObject = nullptr; lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = propertyData; diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp index 6fed538fa8..63b6435112 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper.cpp +++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp @@ -870,7 +870,6 @@ ReturnedValue QObjectWrapper::virtualResolveLookupGetter(const Object *object, E } lookup->qobjectLookup.ic = This->internalClass(); - lookup->qobjectLookup.staticQObject = nullptr; lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = property; diff --git a/src/qml/jsruntime/qv4qobjectwrapper_p.h b/src/qml/jsruntime/qv4qobjectwrapper_p.h index 2558ede401..5543c4d5a6 100644 --- a/src/qml/jsruntime/qv4qobjectwrapper_p.h +++ b/src/qml/jsruntime/qv4qobjectwrapper_p.h @@ -233,8 +233,7 @@ inline ReturnedValue QObjectWrapper::lookupGetterImpl(Lookup *lookup, ExecutionE if (!o || o->internalClass != lookup->qobjectLookup.ic) return revertLookup(); - const Heap::QObjectWrapper *This = lookup->qobjectLookup.staticQObject ? lookup->qobjectLookup.staticQObject : - static_cast<const Heap::QObjectWrapper *>(o); + const Heap::QObjectWrapper *This = static_cast<const Heap::QObjectWrapper *>(o); QObject *qobj = This->object(); if (QQmlData::wasDeleted(qobj)) return QV4::Encode::undefined(); diff --git a/src/qml/qml/qqmltypewrapper.cpp b/src/qml/qml/qqmltypewrapper.cpp index 24c5aecc00..c5fa4a04ec 100644 --- a/src/qml/qml/qqmltypewrapper.cpp +++ b/src/qml/qml/qqmltypewrapper.cpp @@ -459,16 +459,16 @@ ReturnedValue QQmlTypeWrapper::virtualResolveLookupGetter(const Object *object, if (!includeEnums || !name->startsWithUpper()) { QQmlData *ddata = QQmlData::get(qobjectSingleton, false); if (ddata && ddata->propertyCache) { - ScopedValue val(scope, Value::fromReturnedValue(QV4::QObjectWrapper::wrap(engine, qobjectSingleton))); QQmlPropertyData *property = ddata->propertyCache->property(name.getPointer(), qobjectSingleton, qmlContext); if (property) { - lookup->qobjectLookup.ic = This->internalClass(); - lookup->qobjectLookup.staticQObject = static_cast<Heap::QObjectWrapper *>(val->heapObject()); + ScopedValue val(scope, Value::fromReturnedValue(QV4::QObjectWrapper::wrap(engine, qobjectSingleton))); + lookup->qobjectLookup.qmlTypeIc = This->internalClass(); + lookup->qobjectLookup.ic = val->objectValue()->internalClass(); lookup->qobjectLookup.propertyCache = ddata->propertyCache; lookup->qobjectLookup.propertyCache->addref(); lookup->qobjectLookup.propertyData = property; - lookup->getter = QV4::QObjectWrapper::lookupGetter; - return lookup->getter(lookup, engine, *This); + lookup->getter = QQmlTypeWrapper::lookupSingletonProperty; + return lookup->getter(lookup, engine, *object); } // Fall through to base implementation } @@ -488,6 +488,43 @@ bool QQmlTypeWrapper::virtualResolveLookupSetter(Object *object, ExecutionEngine return Object::virtualResolveLookupSetter(object, engine, lookup, value); } +ReturnedValue QQmlTypeWrapper::lookupSingletonProperty(Lookup *l, ExecutionEngine *engine, const Value &object) +{ + const auto revertLookup = [l, engine, &object]() { + l->qobjectLookup.propertyCache->release(); + l->qobjectLookup.propertyCache = nullptr; + l->getter = Lookup::getterGeneric; + return Lookup::getterGeneric(l, engine, object); + }; + + // we can safely cast to a QV4::Object here. If object is something else, + // the internal class won't match + Heap::Object *o = static_cast<Heap::Object *>(object.heapObject()); + if (!o || o->internalClass != l->qobjectLookup.qmlTypeIc) + return revertLookup(); + + Heap::QQmlTypeWrapper *This = static_cast<Heap::QQmlTypeWrapper *>(o); + + QQmlType type = This->type(); + if (!type.isValid()) + return revertLookup(); + + if (!type.isSingleton()) + return revertLookup(); + + QQmlEngine *e = engine->qmlEngine(); + QQmlType::SingletonInstanceInfo *siinfo = type.singletonInstanceInfo(); + siinfo->init(e); + + QObject *qobjectSingleton = siinfo->qobjectApi(e); + if (!qobjectSingleton) + return revertLookup(); + + Scope scope(engine); + ScopedValue obj(scope, QV4::QObjectWrapper::wrap(engine, qobjectSingleton)); + return QObjectWrapper::lookupGetterImpl(l, engine, obj, /*useOriginalProperty*/ true, revertLookup); +} + void Heap::QQmlScopedEnumWrapper::destroy() { QQmlType::derefHandle(typePrivate); diff --git a/src/qml/qml/qqmltypewrapper_p.h b/src/qml/qml/qqmltypewrapper_p.h index 44e82dec2b..5f3cb2523f 100644 --- a/src/qml/qml/qqmltypewrapper_p.h +++ b/src/qml/qml/qqmltypewrapper_p.h @@ -114,6 +114,8 @@ struct Q_QML_EXPORT QQmlTypeWrapper : Object static ReturnedValue virtualResolveLookupGetter(const Object *object, ExecutionEngine *engine, Lookup *lookup); static bool virtualResolveLookupSetter(Object *object, ExecutionEngine *engine, Lookup *lookup, const Value &value); + static ReturnedValue lookupSingletonProperty(Lookup *l, ExecutionEngine *engine, const Value &base); + protected: static ReturnedValue virtualGet(const Managed *m, PropertyKey id, const Value *receiver, bool *hasProperty); static bool virtualPut(Managed *m, PropertyKey id, const Value &value, Value *receiver); diff --git a/tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml b/tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml new file mode 100644 index 0000000000..3166ab647d --- /dev/null +++ b/tests/auto/qml/qqmlecmascript/data/SingletonLookupTest.qml @@ -0,0 +1,14 @@ +import QtQml 2.0 +import Test.Singletons 1.0 + +QtObject { + property Component singletonAccessor : Component { + QtObject { + property var singletonHolder; + property int result: singletonHolder.testVar + } + } + + property int firstLookup: singletonAccessor.createObject(this, { singletonHolder: CppSingleton1 }).result; + property int secondLookup: singletonAccessor.createObject(this, { singletonHolder: CppSingleton2 }).result; +} diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp index 007ad99655..c24c495b13 100644 --- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp +++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp @@ -369,6 +369,7 @@ private slots: void intMinDividedByMinusOne(); void undefinedPropertiesInObjectWrapper(); void hugeRegexpQuantifiers(); + void singletonTypeWrapperLookup(); private: // static void propertyVarWeakRefCallback(v8::Persistent<v8::Value> object, void* parameter); @@ -8979,6 +8980,55 @@ void tst_qqmlecmascript::hugeRegexpQuantifiers() QVERIFY(value.isRegExp()); } +struct CppSingleton1 : public QObject +{ + Q_OBJECT + Q_PROPERTY(int testVar MEMBER testVar CONSTANT) +public: + const int testVar = 0; +}; + +struct CppSingleton2 : public QObject +{ + Q_OBJECT + Q_PROPERTY(int testVar MEMBER testVar CONSTANT) +public: + const int testVar = 1; +}; + +void tst_qqmlecmascript::singletonTypeWrapperLookup() +{ + QQmlEngine engine; + + auto singletonTypeId1 = qmlRegisterSingletonType<CppSingleton1>("Test.Singletons", 1, 0, "CppSingleton1", + [](QQmlEngine *, QJSEngine *) -> QObject * { + return new CppSingleton1; + }); + + auto singletonTypeId2 = qmlRegisterSingletonType<CppSingleton2>("Test.Singletons", 1, 0, "CppSingleton2", + [](QQmlEngine *, QJSEngine *) -> QObject * { + return new CppSingleton2; + }); + + auto cleanup = qScopeGuard([&]() { + qmlUnregisterType(singletonTypeId1); + qmlUnregisterType(singletonTypeId2); + }); + + QQmlComponent component(&engine, testFileUrl("SingletonLookupTest.qml")); + QScopedPointer<QObject> test(component.create()); + QVERIFY2(!test.isNull(), qPrintable(component.errorString())); + + auto singleton1 = engine.singletonInstance<CppSingleton1*>(singletonTypeId1); + QVERIFY(singleton1); + + auto singleton2 = engine.singletonInstance<CppSingleton2*>(singletonTypeId2); + QVERIFY(singleton2); + + QCOMPARE(test->property("firstLookup").toInt(), singleton1->testVar); + QCOMPARE(test->property("secondLookup").toInt(), singleton2->testVar); +} + QTEST_MAIN(tst_qqmlecmascript) #include "tst_qqmlecmascript.moc" |