QML: Unify treatment of wrappers when dealing with QObjectMethod

A method can belong to a QObjectWrapper, a QQmlValueTypeWrapper, or a
QQmlTypeWrapper. store only one wrapper pointer and allow for all
variants. Furthermore, keep a reference to the wrapper so that it
doesn't get garbage collected. We still need it to retrieve the
metaobject, even if we're calling the method on a different instance.

Pick-to: 6.2 6.5 6.6
Fixes: QTBUG-115115
Change-Id: I1759fb687918ff79829fef776e0a93d29373b30f
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>
This commit is contained in:
Ulf Hermann 2023-07-07 13:50:07 +02:00
parent 1ab49e0d6f
commit 63b622d590
5 changed files with 212 additions and 106 deletions

View File

@ -263,7 +263,7 @@ ReturnedValue QObjectWrapper::getProperty(
if (!global)
global = engine->rootContext();
return QObjectMethod::create(
global, (flags & AttachMethods) ? object : nullptr, property->coreIndex());
global, (flags & AttachMethods) ? wrapper : nullptr, property->coreIndex());
} else if (property->isSignalHandler()) {
QmlSignalHandler::initProto(engine);
return engine->memoryManager->allocate<QmlSignalHandler>(
@ -271,7 +271,7 @@ ReturnedValue QObjectWrapper::getProperty(
} else {
ExecutionContext *global = engine->rootContext();
return QObjectMethod::create(
global, (flags & AttachMethods) ? object : nullptr, property->coreIndex());
global, (flags & AttachMethods) ? wrapper : nullptr, property->coreIndex());
}
}
@ -290,7 +290,8 @@ ReturnedValue QObjectWrapper::getProperty(
}
}
static OptionalReturnedValue getDestroyOrToStringMethod(ExecutionEngine *v4, String *name, QObject *qobj, bool *hasProperty = nullptr)
static OptionalReturnedValue getDestroyOrToStringMethod(
ExecutionEngine *v4, String *name, Heap::Object *qobj, bool *hasProperty = nullptr)
{
int index = 0;
if (name->equals(v4->id_destroy()))
@ -347,7 +348,7 @@ ReturnedValue QObjectWrapper::getQmlProperty(
ExecutionEngine *v4 = engine();
if (auto methodValue = getDestroyOrToStringMethod(v4, name, d()->object(), hasProperty))
if (auto methodValue = getDestroyOrToStringMethod(v4, name, d(), hasProperty))
return *methodValue;
QQmlPropertyData local;
@ -390,7 +391,7 @@ ReturnedValue QObjectWrapper::getQmlProperty(
return Encode::null();
}
if (auto methodValue = getDestroyOrToStringMethod(engine, name, object, hasProperty))
if (auto methodValue = getDestroyOrToStringMethod(engine, name, wrapper, hasProperty))
return *methodValue;
QQmlData *ddata = QQmlData::get(object, false);
@ -736,15 +737,6 @@ ReturnedValue QObjectWrapper::wrapConst_slowPath(ExecutionEngine *engine, QObjec
return constWrapper.asReturnedValue();
}
Heap::QObjectMethod *QObjectWrapper::cloneMethod(
ExecutionEngine *engine, Heap::QObjectMethod *cloneFrom,
Heap::Object *wrapper, QObject *object)
{
Scope scope(engine);
Scoped<QObjectMethod> method(scope, QObjectMethod::create(engine, cloneFrom, wrapper, object));
return method ? method->d() : nullptr;
}
void QObjectWrapper::markWrapper(QObject *object, MarkStack *markStack)
{
if (QQmlData::wasDeleted(object))
@ -984,7 +976,7 @@ ReturnedValue QObjectWrapper::virtualResolveLookupGetter(const Object *object, E
return Encode::undefined();
QQmlData *ddata = QQmlData::get(qobj, false);
if (auto methodValue = getDestroyOrToStringMethod(engine, name, qobj)) {
if (auto methodValue = getDestroyOrToStringMethod(engine, name, This->d())) {
Scoped<QObjectMethod> method(scope, *methodValue);
setupQObjectMethodLookup(
lookup, ddata ? ddata : QQmlData::get(qobj, true), nullptr, This, method->d());
@ -2312,34 +2304,33 @@ ReturnedValue CallArgument::toValue(ExecutionEngine *engine)
return Encode::undefined();
}
ReturnedValue QObjectMethod::create(ExecutionContext *scope, QObject *object, int index)
ReturnedValue QObjectMethod::create(ExecutionContext *scope, Heap::Object *wrapper, int index)
{
Scope valueScope(scope);
Scoped<QObjectMethod> method(valueScope, valueScope.engine->memoryManager->allocate<QObjectMethod>(scope));
method->d()->setObject(object);
method->d()->index = index;
Scoped<QObjectMethod> method(
valueScope,
valueScope.engine->memoryManager->allocate<QObjectMethod>(scope, wrapper, index));
return method.asReturnedValue();
}
ReturnedValue QObjectMethod::create(ExecutionContext *scope, Heap::QQmlValueTypeWrapper *valueType, int index)
{
Scope valueScope(scope);
Scoped<QObjectMethod> method(valueScope, valueScope.engine->memoryManager->allocate<QObjectMethod>(scope));
method->d()->index = index;
method->d()->valueTypeWrapper.set(valueScope.engine, valueType);
Scoped<QObjectMethod> method(
valueScope,
valueScope.engine->memoryManager->allocate<QObjectMethod>(scope, valueType, index));
return method.asReturnedValue();
}
ReturnedValue QObjectMethod::create(
ExecutionEngine *engine, Heap::QObjectMethod *cloneFrom,
Heap::Object *wrapper, QObject *object)
Heap::Object *wrapper, Heap::Object *object)
{
Scope valueScope(engine);
Scoped<QQmlValueTypeWrapper> valueTypeWrapper(valueScope);
if (cloneFrom->valueTypeWrapper) {
Scoped<QQmlValueTypeWrapper> ref(valueScope, cloneFrom->valueTypeWrapper);
if (cloneFrom->wrapper) {
Scoped<QQmlValueTypeWrapper> ref(valueScope, cloneFrom->wrapper);
if (ref) {
valueTypeWrapper = QQmlValueTypeWrapper::create(engine, ref->d(), wrapper);
} else {
@ -2352,16 +2343,12 @@ ReturnedValue QObjectMethod::create(
Scoped<ExecutionContext> context(valueScope, cloneFrom->scope.get());
Scoped<QObjectMethod> method(
valueScope, engine->memoryManager->allocate<QV4::QObjectMethod>(context));
valueScope,
engine->memoryManager->allocate<QV4::QObjectMethod>(
context, valueTypeWrapper ? valueTypeWrapper->d() : object, cloneFrom->index));
method->d()->index = cloneFrom->index;
method->d()->methodCount = cloneFrom->methodCount;
if (valueTypeWrapper)
method->d()->valueTypeWrapper.set(engine, valueTypeWrapper->d());
else
method->d()->setObject(object);
Q_ASSERT(method->d()->methods == nullptr);
switch (cloneFrom->methodCount) {
case 0:
@ -2384,50 +2371,70 @@ ReturnedValue QObjectMethod::create(
return method.asReturnedValue();
}
void Heap::QObjectMethod::init(QV4::ExecutionContext *scope)
void Heap::QObjectMethod::init(QV4::ExecutionContext *scope, Object *object, int methodIndex)
{
Heap::FunctionObject::init(scope);
wrapper.set(internalClass->engine, object);
index = methodIndex;
}
const QMetaObject *Heap::QObjectMethod::metaObject() const
{
if (valueTypeWrapper)
return valueTypeWrapper->metaObject();
if (QObject *self = object())
return self->metaObject();
Scope scope(internalClass->engine);
if (Scoped<QV4::QObjectWrapper> objectWrapper(scope, wrapper); objectWrapper)
return objectWrapper->metaObject();
if (Scoped<QV4::QQmlTypeWrapper> typeWrapper(scope, wrapper); typeWrapper)
return typeWrapper->metaObject();
if (Scoped<QV4::QQmlValueTypeWrapper> valueWrapper(scope, wrapper); valueWrapper)
return valueWrapper->metaObject();
return nullptr;
}
QObject *Heap::QObjectMethod::object() const
{
Scope scope(internalClass->engine);
if (Scoped<QV4::QObjectWrapper> objectWrapper(scope, wrapper); objectWrapper)
return objectWrapper->object();
if (Scoped<QV4::QQmlTypeWrapper> typeWrapper(scope, wrapper); typeWrapper)
return typeWrapper->object();
return nullptr;
}
bool Heap::QObjectMethod::isDetached() const
{
if (qObj.isValid())
return false;
if (Heap::QQmlValueTypeWrapper *wrapper = valueTypeWrapper.get())
return wrapper->object() == nullptr;
if (!wrapper)
return true;
QV4::Scope scope(internalClass->engine);
if (Scoped<QV4::QQmlValueTypeWrapper> valueWrapper(scope, wrapper); valueWrapper)
return valueWrapper->d()->object() == nullptr;
return false;
}
bool Heap::QObjectMethod::isAttachedTo(QObject *o) const
{
if (qObj.isValid() && qObj != o)
return false;
QV4::Scope scope(internalClass->engine);
if (Scoped<QV4::QObjectWrapper> objectWrapper(scope, wrapper); objectWrapper)
return objectWrapper->object() == o;
if (Scoped<QV4::QQmlTypeWrapper> typeWrapper(scope, wrapper); typeWrapper)
return typeWrapper->object() == o;
if (Heap::QQmlValueTypeWrapper *wrapper = valueTypeWrapper.get()) {
if (Scoped<QV4::QQmlValueTypeWrapper> valueWrapper(scope, wrapper); valueWrapper) {
QV4::Scope scope(wrapper->internalClass->engine);
QV4::Scoped<QV4::QObjectWrapper> qobject(scope, wrapper->object());
if (qobject && qobject->object() == o)
return true;
QV4::Scoped<QV4::QQmlTypeWrapper> type(scope, wrapper->object());
if (type && type->object() == o)
return true;
if (QV4::Scoped<QV4::QObjectWrapper> qobject(scope, valueWrapper->d()->object()); qobject)
return qobject->object() == o;
if (QV4::Scoped<QV4::QQmlTypeWrapper> type(scope, valueWrapper->d()->object()); type)
return type->object() == o;
// Attached to some nested value type or sequence object
return false;
}
return true;
return false;
}
Heap::QObjectMethod::ThisObjectMode Heap::QObjectMethod::checkThisObject(
@ -2437,7 +2444,7 @@ Heap::QObjectMethod::ThisObjectMode Heap::QObjectMethod::checkThisObject(
if (!thisMeta) {
// You can only get a detached method via a lookup, and then you have a thisObject.
Q_ASSERT(valueTypeWrapper || qObj);
Q_ASSERT(wrapper);
return Included;
}
@ -2476,11 +2483,8 @@ Heap::QObjectMethod::ThisObjectMode Heap::QObjectMethod::checkThisObject(
Q_UNREACHABLE_RETURN(Invalid);
};
if (QObject *o = object())
return check(o->metaObject());
if (valueTypeWrapper)
return check(valueTypeWrapper->metaObject());
if (const QMetaObject *meta = metaObject())
return check(meta);
// If the QObjectMethod is detached, we can only have gotten here via a lookup.
// The lookup checks that the QQmlPropertyCache matches.
@ -2554,15 +2558,6 @@ void Heap::QObjectMethod::ensureMethodsCache(const QMetaObject *thisMeta)
Q_ASSERT(methodCount > 0);
}
static QObject *qObject(const Value *v)
{
if (const QObjectWrapper *o = v->as<QObjectWrapper>())
return o->object();
if (const QQmlTypeWrapper *t = v->as<QQmlTypeWrapper>())
return t->object();
return nullptr;
}
ReturnedValue QObjectMethod::method_toString(ExecutionEngine *engine, QObject *o) const
{
return engine->newString(
@ -2603,20 +2598,24 @@ ReturnedValue QObjectMethod::callInternal(const Value *thisObject, const Value *
const QMetaObject *thisMeta = nullptr;
QObject *o = qObject(thisObject);
Heap::QQmlValueTypeWrapper *wrapper = nullptr;
if (o) {
thisMeta = o->metaObject();
} else if (const QQmlValueTypeWrapper *value = thisObject->as<QQmlValueTypeWrapper>()) {
wrapper = value->d();
thisMeta = wrapper->metaObject();
QObject *o = nullptr;
Heap::QQmlValueTypeWrapper *valueWrapper = nullptr;
if (const QObjectWrapper *w = thisObject->as<QObjectWrapper>()) {
thisMeta = w->metaObject();
o = w->object();
} else if (const QQmlTypeWrapper *w = thisObject->as<QQmlTypeWrapper>()) {
thisMeta = w->metaObject();
o = w->object();
} else if (const QQmlValueTypeWrapper *w = thisObject->as<QQmlValueTypeWrapper>()) {
thisMeta = w->metaObject();
valueWrapper = w->d();
}
Heap::QObjectMethod::ThisObjectMode mode = Heap::QObjectMethod::Invalid;
if (o && o == d()->object()) {
mode = Heap::QObjectMethod::Explicit;
// Nothing to do; objects are the same. This should be common
} else if (wrapper && wrapper == d()->valueTypeWrapper) {
} else if (valueWrapper && valueWrapper == d()->wrapper) {
mode = Heap::QObjectMethod::Explicit;
// Nothing to do; gadgets are the same. This should be somewhat common
} else {
@ -2630,20 +2629,24 @@ ReturnedValue QObjectMethod::callInternal(const Value *thisObject, const Value *
QQmlObjectOrGadget object = [&](){
if (mode == Heap::QObjectMethod::Included) {
if (QObject *qObject = d()->qObj)
return QQmlObjectOrGadget(qObject);
wrapper = d()->valueTypeWrapper;
Q_ASSERT(wrapper);
return QQmlObjectOrGadget(wrapper->metaObject(), wrapper->gadgetPtr());
QV4::Scope scope(v4);
if (QV4::Scoped<QV4::QObjectWrapper> qobject(scope, d()->wrapper); qobject)
return QQmlObjectOrGadget(qobject->object());
if (QV4::Scoped<QV4::QQmlTypeWrapper> type(scope, d()->wrapper); type)
return QQmlObjectOrGadget(type->object());
if (QV4::Scoped<QV4::QQmlValueTypeWrapper> value(scope, d()->wrapper); value) {
valueWrapper = value->d();
return QQmlObjectOrGadget(valueWrapper->metaObject(), valueWrapper->gadgetPtr());
}
Q_UNREACHABLE();
} else {
if (o)
return QQmlObjectOrGadget(o);
Q_ASSERT(wrapper);
if (!wrapper->enforcesLocation())
QV4::ReferenceObject::readReference(wrapper);
return QQmlObjectOrGadget(thisMeta, wrapper->gadgetPtr());
Q_ASSERT(valueWrapper);
if (!valueWrapper->enforcesLocation())
QV4::ReferenceObject::readReference(valueWrapper);
return QQmlObjectOrGadget(thisMeta, valueWrapper->gadgetPtr());
}
}();
@ -2667,9 +2670,9 @@ ReturnedValue QObjectMethod::callInternal(const Value *thisObject, const Value *
// The method might change the value.
const auto doCall = [&](const auto &call) {
if (!method->isConstant()) {
if (wrapper && wrapper->isReference()) {
if (valueWrapper && valueWrapper->isReference()) {
ScopedValue rv(scope, call());
d()->valueTypeWrapper->writeBack();
valueWrapper->writeBack();
return rv->asReturnedValue();
}
}

View File

@ -61,23 +61,21 @@ private:
};
#define QObjectMethodMembers(class, Member) \
Member(class, Pointer, QQmlValueTypeWrapper *, valueTypeWrapper)
Member(class, Pointer, Object *, wrapper) \
DECLARE_HEAP_OBJECT(QObjectMethod, FunctionObject) {
DECLARE_MARKOBJECTS(QObjectMethod)
QV4QPointer<QObject> qObj;
QQmlPropertyData *methods;
alignas(alignof(QQmlPropertyData)) std::byte _singleMethod[sizeof(QQmlPropertyData)];
int methodCount;
int index;
void init(QV4::ExecutionContext *scope);
void init(QV4::ExecutionContext *scope, Object *wrapper, int index);
void destroy()
{
if (methods != reinterpret_cast<const QQmlPropertyData *>(&_singleMethod))
delete[] methods;
qObj.destroy();
FunctionObject::destroy();
}
@ -85,8 +83,7 @@ DECLARE_HEAP_OBJECT(QObjectMethod, FunctionObject) {
QString name() const;
const QMetaObject *metaObject() const;
QObject *object() const { return qObj.data(); }
void setObject(QObject *o) { qObj = o; }
QObject *object() const;
bool isDetached() const;
bool isAttachedTo(QObject *o) const;
@ -144,6 +141,13 @@ struct Q_QML_EXPORT QObjectWrapper : public Object
static void initializeBindings(ExecutionEngine *engine);
const QMetaObject *metaObject() const
{
if (QObject *o = object())
return o->metaObject();
return nullptr;
}
QObject *object() const { return d()->object(); }
ReturnedValue getQmlProperty(
@ -220,10 +224,6 @@ protected:
private:
Q_NEVER_INLINE static ReturnedValue wrap_slowPath(ExecutionEngine *engine, QObject *object);
Q_NEVER_INLINE static ReturnedValue wrapConst_slowPath(ExecutionEngine *engine, QObject *object);
static Heap::QObjectMethod *cloneMethod(
ExecutionEngine *engine, Heap::QObjectMethod *cloneFrom,
Heap::Object *wrapper, QObject *object);
};
Q_DECLARE_OPERATORS_FOR_FLAGS(QObjectWrapper::Flags)
@ -354,13 +354,11 @@ struct Q_QML_EXPORT QObjectMethod : public QV4::FunctionObject
enum { DestroyMethod = -1, ToStringMethod = -2 };
static ReturnedValue create(
QV4::ExecutionContext *scope, QObject *object, int index);
static ReturnedValue create(QV4::ExecutionContext *scope, Heap::Object *wrapper, int index);
static ReturnedValue create(
QV4::ExecutionContext *scope, Heap::QQmlValueTypeWrapper *valueType, int index);
static ReturnedValue create(
QV4::ExecutionEngine *engine, Heap::QObjectMethod *cloneFrom,
Heap::Object *wrapper, QObject *object);
static ReturnedValue create(QV4::ExecutionEngine *engine, Heap::QObjectMethod *cloneFrom,
Heap::Object *wrapper, Heap::Object *object);
int methodIndex() const { return d()->index; }
QObject *object() const { return d()->object(); }

View File

@ -50,19 +50,31 @@ bool QQmlTypeWrapper::isSingleton() const
return d()->type().isSingleton();
}
const QMetaObject *QQmlTypeWrapper::metaObject() const
{
const QQmlType type = d()->type();
if (!type.isValid())
return nullptr;
if (type.isSingleton())
return type.metaObject();
return type.attachedPropertiesType(QQmlEnginePrivate::get(engine()->qmlEngine()));
}
QObject *QQmlTypeWrapper::object() const
{
const QQmlType type = d()->type();
if (!type.isValid())
return nullptr;
QQmlEngine *qmlEngine = engine()->qmlEngine();
QQmlEnginePrivate *qmlEngine = QQmlEnginePrivate::get(engine()->qmlEngine());
if (type.isSingleton())
return QQmlEnginePrivate::get(qmlEngine)->singletonInstance<QObject *>(type);
return qmlEngine->singletonInstance<QObject *>(type);
return qmlAttachedPropertiesObject(
d()->object,
type.attachedPropertiesFunction(QQmlEnginePrivate::get(qmlEngine)));
type.attachedPropertiesFunction(qmlEngine));
}
QObject* QQmlTypeWrapper::singletonObject() const

View File

@ -66,6 +66,7 @@ struct Q_QML_EXPORT QQmlTypeWrapper : Object
V4_NEEDS_DESTROY
bool isSingleton() const;
const QMetaObject *metaObject() const;
QObject *object() const;
QObject *singletonObject() const;

View File

@ -307,6 +307,7 @@ private slots:
void symbolToVariant();
void garbageCollectedObjectMethodBase();
public:
Q_INVOKABLE QJSValue throwingCppMethod1();
Q_INVOKABLE void throwingCppMethod2();
@ -6163,6 +6164,97 @@ void tst_QJSEngine::symbolToVariant()
QCOMPARE(val.toVariant(QJSValue::ConvertJSObjects), QStringLiteral("Symbol(asymbol)"));
}
class PACHelper : public QObject {
Q_OBJECT
public:
Q_INVOKABLE bool shExpMatch(const QString &, const QString &) { return false; }
Q_INVOKABLE QString dnsResolve(const QString &) { return QString{}; }
};
class ProxyAutoConf {
public:
void exposeQObjectMethodsAsGlobal(QJSEngine *engine, QObject *object)
{
QJSValue helper = engine->newQObject(object);
QJSValue g = engine->globalObject();
QJSValueIterator it(helper);
while (it.hasNext()) {
it.next();
if (!it.value().isCallable())
continue;
g.setProperty(it.name(), it.value());
}
}
bool parse(const QString & pacBytes)
{
jsFindProxyForURL = QJSValue();
engine = std::make_unique<QJSEngine>();
exposeQObjectMethodsAsGlobal(engine.get(), new PACHelper);
engine->evaluate(pacBytes);
jsFindProxyForURL = engine->globalObject().property(QStringLiteral("FindProxyForURL"));
return true;
}
QString findProxyForUrl(const QString &url, const QString &host)
{
QJSValueList args;
args << url << host;
engine->collectGarbage();
QJSValue callResult = jsFindProxyForURL.call(args);
return callResult.toString().trimmed();
}
private:
std::unique_ptr<QJSEngine> engine;
QJSValue jsFindProxyForURL;
};
QString const pacstring = R"js(
function FindProxyForURL(host) {
list_split_all = Array(
"oneoneoneoneo.oneo.oneo.oneoneo.one",
"twotwotwotwotw.otwo.twot.wotwotw.otw",
"threethreethr.eeth.reet.hreethr.eet",
"fourfourfourfo.urfo.urfo.urfourf.our",
"fivefivefivef.ivef.ivef.ivefive.fiv",
"sixsixsixsixsi.xsix.sixs.ixsixsi.xsi",
"sevensevenseve.nsev.ense.venseve.nse",
"eight.eighteigh.tei",
"*.nin.eninen.ine"
)
list_myip_direct =
"10.254.0.0/255.255.0.0"
for (i = 0; i < list_split_all.length; ++i)
for (j = 0; j < list_myip_direct.length; ++j)
shExpMatch(host, list_split_all)
shExpMatch()
dnsResolve()}
)js";
void tst_QJSEngine::garbageCollectedObjectMethodBase()
{
ProxyAutoConf proxyConf;
bool pac_read = false;
const auto processUrl = [&](QString const &url, QString const &host)
{
if (!pac_read) {
proxyConf.parse(pacstring);
pac_read = true;
}
return proxyConf.findProxyForUrl(url, host);
};
const QString url = QStringLiteral("https://servername.domain.test");
const QString host = QStringLiteral("servername.domain.test");
for (size_t i = 0; i < 5; ++i) {
auto future = std::async(processUrl, url, host);
QCOMPARE(future.get(), QLatin1String("Error: Insufficient arguments"));
}
}
QTEST_MAIN(tst_QJSEngine)
#include "tst_qjsengine.moc"