QtQml: Do not retroactively detach or re-attach methods

If a method is attached to an object it means that it was stored in a
property. The property can be re-assigned, with a different method from
a different thisObject. We cannot re-use it at all when invoking the
lookup again.

There are a few cases we care about:

1. The lookup immediately calls the property.
   a, The lookup retrieves a method. The method will be detached from
      the get go. There is no ambiguity (unless it's overridden).
   b, The lookup retrieves a var property with a method inside. The
      method will be attached to its original thisObject. The method may
      have been replaced with something else in the mean time.
      Therefore, we re-fetch the value on each call. Since we already
      have a QV4::QObjectMethod in the property, we don't need to
      rebuild it. So the lookup is still not expensive.
2. The lookup stores the property in a local.
   a, The lookup retrieves a method. The method will be attached,
      signaling any future callers that they need to be careful here.
   b, The lookup retrieves a var property with a method inside. The
      method is already attached to its original thisObject and stays
      that way.
3. We call a local that contains a method. This can only be a method
   that contains a thisObject. We may still decide to use the thisObject
   as provided by the call, depending on the NativeMethodBehavior
   pragma.

Pick-to: 6.5 6.6
Fixes: QTBUG-114086
Change-Id: Ia9a9c06355fd7d5052cdf79ac3ffddfa32f56573
Reviewed-by: Fabian Kosmale <fabian.kosmale@qt.io>

1 files changed, 35 insertions, 0 deletions

diff --git a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
index 9fdd05fa9d..7da38bd1ce 100644
--- a/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
+++ b/tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp
@@ -415,6 +415,7 @@ private slots:
 
     void objectMethodClone();
     void unregisteredValueTypeConversion();
+    void retainThis();
 
 private:
     QQmlEngine engine;
@@ -8002,6 +8003,40 @@ void tst_qqmllanguage::unregisteredValueTypeConversion()
     QCOMPARE(handler->gadgeted, 1);
 }
 
+void tst_qqmllanguage::retainThis()
+{
+    QQmlEngine e;
+    const QUrl url = testFileUrl("retainThis.qml");
+    QQmlComponent c(&e, url);
+    QVERIFY2(c.isReady(), qPrintable(c.errorString()));
+
+    const QString warning = u"Calling C++ methods with 'this' objects different "
+                            "from the one they were retrieved from is broken, due to "
+                            "historical reasons. The original object is used as 'this' "
+                            "object. You can allow the given 'this' object to be used "
+                            "by setting 'pragma NativeMethodBehavior: AcceptThisObject'"_s;
+
+    // Both cases objA because we retain the thisObject.
+    for (int i = 0; i < 2; ++i) {
+        QTest::ignoreMessage(QtWarningMsg, qPrintable(url.toString() + u":12: "_s + warning));
+        QTest::ignoreMessage(QtDebugMsg, "objA says hello");
+        QTest::ignoreMessage(QtWarningMsg, qPrintable(url.toString() + u":13: "_s + warning));
+        QTest::ignoreMessage(QtDebugMsg, "objA says 5 + 6 = 11");
+    }
+
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(url.toString() + u":22: "_s + warning));
+    QTest::ignoreMessage(QtDebugMsg, "objA says hello");
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(url.toString() + u":22: "_s + warning));
+    QTest::ignoreMessage(QtDebugMsg, "objB says hello");
+
+    QTest::ignoreMessage(QtDebugMsg, "objC says 7 + 7 = 14");
+    QTest::ignoreMessage(QtWarningMsg, qPrintable(url.toString() + u":32: "_s + warning));
+    QTest::ignoreMessage(QtDebugMsg, "objB says 7 + 7 = 14");
+
+    QScopedPointer<QObject> o(c.create());
+    QVERIFY(!o.isNull());
+}
+
 QTEST_MAIN(tst_qqmllanguage)
 
 #include "tst_qqmllanguage.moc"