Directly load already known metaproperties in QV4QObjectWrapper

A method and a property can have the same name in a QObject. This is
not directly expressible in a JS object, but when iterating the
properties of a wrapped QObject we should not look them up by name as
we might find the wrong one this way. However, as we already know what
we are looking for, there is no need for any further searching anyway.

Task-number: QTBUG-58887
Change-Id: I68574008c7a078baab9b343d550cc27956b0d5a9
Reviewed-by: hjk <hjk@qt.io>
Reviewed-by: Simon Hausmann <simon.hausmann@qt.io>

4 files changed, 72 insertions, 8 deletions

diff --git a/src/qml/jsruntime/qv4qobjectwrapper.cpp b/src/qml/jsruntime/qv4qobjectwrapper.cpp
index 7260e71fab..346ca62a6f 100644
--- a/src/qml/jsruntime/qv4qobjectwrapper.cpp
+++ b/src/qml/jsruntime/qv4qobjectwrapper.cpp
@@ -673,18 +673,24 @@ void QObjectWrapper::advanceIterator(Managed *m, ObjectIterator *it, Value *name
 
     QObjectWrapper *that = static_cast<QObjectWrapper*>(m);
 
-    if (that->d()->object()) {
-        const QMetaObject *mo = that->d()->object()->metaObject();
+    QObject *thatObject = that->d()->object();
+    if (thatObject && !QQmlData::wasDeleted(thatObject)) {
+        const QMetaObject *mo = thatObject->metaObject();
         // These indices don't apply to gadgets, so don't block them.
         const bool preventDestruction = mo->superClass() || mo == &QObject::staticMetaObject;
         const int propertyCount = mo->propertyCount();
         if (it->arrayIndex < static_cast<uint>(propertyCount)) {
-            Scope scope(that->engine());
-            ScopedString propName(scope, that->engine()->newString(QString::fromUtf8(mo->property(it->arrayIndex).name())));
+            ExecutionEngine *thatEngine = that->engine();
+            Scope scope(thatEngine);
+            const QMetaProperty property = mo->property(it->arrayIndex);
+            ScopedString propName(scope, thatEngine->newString(QString::fromUtf8(property.name())));
             name->setM(propName->d());
             ++it->arrayIndex;
             *attributes = QV4::Attr_Data;
-            p->value = that->get(propName);
+
+            QQmlPropertyData local;
+            local.load(property);
+            p->value = that->getProperty(thatEngine, thatObject, &local);
             return;
         }
         const int methodCount = mo->methodCount();
@@ -694,11 +700,15 @@ void QObjectWrapper::advanceIterator(Managed *m, ObjectIterator *it, Value *name
             ++it->arrayIndex;
             if (method.access() == QMetaMethod::Private || (preventDestruction && (index == deleteLaterIdx || index == destroyedIdx1 || index == destroyedIdx2)))
                 continue;
-            Scope scope(that->engine());
-            ScopedString methodName(scope, that->engine()->newString(QString::fromUtf8(method.name())));
+            ExecutionEngine *thatEngine = that->engine();
+            Scope scope(thatEngine);
+            ScopedString methodName(scope, thatEngine->newString(QString::fromUtf8(method.name())));
             name->setM(methodName->d());
             *attributes = QV4::Attr_Data;
-            p->value = that->get(methodName);
+
+            QQmlPropertyData local;
+            local.load(method);
+            p->value = that->getProperty(thatEngine, thatObject, &local);
             return;
         }
     }
diff --git a/tests/auto/qml/qqmlecmascript/testtypes.cpp b/tests/auto/qml/qqmlecmascript/testtypes.cpp
index d9ddcd71a7..63c2918325 100644
--- a/tests/auto/qml/qqmlecmascript/testtypes.cpp
+++ b/tests/auto/qml/qqmlecmascript/testtypes.cpp
@@ -515,6 +515,8 @@ void registerTypes()
     qmlRegisterType<QObjectContainer>("Qt.test", 1, 0, "QObjectContainer");
     qmlRegisterType<QObjectContainerWithGCOnAppend>("Qt.test", 1, 0, "QObjectContainerWithGCOnAppend");
     qmlRegisterType<FloatingQObject>("Qt.test", 1, 0, "FloatingQObject");
+
+    qmlRegisterType<ClashingNames>("Qt.test", 1, 0, "ClashingNames");
 }
 
 #include "testtypes.moc"
diff --git a/tests/auto/qml/qqmlecmascript/testtypes.h b/tests/auto/qml/qqmlecmascript/testtypes.h
index 1f7f3344ef..eedeb66647 100644
--- a/tests/auto/qml/qqmlecmascript/testtypes.h
+++ b/tests/auto/qml/qqmlecmascript/testtypes.h
@@ -1714,6 +1714,14 @@ public:
     virtual void componentComplete();
 };
 
