Fix use of Component.onCompleted scripts in QQmlPropertyMap sub-types

After commit 9333ea8649838d7e0400b0e94c8cbd4fa5d216b0 when looking up
properties such as "console" in Component.onComplete handlers of
QQmlProperty sub-types, we end up implicitly creating "console"
properties, which is not correct.

At the same time, there are use-cases (and tests such as
tst_QQmlPropertyMap::QTBUG_35233) where a binding expression accesses a
property of a property map sub-type and it doesn't exist. Then later
that property is added and we must re-evaluate that binding. That works
because the implicit property creation when reading it also results in a
property capture.

Therefore we introduce the HFH (hack from hell): Detect the use of a
QQmlPropertyMap sub-type and go back to the old lookup behavior. The
path forward should be a deprecation of QQmlPropertyMap and perhaps the
ability to define abstract interfaces in QML.

Change-Id: Ib87bc1ebae44d7b332be47f1dcb614a19334b559
Reviewed-by: Lars Knoll <lars.knoll@qt.io>

1 files changed, 32 insertions, 5 deletions

diff --git a/src/qml/jsruntime/qv4qmlcontext.cpp b/src/qml/jsruntime/qv4qmlcontext.cpp
index dc69016559..c8ef87b979 100644
--- a/src/qml/jsruntime/qv4qmlcontext.cpp
+++ b/src/qml/jsruntime/qv4qmlcontext.cpp
@@ -125,6 +125,37 @@ ReturnedValue QQmlContextWrapper::virtualGet(const Managed *m, PropertyKey id, c
     QObject *scopeObject = resource->getScopeObject();
 
     ScopedString name(scope, id.asStringOrSymbol());
+
+    const auto performGobalLookUp = [&result, v4, &name, hasProperty]() {
+        bool hasProp = false;
+        result = v4->globalObject->get(name, &hasProp);
+        if (hasProp) {
+            if (hasProperty)
+                *hasProperty = hasProp;
+            return true;
+        }
+        return false;
+    };
+
+    // If the scope object is a QAbstractDynamicMetaObject, then QMetaObject::indexOfProperty
+    // will call createProperty() on the QADMO and implicitly create the property. While that
+    // is questionable behavior, there are two use-cases that we support in the light of this:
+    //
+    //    (1) The implicit creation of properties is necessary because it will also result in
+    //        a recorded capture, which will allow a re-evaluation of bindings when the value
+    //        is populated later. See QTBUG-35233 and the test-case in tst_qqmlpropertymap.
+    //
+    //    (1) Looking up "console" in order to place a console.log() call for example must
+    //        find the console instead of creating a new property. Therefore we prioritize the
+    //        lookup in the global object here.
+    //
+    // Note: The scope object is only a QADMO for example when somebody registers a QQmlPropertyMap
+    // sub-class as QML type and then instantiates it in .qml.
+    if (scopeObject && QQmlPropertyCache::isDynamicMetaObject(scopeObject->metaObject())) {
+        if (performGobalLookUp())
+            return result->asReturnedValue();
+    }
+
     if (context->imports && name->startsWithUpper()) {
         // Search for attached properties, enums and imported scripts
         QQmlTypeNameCache::Result r = context->imports->query(name, QQmlImport::AllowRecursion);
@@ -217,12 +248,8 @@ ReturnedValue QQmlContextWrapper::virtualGet(const Managed *m, PropertyKey id, c
 
     // Do a lookup in the global object here to avoid expressionContext->unresolvedNames becoming
     // true if we access properties of the global object.
-    result = v4->globalObject->get(name, &hasProp);
-    if (hasProp) {
-        if (hasProperty)
-            *hasProperty = hasProp;
+    if (performGobalLookUp())
         return result->asReturnedValue();
-    }
 
     expressionContext->unresolvedNames = true;