From 43ff11f564d90f7818d741ae4304462938c38438 Mon Sep 17 00:00:00 2001 From: Kai Dohmen Date: Thu, 28 Apr 2016 22:08:55 +0200 Subject: Remove deleted transport objects Added a QMultiHash which maps transport objects to wrapped object ids. transportRemoved iterates over all matching wrapped objects and removes the passed transport object from their transports-vector. If the transports-vector is empty after removing the passed transport object the objectDestroyed will be called on the wrapped object. transportRemoved will be called either on the transports destoryed signal or on disconnecting the webchannel from it. Without this changes the QMetaObjectPublisher::wrappedObjects and ::registeredObjectIds would only be cleaned up if the website calls deleteLater on QObjects but not on website reloads. Task-number: QTBUG-50074 Change-Id: If294564fee2406edd7fb578852aeb269cac23a92 Reviewed-by: Milian Wolff (cherry picked from commit fa2374d7c4dedea907e2df26fdad28bdee73b122) Reviewed-by: Frederik Gladhorn --- src/webchannel/qmetaobjectpublisher.cpp | 24 +++++++++++++++ src/webchannel/qmetaobjectpublisher_p.h | 7 +++++ src/webchannel/qwebchannel.cpp | 5 ++- tests/auto/webchannel/tst_webchannel.cpp | 53 ++++++++++++++++++++++++++++++++ tests/auto/webchannel/tst_webchannel.h | 2 ++ 5 files changed, 90 insertions(+), 1 deletion(-) diff --git a/src/webchannel/qmetaobjectpublisher.cpp b/src/webchannel/qmetaobjectpublisher.cpp index b3fc53d..cd6ad70 100644 --- a/src/webchannel/qmetaobjectpublisher.cpp +++ b/src/webchannel/qmetaobjectpublisher.cpp @@ -449,6 +449,29 @@ void QMetaObjectPublisher::objectDestroyed(const QObject *object) pendingPropertyUpdates.remove(object); } +void QMetaObjectPublisher::transportRemoved(QWebChannelAbstractTransport *transport) +{ + QHash::iterator it = transportedWrappedObjects.find(transport); + // It is not allowed to modify a container while iterating over it. So save + // objects which should be removed and call objectDestroyed() on them later. + QVector objectsForDeletion; + while (it != transportedWrappedObjects.end() && it.key() == transport) { + if (wrappedObjects.contains(it.value())) { + QVector &transports = wrappedObjects[it.value()].transports; + transports.removeOne(transport); + if (transports.isEmpty()) + objectsForDeletion.append(wrappedObjects[it.value()].object); + } + + it++; + } + + transportedWrappedObjects.remove(transport); + + foreach (QObject *obj, objectsForDeletion) + objectDestroyed(obj); +} + QObject *QMetaObjectPublisher::unwrapObject(const QString &objectId) const { if (!objectId.isEmpty()) { @@ -520,6 +543,7 @@ QJsonValue QMetaObjectPublisher::wrapResult(const QVariant &result, QWebChannelA oi.transports = webChannel->d_func()->transports; } wrappedObjects.insert(id, oi); + transportedWrappedObjects.insert(transport, id); initializePropertyUpdates(object, classInfo); } else if (wrappedObjects.contains(id)) { diff --git a/src/webchannel/qmetaobjectpublisher_p.h b/src/webchannel/qmetaobjectpublisher_p.h index 048a33c..95e5077 100644 --- a/src/webchannel/qmetaobjectpublisher_p.h +++ b/src/webchannel/qmetaobjectpublisher_p.h @@ -170,6 +170,11 @@ public: QVariant toVariant(const QJsonValue &value, int targetType) const; + /** + * Remove wrapped objects which last transport relation is with the passed transport object. + */ + void transportRemoved(QWebChannelAbstractTransport *transport); + /** * Given a QVariant containing a QObject*, wrap the object and register for property updates * return the objects class information. @@ -251,6 +256,8 @@ private: // Map of objects wrapped from invocation returns QHash wrappedObjects; + // Map of transports to wrapped object ids + QMultiHash transportedWrappedObjects; // Map of objects to maps of signal indices to a set of all their property indices. // The last value is a set as a signal can be the notify signal of multiple properties. diff --git a/src/webchannel/qwebchannel.cpp b/src/webchannel/qwebchannel.cpp index 1e8ed8f..b591ee1 100644 --- a/src/webchannel/qwebchannel.cpp +++ b/src/webchannel/qwebchannel.cpp @@ -75,9 +75,11 @@ QT_BEGIN_NAMESPACE */ void QWebChannelPrivate::_q_transportDestroyed(QObject *object) { - const int idx = transports.indexOf(static_cast(object)); + QWebChannelAbstractTransport *transport = static_cast(object); + const int idx = transports.indexOf(transport); if (idx != -1) { transports.remove(idx); + publisher->transportRemoved(transport); } } @@ -246,6 +248,7 @@ void QWebChannel::disconnectFrom(QWebChannelAbstractTransport *transport) disconnect(transport, 0, this, 0); disconnect(transport, 0, d->publisher, 0); d->transports.remove(idx); + d->publisher->transportRemoved(transport); } } diff --git a/tests/auto/webchannel/tst_webchannel.cpp b/tests/auto/webchannel/tst_webchannel.cpp index 71779b9..5ee26ec 100644 --- a/tests/auto/webchannel/tst_webchannel.cpp +++ b/tests/auto/webchannel/tst_webchannel.cpp @@ -634,6 +634,28 @@ void TestWebChannel::testWrapRegisteredObject() QCOMPARE(obj.objectName(), returnedId); } +void TestWebChannel::testRemoveUnusedTransports() +{ + QWebChannel channel; + DummyTransport *dummyTransport = new DummyTransport(this); + TestObject obj; + + channel.connectTo(dummyTransport); + channel.d_func()->publisher->initializeClient(dummyTransport); + + QMetaObjectPublisher *pub = channel.d_func()->publisher; + pub->wrapResult(QVariant::fromValue(&obj), dummyTransport); + + QCOMPARE(pub->wrappedObjects.size(), 1); + QCOMPARE(pub->registeredObjectIds.size(), 1); + + channel.disconnectFrom(dummyTransport); + delete dummyTransport; + + QCOMPARE(pub->wrappedObjects.size(), 0); + QCOMPARE(pub->registeredObjectIds.size(), 0); +} + void TestWebChannel::testPassWrappedObjectBack() { QWebChannel channel; @@ -759,6 +781,37 @@ void TestWebChannel::benchRegisterObjects() channel.registerObjects(objects); } } + +void TestWebChannel::benchRemoveTransport() +{ + QWebChannel channel; + QList dummyTransports; + for (int i = 500; i > 0; i--) + dummyTransports.append(new DummyTransport(this)); + + QList> objs; + QMetaObjectPublisher *pub = channel.d_func()->publisher; + + foreach (DummyTransport *transport, dummyTransports) { + channel.connectTo(transport); + channel.d_func()->publisher->initializeClient(transport); + + /* 30 objects per transport */ + for (int i = 30; i > 0; i--) { + QSharedPointer obj = QSharedPointer::create(); + objs.append(obj); + pub->wrapResult(QVariant::fromValue(obj.data()), transport); + } + } + + QBENCHMARK_ONCE { + foreach (DummyTransport *transport, dummyTransports) + pub->transportRemoved(transport); + } + + qDeleteAll(dummyTransports); +} + #ifdef WEBCHANNEL_TESTS_CAN_USE_JS_ENGINE class SubclassedTestObject : public TestObject diff --git a/tests/auto/webchannel/tst_webchannel.h b/tests/auto/webchannel/tst_webchannel.h index 3469d41..0f5cf1c 100644 --- a/tests/auto/webchannel/tst_webchannel.h +++ b/tests/auto/webchannel/tst_webchannel.h @@ -283,6 +283,7 @@ private slots: void testSetPropertyConversion(); void testDisconnect(); void testWrapRegisteredObject(); + void testRemoveUnusedTransports(); void testPassWrappedObjectBack(); void testInfiniteRecursion(); @@ -290,6 +291,7 @@ private slots: void benchInitializeClients(); void benchPropertyUpdates(); void benchRegisterObjects(); + void benchRemoveTransport(); void qtbug46548_overriddenProperties(); -- cgit v1.2.1 From 3b28a6956cb3cab087481312dc82a907b118e0de Mon Sep 17 00:00:00 2001 From: Milian Wolff Date: Thu, 16 Jun 2016 22:55:43 +0200 Subject: Add a test that emits signals from C++ and checks them in QML This is again trying to reproduce the issue described in the bug report, but it also passes for me. Still, it's good to have this tested in more depth. Change-Id: Ibaaefd7359d558c3b59af3d86a1260ab06caa598 Task-number: QTBUG-54074 Reviewed-by: Frederik Gladhorn --- tests/auto/qml/qml.cpp | 2 ++ tests/auto/qml/qml.pro | 6 ++-- tests/auto/qml/testobject.cpp | 58 ++++++++++++++++++++++++++++++++++++++ tests/auto/qml/testobject.h | 59 +++++++++++++++++++++++++++++++++++++++ tests/auto/qml/tst_webchannel.qml | 43 +++++++++++++++++++++++++++- 5 files changed, 165 insertions(+), 3 deletions(-) create mode 100644 tests/auto/qml/testobject.cpp create mode 100644 tests/auto/qml/testobject.h diff --git a/tests/auto/qml/qml.cpp b/tests/auto/qml/qml.cpp index 0c6919e..1d99fec 100644 --- a/tests/auto/qml/qml.cpp +++ b/tests/auto/qml/qml.cpp @@ -40,11 +40,13 @@ #include "testtransport.h" #include "testwebchannel.h" +#include "testobject.h" int main(int argc, char **argv) { qmlRegisterType("QtWebChannel.Tests", 1, 0, "TestTransport"); qmlRegisterType("QtWebChannel.Tests", 1, 0, "TestWebChannel"); + qmlRegisterType("QtWebChannel.Tests", 1, 0, "TestObject"); return quick_test_main(argc, argv, "qml", QUICK_TEST_SOURCE_DIR); } diff --git a/tests/auto/qml/qml.pro b/tests/auto/qml/qml.pro index 5dbac40..82a951a 100644 --- a/tests/auto/qml/qml.pro +++ b/tests/auto/qml/qml.pro @@ -10,11 +10,13 @@ IMPORTPATH += $$OUT_PWD/../../../qml $$PWD SOURCES += \ qml.cpp \ testtransport.cpp \ - testwebchannel.cpp + testwebchannel.cpp \ + testobject.cpp HEADERS += \ testtransport.h \ - testwebchannel.h + testwebchannel.h \ + testobject.h OTHER_FILES += \ Client.qml \ diff --git a/tests/auto/qml/testobject.cpp b/tests/auto/qml/testobject.cpp new file mode 100644 index 0000000..894f2e1 --- /dev/null +++ b/tests/auto/qml/testobject.cpp @@ -0,0 +1,58 @@ +/**************************************************************************** +** +** Copyright (C) 2016 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Milian Wolff +** Contact: http://www.qt.io/licensing/ +** +** This file is part of the QtWebChannel module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL21$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see http://www.qt.io/terms-conditions. For further +** information use the contact form at http://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 or version 3 as published by the Free +** Software Foundation and appearing in the file LICENSE.LGPLv21 and +** LICENSE.LGPLv3 included in the packaging of this file. Please review the +** following information to ensure the GNU Lesser General Public License +** requirements will be met: https://www.gnu.org/licenses/lgpl.html and +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** As a special exception, The Qt Company gives you certain additional +** rights. These rights are described in The Qt Company LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include "testobject.h" + + +QT_BEGIN_NAMESPACE + +TestObject::TestObject(QObject* parent) + : QObject(parent) +{ +} + +TestObject::~TestObject() +{ +} + +void TestObject::triggerSignals() +{ + emit testSignalBool(true); + emit testSignalBool(false); + + emit testSignalInt(42); + emit testSignalInt(1); + emit testSignalInt(0); +} + +QT_END_NAMESPACE diff --git a/tests/auto/qml/testobject.h b/tests/auto/qml/testobject.h new file mode 100644 index 0000000..b20f2e4 --- /dev/null +++ b/tests/auto/qml/testobject.h @@ -0,0 +1,59 @@ +/**************************************************************************** +** +** Copyright (C) 2016 Klarälvdalens Datakonsult AB, a KDAB Group company, info@kdab.com, author Milian Wolff +** Contact: http://www.qt.io/licensing/ +** +** This file is part of the QtWebChannel module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL21$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see http://www.qt.io/terms-conditions. For further +** information use the contact form at http://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 2.1 or version 3 as published by the Free +** Software Foundation and appearing in the file LICENSE.LGPLv21 and +** LICENSE.LGPLv3 included in the packaging of this file. Please review the +** following information to ensure the GNU Lesser General Public License +** requirements will be met: https://www.gnu.org/licenses/lgpl.html and +** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html. +** +** As a special exception, The Qt Company gives you certain additional +** rights. These rights are described in The Qt Company LGPL Exception +** version 1.1, included in the file LGPL_EXCEPTION.txt in this package. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + + +#ifndef TESTOBJECT_H +#define TESTOBJECT_H + +#include + +QT_BEGIN_NAMESPACE + +class TestObject : public QObject +{ + Q_OBJECT +public: + explicit TestObject(QObject *parent = Q_NULLPTR); + ~TestObject(); + +public slots: + void triggerSignals(); + +signals: + void testSignalBool(bool testBool); + void testSignalInt(int testInt); +}; + +QT_END_NAMESPACE + +#endif // TESTOBJECT_H diff --git a/tests/auto/qml/tst_webchannel.qml b/tests/auto/qml/tst_webchannel.qml index 6cb24b0..dc4cf93 100644 --- a/tests/auto/qml/tst_webchannel.qml +++ b/tests/auto/qml/tst_webchannel.qml @@ -94,10 +94,15 @@ TestCase { } } + TestObject { + id: testObject + WebChannel.id: "testObject" + } + TestWebChannel { id: webChannel transports: [client.serverTransport] - registeredObjects: [myObj, myOtherObj, myFactory] + registeredObjects: [myObj, myOtherObj, myFactory, testObject] } function initChannel() { @@ -412,4 +417,40 @@ TestCase { myObj.mySignal(0, myObj); compare(signalArg, 42); } + + // see also: https://bugreports.qt.io/browse/QTBUG-54074 + function test_signalArgumentTypeConversion() + { + var signalArgs = []; + function logSignalArgs(arg) { + signalArgs.push(arg); + } + var channel = client.createChannel(function(channel) { + var testObject = channel.objects.testObject; + testObject.testSignalBool.connect(logSignalArgs); + testObject.testSignalInt.connect(logSignalArgs); + testObject.triggerSignals(); + }); + client.awaitInit(); + + var msg = client.awaitMessage(); + compare(msg.type, JSClient.QWebChannelMessageTypes.connectToSignal); + compare(msg.object, "testObject"); + + msg = client.awaitMessage(); + compare(msg.type, JSClient.QWebChannelMessageTypes.connectToSignal); + compare(msg.object, "testObject"); + + msg = client.awaitMessage(); + compare(msg.type, JSClient.QWebChannelMessageTypes.invokeMethod); + client.awaitIdle(); + + compare(signalArgs, [ + true, + false, + 42, + 1, + 0 + ]); + } } -- cgit v1.2.1 From 58c800edb56d976d95cb2097e27cd39cfc02bf50 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Thu, 28 Jul 2016 13:51:31 +0200 Subject: Bump version Change-Id: I5508bb77d24a5c1dcacf4d0dee57c31195c0a188 --- .qmake.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.qmake.conf b/.qmake.conf index 40e5787..45d16f2 100644 --- a/.qmake.conf +++ b/.qmake.conf @@ -1,4 +1,4 @@ load(qt_build_config) CONFIG += warning_clean -MODULE_VERSION = 5.7.0 +MODULE_VERSION = 5.7.1 -- cgit v1.2.1