QQmlPropertyCache: check methods before properties

When creating the QQmlPropertyData, search within the methods list
before searching for properties. The reason is that if the meta object
is dynamic, looking up a property will always return a result (if the
property doesn't exist, it will be created) and therefore all methods
will be obscured.
By swapping the search order, we eliminate this risk (methods are not
dynamically added).

Task-number: QTBUG-29836
Change-Id: Ie367f757c37ef4bc834a6c1c009f27bcf344fe76
Reviewed-by: Matthew Vogt <matthew.vogt@qinetic.com.au>
Reviewed-by: Alan Alpert <aalpert@blackberry.com>

diff --git a/src/qml/qml/qqmlpropertycache.cpp b/src/qml/qml/qqmlpropertycache.cpp
index dee9f26..4712fbd 100644 (file)
--- a/src/qml/qml/qqmlpropertycache.cpp
+++ b/src/qml/qml/qqmlpropertycache.cpp
@@ -1317,6 +1317,31 @@ QQmlPropertyData qQmlPropertyCacheCreate(const QMetaObject *metaObject, const QS
     Q_ASSERT(metaObject);
 
     QQmlPropertyData rv;
+
+    /* It's important to check the method list before checking for properties;
+     * otherwise, if the meta object is dynamic, a property will be created even
+     * if not found and it might obscure a method having the same name. */
+
+    //Used to block access to QObject::destroyed() and QObject::deleteLater() from QML
+    static const int destroyedIdx1 = QObject::staticMetaObject.indexOfSignal("destroyed(QObject*)");
+    static const int destroyedIdx2 = QObject::staticMetaObject.indexOfSignal("destroyed()");
+    static const int deleteLaterIdx = QObject::staticMetaObject.indexOfSlot("deleteLater()");
+
+    int methodCount = metaObject->methodCount();
+    for (int ii = methodCount - 1; ii >= 0; --ii) {
+        if (ii == destroyedIdx1 || ii == destroyedIdx2 || ii == deleteLaterIdx)
+            continue;
+        QMetaMethod m = metaObject->method(ii);
+        if (m.access() == QMetaMethod::Private)
+            continue;
+        QString methodName = QString::fromUtf8(m.name().constData());
+
+        if (methodName == property) {
+            rv.load(m);
+            return rv;
+        }
+    }
+
     {
         const QMetaObject *cmo = metaObject;
         const QByteArray propertyName = property.toUtf8();
@@ -1342,27 +1367,6 @@ QQmlPropertyData qQmlPropertyCacheCreate(const QMetaObject *metaObject, const QS
             }
         }
     }
-
-    //Used to block access to QObject::destroyed() and QObject::deleteLater() from QML
-    static const int destroyedIdx1 = QObject::staticMetaObject.indexOfSignal("destroyed(QObject*)");
-    static const int destroyedIdx2 = QObject::staticMetaObject.indexOfSignal("destroyed()");
-    static const int deleteLaterIdx = QObject::staticMetaObject.indexOfSlot("deleteLater()");
-
-    int methodCount = metaObject->methodCount();
-    for (int ii = methodCount - 1; ii >= 0; --ii) {
-        if (ii == destroyedIdx1 || ii == destroyedIdx2 || ii == deleteLaterIdx)
-            continue;
-        QMetaMethod m = metaObject->method(ii);
-        if (m.access() == QMetaMethod::Private)
-            continue;
-        QString methodName = QString::fromUtf8(m.name().constData());
-
-        if (methodName == property) {
-            rv.load(m);
-            return rv;
-        }
-    }
-
     return rv;
 }
 

diff --git a/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp b/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp
index 2b772ba..ca212d3 100644 (file)
--- a/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp
+++ b/tests/auto/qml/qqmlpropertymap/tst_qqmlpropertymap.cpp
@@ -67,6 +67,7 @@ private slots:
     void QTBUG_17868();
     void metaObjectAccessibility();
     void QTBUG_31226();
+    void QTBUG_29836();
 };
 
 void tst_QQmlPropertyMap::insert()
@@ -287,13 +288,17 @@ class MyEnhancedPropertyMap : public QQmlPropertyMap
 {
     Q_OBJECT
 public:
-    MyEnhancedPropertyMap() : QQmlPropertyMap(this, 0) {}
+    MyEnhancedPropertyMap() : QQmlPropertyMap(this, 0), m_testSlotCalled(false) {}
+    bool testSlotCalled() const { return m_testSlotCalled; }
 
 signals:
     void testSignal();
 
 public slots:
-    void testSlot() {}
+    void testSlot() { m_testSlotCalled = true; }
+
+private:
+    bool m_testSlotCalled;
 };
 
 void tst_QQmlPropertyMap::metaObjectAccessibility()
@@ -341,6 +346,29 @@ void tst_QQmlPropertyMap::QTBUG_31226()
 
 }
 
+void tst_QQmlPropertyMap::QTBUG_29836()
+{
+    MyEnhancedPropertyMap map;
+    QCOMPARE(map.testSlotCalled(), false);
+
+    QQmlEngine engine;
+    QQmlContext context(&engine);
+    context.setContextProperty("enhancedMap", &map);
+    QQmlComponent c(&engine);
+    c.setData("import QtQuick 2.0\n"
+              "Item {\n"
+              "  Timer { interval: 5; running: true; onTriggered: enhancedMap.testSlot() }\n"
+              "}",
+              QUrl());
+    QObject *obj = c.create(&context);
+    QVERIFY(obj);
+
+    QTRY_COMPARE(map.testSlotCalled(), true);
+
+    delete obj;
+
+}
+
 QTEST_MAIN(tst_QQmlPropertyMap)
 
 #include "tst_qqmlpropertymap.moc"