From 3c0b8076e79f64b5ba575fca7ac62c6a1b740ce4 Mon Sep 17 00:00:00 2001 From: Ulf Hermann Date: Mon, 5 Jun 2023 11:49:51 +0200 Subject: 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 --- tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp | 35 ++++++++++++++++++++++++ 1 file changed, 35 insertions(+) (limited to 'tests/auto/qml/qqmllanguage/tst_qqmllanguage.cpp') 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 o(c.create()); + QVERIFY(!o.isNull()); +} + QTEST_MAIN(tst_qqmllanguage) #include "tst_qqmllanguage.moc" -- cgit v1.2.3