+class ClashingNames : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(bool clashes READ clashes CONSTANT)
+public:
+    Q_INVOKABLE bool clashes() const { return true; }
+};
+
 void registerTypes();
 
 #endif // TESTTYPES_H
diff --git a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
index 83725dbf12..b5df8307c0 100644
--- a/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
+++ b/tests/auto/qml/qqmlecmascript/tst_qqmlecmascript.cpp
@@ -46,6 +46,7 @@
 #include <private/qv4runtime_p.h>
 #include <private/qv4object_p.h>
 #include <private/qqmlcomponentattached_p.h>
+#include <private/qv4objectiterator_p.h>
 
 #ifdef Q_CC_MSVC
 #define NO_INLINE __declspec(noinline)
@@ -260,6 +261,7 @@ private slots:
     void nonNotifyable();
     void deleteWhileBindingRunning();
     void callQtInvokables();
+    void resolveClashingProperties();
     void invokableObjectArg();
     void invokableObjectRet();
     void invokableEnumRet();
@@ -2987,6 +2989,48 @@ void tst_qqmlecmascript::callQtInvokables()
     QVERIFY(callback.isCallable());
 }
 
+void tst_qqmlecmascript::resolveClashingProperties()
+{
+    ClashingNames *o = new ClashingNames();
+    QQmlEngine qmlengine;
+    QQmlEnginePrivate *ep = QQmlEnginePrivate::get(&qmlengine);
+
+    QV4::ExecutionEngine *engine = QV8Engine::getV4(ep->v8engine());
+    QV4::Scope scope(engine);
+
+    QV4::ScopedValue object(scope, QV4::QObjectWrapper::wrap(engine, o));
+    QV4::ObjectIterator it(scope, object->as<QV4::Object>(), QV4::ObjectIterator::EnumerableOnly);
+    QV4::ScopedValue name(scope);
+    QV4::ScopedValue value(scope);
+
+    bool seenProperty = false;
+    bool seenMethod = false;
+    while (true) {
+        QV4::Value v;
+        name = it.nextPropertyNameAsString(&v);
+        if (name->isNull())
+            break;
+        QString key = name->toQStringNoThrow();
+        if (key == QLatin1String("clashes")) {
+            value = v;
+            QV4::ScopedValue typeString(scope, QV4::Runtime::method_typeofValue(engine, value));
+            QString type = typeString->toQStringNoThrow();
+            if (type == QLatin1String("boolean")) {
+                QVERIFY(!seenProperty);
+                seenProperty = true;
+            } else if (type == QLatin1String("function")) {
+                QVERIFY(!seenMethod);
+                seenMethod = true;
+            } else {
+                QFAIL(qPrintable(QString::fromLatin1("found 'clashes' property of type %1")
+                                 .arg(type)));
+            }
+        }
+    }
+    QVERIFY(seenProperty);
+    QVERIFY(seenMethod);
+}
+
 // QTBUG-13047 (check that you can pass registered object types as args)
 void tst_qqmlecmascript::invokableObjectArg()
 {