From c07f87e35062e39e9d4b3b978362dbf94dd36fbb Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Tue, 9 Oct 2012 15:44:25 -0300 Subject: Fixed crash during model destruction in QML. --- libqmenumodel/src/qdbusmenumodel.cpp | 4 +- libqmenumodel/src/qmenumodel.cpp | 14 ++++-- libqmenumodel/src/qmenumodel.h | 2 +- tests/client/CMakeLists.txt | 5 +- tests/client/loadmodel.qml | 38 +++++++++++++++ tests/client/modeltest.cpp | 22 +++++++++ tests/client/qmlfiles.h.in | 2 + tests/client/qmltest.cpp | 91 ++++++++++++++++++++++++++++++++++++ tests/client/script_qmltest.py | 12 +++++ 9 files changed, 181 insertions(+), 9 deletions(-) create mode 100644 tests/client/loadmodel.qml create mode 100644 tests/client/qmlfiles.h.in create mode 100644 tests/client/qmltest.cpp create mode 100755 tests/client/script_qmltest.py diff --git a/libqmenumodel/src/qdbusmenumodel.cpp b/libqmenumodel/src/qdbusmenumodel.cpp index ae88c35..6d4746c 100644 --- a/libqmenumodel/src/qdbusmenumodel.cpp +++ b/libqmenumodel/src/qdbusmenumodel.cpp @@ -85,7 +85,7 @@ void QDBusMenuModel::stop() /*! \internal */ void QDBusMenuModel::serviceVanish(GDBusConnection *) { - setMenuModel(NULL); + setMenuModel(NULL, true); } /*! \internal */ @@ -94,7 +94,7 @@ void QDBusMenuModel::serviceAppear(GDBusConnection *connection) GMenuModel *model = reinterpret_cast(g_dbus_menu_model_get(connection, busName().toLatin1(), objectPath().toLatin1())); - setMenuModel(model); + setMenuModel(model, true); if (model == NULL) { stop(); } diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index 4c0fc23..cac2f1a 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -46,23 +46,25 @@ QMenuModel::QMenuModel(GMenuModel *other, QObject *parent) rolesNames[Extra] = "extra"; } setRoleNames(rolesNames); - setMenuModel(other); + setMenuModel(other, true); } /*! \internal */ QMenuModel::~QMenuModel() { - setMenuModel(NULL); + setMenuModel(NULL, false); } /*! \internal */ -void QMenuModel::setMenuModel(GMenuModel *other) +void QMenuModel::setMenuModel(GMenuModel *other, bool notify) { if (m_menuModel == other) { return; } - beginResetModel(); + if (notify) { + beginResetModel(); + } if (m_menuModel) { g_signal_handler_disconnect(m_menuModel, m_signalChangedId); @@ -81,7 +83,9 @@ void QMenuModel::setMenuModel(GMenuModel *other) this); } - endResetModel(); + if (notify) { + endResetModel(); + } } /*! \internal */ diff --git a/libqmenumodel/src/qmenumodel.h b/libqmenumodel/src/qmenumodel.h index 7520480..e482e9b 100644 --- a/libqmenumodel/src/qmenumodel.h +++ b/libqmenumodel/src/qmenumodel.h @@ -48,7 +48,7 @@ public: protected: QMenuModel(GMenuModel *other=0, QObject *parent=0); - void setMenuModel(GMenuModel *model); + void setMenuModel(GMenuModel *model, bool notify); GMenuModel *menuModel() const; private: diff --git a/tests/client/CMakeLists.txt b/tests/client/CMakeLists.txt index 7455b16..9bce370 100644 --- a/tests/client/CMakeLists.txt +++ b/tests/client/CMakeLists.txt @@ -1,6 +1,6 @@ macro(declare_test testname) add_executable(${testname} ${testname}.cpp) - qt5_use_modules(${testname} Core DBus Widgets Test) + qt5_use_modules(${testname} Core DBus Widgets Test Qml Quick) target_link_libraries(${testname} qmenumodel dbusmenuscript @@ -50,5 +50,8 @@ declare_test(servicetest) declare_test(menuchangestest) declare_test(modeltest) declare_test(actiongrouptest) +declare_test(qmltest) declare_simple_test(convertertest) +configure_file(${CMAKE_CURRENT_SOURCE_DIR}/qmlfiles.h.in + ${CMAKE_CURRENT_BINARY_DIR}/qmlfiles.h) diff --git a/tests/client/loadmodel.qml b/tests/client/loadmodel.qml new file mode 100644 index 0000000..a91c18b --- /dev/null +++ b/tests/client/loadmodel.qml @@ -0,0 +1,38 @@ +import QtQuick 2.0 +import QMenuModel 0.1 + +Item { + id: root + width: 100 + height: 100 + + property bool reset: resetModel + + onResetChanged: { + if (reset) { + console.log("Remove page"); + view.model.destroy(); + //pop(); + } + } + + ListView { + id: view + anchors.fill: parent + delegate: Text { + text: label + } + onCountChanged: { + console.log("Row count: " + count); + } + } + + Component.onCompleted: { + var model = Qt.createQmlObject("import QMenuModel 0.1; QDBusMenuModel { id: menuModel; busType: globalBusType; busName: globalBusName; objectPath: globalObjectPath; }", view, ""); + model.start(); + console.log("New model: " + model) + console.log("New model2: " + model) + view.model = model; + } +} + diff --git a/tests/client/modeltest.cpp b/tests/client/modeltest.cpp index 542b38a..8f20bc3 100644 --- a/tests/client/modeltest.cpp +++ b/tests/client/modeltest.cpp @@ -209,6 +209,28 @@ private Q_SLOTS: QCOMPARE(v.toString(), QString("42")); } + /* + * Test if model is destroyed without crash + */ + void testDestroyModel() + { + // Make menu available + m_script.publishMenu(); + m_script.run(); + + // create a new model + QDBusMenuModel *model = new QDBusMenuModel(); + model->setBusType(DBusEnums::SessionBus); + model->setBusName(MENU_SERVICE_NAME); + model->setObjectPath(MENU_OBJECT_PATH); + model->start(); + + // Wait for dbus sync + QTest::qWait(500); + + delete model; + } + }; QTEST_MAIN(ModelTest) diff --git a/tests/client/qmlfiles.h.in b/tests/client/qmlfiles.h.in new file mode 100644 index 0000000..ebb534c --- /dev/null +++ b/tests/client/qmlfiles.h.in @@ -0,0 +1,2 @@ +const char* QML_BASE_DIR = "@libqmenumodel_BINARY_DIR@"; +const char* LOADMODEL_QML = "@CMAKE_CURRENT_SOURCE_DIR@/loadmodel.qml"; diff --git a/tests/client/qmltest.cpp b/tests/client/qmltest.cpp new file mode 100644 index 0000000..ae0d914 --- /dev/null +++ b/tests/client/qmltest.cpp @@ -0,0 +1,91 @@ +/* + * Copyright 2012 Canonical Ltd. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation; version 3. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program. If not, see . + * + * Authors: + * Renato Araujo Oliveira Filho + */ + +extern "C" { +#include +} + +#include "qdbusmenumodel.h" +#include "dbusmenuscript.h" +#include "qmlfiles.h" + +#include +#include +#include +#include + +#include +#include +#include + + +class QMLTest : public QObject +{ + Q_OBJECT +private: + DBusMenuScript m_script; + +private Q_SLOTS: + void initTestCase() + { + g_type_init(); + Q_ASSERT(m_script.connect()); + } + + void cleanupTestCase() + { + m_script.quit(); + } + + void init() + { + } + + void cleanup() + { + m_script.unpublishMenu(); + } + + /* + * Test if model is destroyed without crash + */ + void destoyModel() + { + m_script.publishMenu(); + m_script.run(); + QTest::qWait(500); + + QQuickView *view = new QQuickView; + view->engine()->addImportPath(QML_BASE_DIR); + view->engine()->rootContext()->setContextProperty("resetModel", QVariant(false)); + view->engine()->rootContext()->setContextProperty("globalBusType", DBusEnums::SessionBus); + view->engine()->rootContext()->setContextProperty("globalBusName", MENU_SERVICE_NAME); + view->engine()->rootContext()->setContextProperty("globalObjectPath", MENU_OBJECT_PATH); + view->setSource(QUrl::fromLocalFile(LOADMODEL_QML)); + view->show(); + QTest::qWait(500); + view->engine()->rootContext()->setContextProperty("resetModel", true); + QTest::qWait(500); + } + +}; + +QTEST_MAIN(QMLTest) + +#include "qmltest.moc" diff --git a/tests/client/script_qmltest.py b/tests/client/script_qmltest.py new file mode 100755 index 0000000..385256c --- /dev/null +++ b/tests/client/script_qmltest.py @@ -0,0 +1,12 @@ +#!/usr/bin/python2.7 + +import time +from menuscript import Script, ActionList, MENU_OBJECT_PATH + +al = ActionList(MENU_OBJECT_PATH) +al.appendItem("Menu0", "Menu0") +al.appendItem("Menu1", "Menu1") + +t = Script.create(al) +t.run() + -- cgit v1.2.3 From 1e659aed9b6882dbe05d812363715c8e0583fc9a Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Wed, 10 Oct 2012 09:47:23 -0300 Subject: Created a new private function "clearModel" instead of change signature of "setModel" function. --- libqmenumodel/src/qdbusmenumodel.cpp | 4 ++-- libqmenumodel/src/qmenumodel.cpp | 31 +++++++++++++++++-------------- libqmenumodel/src/qmenumodel.h | 3 ++- 3 files changed, 21 insertions(+), 17 deletions(-) diff --git a/libqmenumodel/src/qdbusmenumodel.cpp b/libqmenumodel/src/qdbusmenumodel.cpp index 6d4746c..ae88c35 100644 --- a/libqmenumodel/src/qdbusmenumodel.cpp +++ b/libqmenumodel/src/qdbusmenumodel.cpp @@ -85,7 +85,7 @@ void QDBusMenuModel::stop() /*! \internal */ void QDBusMenuModel::serviceVanish(GDBusConnection *) { - setMenuModel(NULL, true); + setMenuModel(NULL); } /*! \internal */ @@ -94,7 +94,7 @@ void QDBusMenuModel::serviceAppear(GDBusConnection *connection) GMenuModel *model = reinterpret_cast(g_dbus_menu_model_get(connection, busName().toLatin1(), objectPath().toLatin1())); - setMenuModel(model, true); + setMenuModel(model); if (model == NULL) { stop(); } diff --git a/libqmenumodel/src/qmenumodel.cpp b/libqmenumodel/src/qmenumodel.cpp index cac2f1a..0fedd62 100644 --- a/libqmenumodel/src/qmenumodel.cpp +++ b/libqmenumodel/src/qmenumodel.cpp @@ -46,31 +46,25 @@ QMenuModel::QMenuModel(GMenuModel *other, QObject *parent) rolesNames[Extra] = "extra"; } setRoleNames(rolesNames); - setMenuModel(other, true); + setMenuModel(other); } /*! \internal */ QMenuModel::~QMenuModel() { - setMenuModel(NULL, false); + clearModel(); } /*! \internal */ -void QMenuModel::setMenuModel(GMenuModel *other, bool notify) +void QMenuModel::setMenuModel(GMenuModel *other) { if (m_menuModel == other) { return; } - if (notify) { - beginResetModel(); - } + beginResetModel(); - if (m_menuModel) { - g_signal_handler_disconnect(m_menuModel, m_signalChangedId); - m_signalChangedId = 0; - g_object_unref(m_menuModel); - } + clearModel(); m_menuModel = other; @@ -83,9 +77,7 @@ void QMenuModel::setMenuModel(GMenuModel *other, bool notify) this); } - if (notify) { - endResetModel(); - } + endResetModel(); } /*! \internal */ @@ -94,6 +86,17 @@ GMenuModel *QMenuModel::menuModel() const return m_menuModel; } +/*! \internal */ +void QMenuModel::clearModel() +{ + if (m_menuModel) { + g_signal_handler_disconnect(m_menuModel, m_signalChangedId); + m_signalChangedId = 0; + g_object_unref(m_menuModel); + m_menuModel = NULL; + } +} + /*! \internal */ QVariant QMenuModel::data(const QModelIndex &index, int role) const { diff --git a/libqmenumodel/src/qmenumodel.h b/libqmenumodel/src/qmenumodel.h index e482e9b..5f17dd6 100644 --- a/libqmenumodel/src/qmenumodel.h +++ b/libqmenumodel/src/qmenumodel.h @@ -48,7 +48,7 @@ public: protected: QMenuModel(GMenuModel *other=0, QObject *parent=0); - void setMenuModel(GMenuModel *model, bool notify); + void setMenuModel(GMenuModel *model); GMenuModel *menuModel() const; private: @@ -59,6 +59,7 @@ private: QVariant getLink(const QModelIndex &index, const QString &linkName) const; QVariant getExtraProperties(const QModelIndex &index) const; QString parseExtraPropertyName(const QString &name) const; + void clearModel(); static void onItemsChanged(GMenuModel *model, gint position, gint removed, gint added, gpointer data); }; -- cgit v1.2.3 From 5dd33f34a444cea648f17fe8aadd93b516948957 Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Wed, 10 Oct 2012 09:49:39 -0300 Subject: Clenup qml file, removed debug messages and unused code. --- tests/client/loadmodel.qml | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/client/loadmodel.qml b/tests/client/loadmodel.qml index a91c18b..2eeb8e6 100644 --- a/tests/client/loadmodel.qml +++ b/tests/client/loadmodel.qml @@ -10,9 +10,7 @@ Item { onResetChanged: { if (reset) { - console.log("Remove page"); view.model.destroy(); - //pop(); } } @@ -22,16 +20,11 @@ Item { delegate: Text { text: label } - onCountChanged: { - console.log("Row count: " + count); - } } Component.onCompleted: { var model = Qt.createQmlObject("import QMenuModel 0.1; QDBusMenuModel { id: menuModel; busType: globalBusType; busName: globalBusName; objectPath: globalObjectPath; }", view, ""); model.start(); - console.log("New model: " + model) - console.log("New model2: " + model) view.model = model; } } -- cgit v1.2.3 From 7c27fad278a0a41e7a1488b06a94021f2d468461 Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Wed, 10 Oct 2012 09:51:12 -0300 Subject: Fixed typo in function name. --- tests/client/qmltest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/client/qmltest.cpp b/tests/client/qmltest.cpp index ae0d914..e9db4ac 100644 --- a/tests/client/qmltest.cpp +++ b/tests/client/qmltest.cpp @@ -65,7 +65,7 @@ private Q_SLOTS: /* * Test if model is destroyed without crash */ - void destoyModel() + void destroyModel() { m_script.publishMenu(); m_script.run(); -- cgit v1.2.3 From f430d60b0a1a458985d41cb964b1a6e569c2e63e Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Thu, 11 Oct 2012 09:15:32 -0300 Subject: Added comments on qml file used in tests. --- tests/client/loadmodel.qml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/client/loadmodel.qml b/tests/client/loadmodel.qml index 2eeb8e6..7a1f045 100644 --- a/tests/client/loadmodel.qml +++ b/tests/client/loadmodel.qml @@ -10,6 +10,7 @@ Item { onResetChanged: { if (reset) { + // destroy the current model and check if it will not crash the QML engine view.model.destroy(); } } @@ -23,6 +24,7 @@ Item { } Component.onCompleted: { + // dynamically create the model to destroy it later var model = Qt.createQmlObject("import QMenuModel 0.1; QDBusMenuModel { id: menuModel; busType: globalBusType; busName: globalBusName; objectPath: globalObjectPath; }", view, ""); model.start(); view.model = model; -- cgit v1.2.3 From 9bf14725a2333ca9535e734e2d6f3d7168a090d4 Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Thu, 11 Oct 2012 16:15:17 -0300 Subject: Created unit test for crash during model destruction. --- tests/client/loadmodel2.qml | 22 ++++++++++++++++++++++ tests/client/qmlfiles.h.in | 1 + tests/client/qmltest.cpp | 29 +++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+) create mode 100644 tests/client/loadmodel2.qml diff --git a/tests/client/loadmodel2.qml b/tests/client/loadmodel2.qml new file mode 100644 index 0000000..dfe265d --- /dev/null +++ b/tests/client/loadmodel2.qml @@ -0,0 +1,22 @@ +import QtQuick 2.0 +import QMenuModel 0.1 + +Item { + width: 100 + height: 100 + + QDBusMenuModel { + id: menuModel + busType: globalBusType + busName: globalBusName + objectPath: globalObjectPath + } + + ListView { + model: menuModel + delegate: Item {} + } + + Component.onCompleted: menuModel.start() +} + diff --git a/tests/client/qmlfiles.h.in b/tests/client/qmlfiles.h.in index ebb534c..876b78f 100644 --- a/tests/client/qmlfiles.h.in +++ b/tests/client/qmlfiles.h.in @@ -1,2 +1,3 @@ const char* QML_BASE_DIR = "@libqmenumodel_BINARY_DIR@"; const char* LOADMODEL_QML = "@CMAKE_CURRENT_SOURCE_DIR@/loadmodel.qml"; +const char* LOADMODEL2_QML = "@CMAKE_CURRENT_SOURCE_DIR@/loadmodel2.qml"; diff --git a/tests/client/qmltest.cpp b/tests/client/qmltest.cpp index e9db4ac..4e6f288 100644 --- a/tests/client/qmltest.cpp +++ b/tests/client/qmltest.cpp @@ -84,6 +84,35 @@ private Q_SLOTS: QTest::qWait(500); } + /* + * Test the menu model disappearing from the bus and reappearing + * while the QML application is running. + */ + void testServiceDisappear() + { + m_script.publishMenu(); + m_script.run(); + QTest::qWait(500); + + QQuickView *view = new QQuickView; + view->engine()->addImportPath(QML_BASE_DIR); + view->engine()->rootContext()->setContextProperty("globalBusType", DBusEnums::SessionBus); + view->engine()->rootContext()->setContextProperty("globalBusName", MENU_SERVICE_NAME); + view->engine()->rootContext()->setContextProperty("globalObjectPath", MENU_OBJECT_PATH); + view->setSource(QUrl::fromLocalFile(LOADMODEL2_QML)); + view->show(); + QTest::qWait(500); + + m_script.unpublishMenu(); + QTest::qWait(500); + + m_script.publishMenu(); + m_script.run(); + QTest::qWait(500); + + delete view; + QTest::qWait(1000); + } }; QTEST_MAIN(QMLTest) -- cgit v1.2.3 From 99229ab719164810345198e59ed6950445a75681 Mon Sep 17 00:00:00 2001 From: Renato Araujo Oliveira Filho Date: Thu, 11 Oct 2012 16:15:30 -0300 Subject: Added new generated test files into bzrignore. --- .bzrignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.bzrignore b/.bzrignore index 13a6812..6097094 100644 --- a/.bzrignore +++ b/.bzrignore @@ -16,6 +16,8 @@ tests/client/actiongrouptest tests/client/convertertest tests/client/menuchangestest tests/client/modeltest +tests/client/qmlfiles.h +tests/client/qmltest tests/client/servicetest doc/html/ -- cgit v1.2.3