From bd7958392dc040ee003523a68758cb061f4a5747 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Sat, 21 Jul 2012 11:25:17 +0200 Subject: [PATCH] Verify pointer is valid when calculating the longest caption The method was missing a check whether the weak pointers in the internal list got deleted. This could in very unlikely cases lead to a crash. In order to verify that adding the null pointer check fixes the crash a unit test is added to simulate the situation of a pointer being deleted. This required to add a mock a few classes of TabBox. A MockTabBoxHandler and MockTabBoxClient are added implementing the specific interfaces. The DeclarativeView is completely mocked to make the linker happy. Including the actual implementation is not possible as it pulls in half of KWin core. BUG: 303840 FIXED-IN: 4.9.0 REVIEW: 105645 --- tabbox/CMakeLists.txt | 1 + tabbox/clientmodel.cpp | 3 + tabbox/tests/CMakeLists.txt | 17 ++++ tabbox/tests/mock_declarative.cpp | 94 ++++++++++++++++++++++ tabbox/tests/mock_tabboxclient.cpp | 38 +++++++++ tabbox/tests/mock_tabboxclient.h | 67 ++++++++++++++++ tabbox/tests/mock_tabboxhandler.cpp | 99 ++++++++++++++++++++++++ tabbox/tests/mock_tabboxhandler.h | 93 ++++++++++++++++++++++ tabbox/tests/test_tabbox_clientmodel.cpp | 47 +++++++++++ tabbox/tests/test_tabbox_clientmodel.h | 38 +++++++++ 10 files changed, 497 insertions(+) create mode 100644 tabbox/tests/CMakeLists.txt create mode 100644 tabbox/tests/mock_declarative.cpp create mode 100644 tabbox/tests/mock_tabboxclient.cpp create mode 100644 tabbox/tests/mock_tabboxclient.h create mode 100644 tabbox/tests/mock_tabboxhandler.cpp create mode 100644 tabbox/tests/mock_tabboxhandler.h create mode 100644 tabbox/tests/test_tabbox_clientmodel.cpp create mode 100644 tabbox/tests/test_tabbox_clientmodel.h diff --git a/tabbox/CMakeLists.txt b/tabbox/CMakeLists.txt index e222b6a63f..2ef58101fa 100644 --- a/tabbox/CMakeLists.txt +++ b/tabbox/CMakeLists.txt @@ -1,4 +1,5 @@ add_subdirectory( qml ) +add_subdirectory( tests ) # Install the KWin/WindowSwitcher service type install( FILES kwinwindowswitcher.desktop DESTINATION ${SERVICETYPES_INSTALL_DIR} ) diff --git a/tabbox/clientmodel.cpp b/tabbox/clientmodel.cpp index 06a0c1a869..b78c1f5ce4 100644 --- a/tabbox/clientmodel.cpp +++ b/tabbox/clientmodel.cpp @@ -93,6 +93,9 @@ QString ClientModel::longestCaption() const QString caption; foreach (QWeakPointer clientPointer, m_clientList) { QSharedPointer client = clientPointer.toStrongRef(); + if (!client) { + continue; + } if (client->caption().size() > caption.size()) { caption = client->caption(); } diff --git a/tabbox/tests/CMakeLists.txt b/tabbox/tests/CMakeLists.txt new file mode 100644 index 0000000000..f8377ac929 --- /dev/null +++ b/tabbox/tests/CMakeLists.txt @@ -0,0 +1,17 @@ +######################################################## +# Test TabBox::ClientModel +######################################################## +set( testTabBoxClientModel_SRCS + ../clientmodel.cpp + ../desktopmodel.cpp + ../tabboxconfig.cpp + ../tabboxhandler.cpp + test_tabbox_clientmodel.cpp + mock_declarative.cpp + mock_tabboxhandler.cpp + mock_tabboxclient.cpp +) + +kde4_add_unit_test( testTabBoxClientModel TESTNAME testTabBoxClientModel ${testTabBoxClientModel_SRCS} ) + +target_link_libraries( testTabBoxClientModel ${KDE4_KDEUI_LIBS} ${QT_QTCORE_LIBRARY} ${QT_QTGUI_LIBRARY} ${QT_QTDECLARATIVE_LIBRARY} ${QT_QTTEST_LIBRARY} ) diff --git a/tabbox/tests/mock_declarative.cpp b/tabbox/tests/mock_declarative.cpp new file mode 100644 index 0000000000..67ac56a9be --- /dev/null +++ b/tabbox/tests/mock_declarative.cpp @@ -0,0 +1,94 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2012 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +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 General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "declarative.h" + +namespace KWin +{ +namespace TabBox +{ +DeclarativeView::DeclarativeView(QAbstractItemModel *model, TabBoxConfig::TabBoxMode mode, QWidget *parent) + : QDeclarativeView(parent) +{ + Q_UNUSED(model) + Q_UNUSED(mode) +} + +void DeclarativeView::currentIndexChanged(int row) +{ + Q_UNUSED(row) +} + +void DeclarativeView::updateQmlSource(bool force) +{ + Q_UNUSED(force) +} + +void DeclarativeView::setCurrentIndex(const QModelIndex &index, bool disableAnimation) +{ + Q_UNUSED(index) + Q_UNUSED(disableAnimation) +} + +bool DeclarativeView::sendKeyEvent(QKeyEvent *e) +{ + Q_UNUSED(e) + return false; +} + +void DeclarativeView::slotEmbeddedChanged(bool enabled) +{ + Q_UNUSED(enabled) +} + +void DeclarativeView::slotUpdateGeometry() +{ +} + +void DeclarativeView::slotWindowChanged(WId wId, unsigned int properties) +{ + Q_UNUSED(wId) + Q_UNUSED(properties) +} + +void DeclarativeView::showEvent(QShowEvent *event) +{ + Q_UNUSED(event) +} + +void DeclarativeView::resizeEvent(QResizeEvent *event) +{ + Q_UNUSED(event) +} + +void DeclarativeView::hideEvent(QHideEvent *event) +{ + Q_UNUSED(event) +} + +bool DeclarativeView::x11Event(XEvent *e) +{ + Q_UNUSED(e) + return false; +} + +} // namespace Tabbox +} // namespace KWin + +#include "../declarative.moc" diff --git a/tabbox/tests/mock_tabboxclient.cpp b/tabbox/tests/mock_tabboxclient.cpp new file mode 100644 index 0000000000..5b5f09ec01 --- /dev/null +++ b/tabbox/tests/mock_tabboxclient.cpp @@ -0,0 +1,38 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2012 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +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 General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "mock_tabboxclient.h" +#include "mock_tabboxhandler.h" + +namespace KWin +{ + +MockTabBoxClient::MockTabBoxClient(QString caption, WId id) + : TabBoxClient() + , m_caption(caption) + , m_wId(id) +{ +} + +void MockTabBoxClient::close() +{ + static_cast(TabBox::tabBox)->closeWindow(this); +} + +} // namespace KWin diff --git a/tabbox/tests/mock_tabboxclient.h b/tabbox/tests/mock_tabboxclient.h new file mode 100644 index 0000000000..ec438e603e --- /dev/null +++ b/tabbox/tests/mock_tabboxclient.h @@ -0,0 +1,67 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2012 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +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 General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#ifndef KWIN_MOCK_TABBOX_CLIENT_H +#define KWIN_MOCK_TABBOX_CLIENT_H + +#include "../tabboxhandler.h" +namespace KWin +{ +class MockTabBoxClient : public TabBox::TabBoxClient +{ +public: + explicit MockTabBoxClient(QString caption, WId id); + virtual bool isMinimized() const { + return false; + } + virtual QString caption() const { + return m_caption; + } + virtual void close(); + virtual int height() const { + return 100; + } + virtual QPixmap icon(const QSize &size = QSize(32, 32)) const { + return QPixmap(size); + } + virtual bool isCloseable() const { + return true; + } + virtual bool isFirstInTabBox() const { + return false; + } + virtual int width() const { + return 100; + } + virtual WId window() const { + return m_wId; + } + virtual int x() const { + return 0; + } + virtual int y() const { + return 0; + } + +private: + QString m_caption; + WId m_wId; +}; +} // namespace KWin +#endif diff --git a/tabbox/tests/mock_tabboxhandler.cpp b/tabbox/tests/mock_tabboxhandler.cpp new file mode 100644 index 0000000000..1d27dc752b --- /dev/null +++ b/tabbox/tests/mock_tabboxhandler.cpp @@ -0,0 +1,99 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2012 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +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 General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "mock_tabboxhandler.h" +#include "mock_tabboxclient.h" + +namespace KWin +{ + +MockTabBoxHandler::MockTabBoxHandler() + : TabBoxHandler() +{ +} + +MockTabBoxHandler::~MockTabBoxHandler() +{ +} + +void MockTabBoxHandler::grabbedKeyEvent(QKeyEvent *event) const +{ + Q_UNUSED(event) +} + +QWeakPointer< TabBox::TabBoxClient > MockTabBoxHandler::activeClient() const +{ + if (!m_windows.isEmpty()) { + return QWeakPointer< TabBox::TabBoxClient >(m_windows.first()); + } + return QWeakPointer(); +} + +QWeakPointer< TabBox::TabBoxClient > MockTabBoxHandler::clientToAddToList(TabBox::TabBoxClient *client, int desktop) const +{ + Q_UNUSED(desktop) + QList< QSharedPointer< TabBox::TabBoxClient > >::const_iterator it = m_windows.constBegin(); + for (; it != m_windows.constEnd(); ++it) { + if ((*it).data() == client) { + return QWeakPointer< TabBox::TabBoxClient >(*it); + } + } + return QWeakPointer< TabBox::TabBoxClient >(); +} + +QWeakPointer< TabBox::TabBoxClient > MockTabBoxHandler::nextClientFocusChain(TabBox::TabBoxClient *client) const +{ + if (!client) { + if (!m_windows.isEmpty()) { + return QWeakPointer< TabBox::TabBoxClient >(m_windows.first()); + } + } + QList< QSharedPointer< TabBox::TabBoxClient > >::const_iterator it = m_windows.constBegin(); + for (; it != m_windows.constEnd(); ++it) { + if ((*it).data() == client) { + ++it; + if (it == m_windows.constEnd()) { + return QWeakPointer< TabBox::TabBoxClient >(m_windows.first()); + } else { + return QWeakPointer< TabBox::TabBoxClient >(*it); + } + } + } + return QWeakPointer< TabBox::TabBoxClient >(); +} + +QWeakPointer< TabBox::TabBoxClient > MockTabBoxHandler::createMockWindow(const QString &caption, WId id) +{ + QSharedPointer< TabBox::TabBoxClient > client(new MockTabBoxClient(caption, id)); + m_windows.append(client); + return QWeakPointer< TabBox::TabBoxClient >(client); +} + +void MockTabBoxHandler::closeWindow(TabBox::TabBoxClient *client) +{ + QList< QSharedPointer< TabBox::TabBoxClient > >::iterator it = m_windows.begin(); + for (; it != m_windows.end(); ++it) { + if ((*it).data() == client) { + m_windows.erase(it); + return; + } + } +} + +} // namespace KWin diff --git a/tabbox/tests/mock_tabboxhandler.h b/tabbox/tests/mock_tabboxhandler.h new file mode 100644 index 0000000000..f13a820aac --- /dev/null +++ b/tabbox/tests/mock_tabboxhandler.h @@ -0,0 +1,93 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2012 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +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 General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#ifndef KWIN_MOCK_TABBOX_HANDLER_H +#define KWIN_MOCK_TABBOX_HANDLER_H + +#include "../tabboxhandler.h" +namespace KWin +{ +class MockTabBoxHandler : public TabBox::TabBoxHandler +{ + Q_OBJECT +public: + MockTabBoxHandler(); + virtual ~MockTabBoxHandler(); + virtual void activateAndClose() { + } + virtual QWeakPointer< TabBox::TabBoxClient > activeClient() const; + virtual int activeScreen() const { + return 0; + } + virtual QWeakPointer< TabBox::TabBoxClient > clientToAddToList(TabBox::TabBoxClient *client, int desktop) const; + virtual int currentDesktop() const { + return 1; + } + virtual QWeakPointer< TabBox::TabBoxClient > desktopClient() const { + return QWeakPointer(); + } + virtual QString desktopName(int desktop) const { + Q_UNUSED(desktop) + return "desktop 1"; + } + virtual QString desktopName(TabBox::TabBoxClient *client) const { + Q_UNUSED(client) + return "desktop"; + } + virtual void elevateClient(TabBox::TabBoxClient *c, WId tabbox, bool elevate) const { + Q_UNUSED(c) + Q_UNUSED(tabbox) + Q_UNUSED(elevate) + } + virtual void hideOutline() { + } + virtual QWeakPointer< TabBox::TabBoxClient > nextClientFocusChain(TabBox::TabBoxClient *client) const; + virtual int nextDesktopFocusChain(int desktop) const { + Q_UNUSED(desktop) + return 1; + } + virtual int numberOfDesktops() const { + return 1; + } + virtual QVector< Window > outlineWindowIds() const { + return QVector(); + } + virtual void raiseClient(TabBox::TabBoxClient *c) const { + Q_UNUSED(c) + } + virtual void restack(TabBox::TabBoxClient *c, TabBox::TabBoxClient *under) { + Q_UNUSED(c) + Q_UNUSED(under) + } + virtual void showOutline(const QRect &outline) { + Q_UNUSED(outline) + } + virtual TabBox::TabBoxClientList stackingOrder() const { + return TabBox::TabBoxClientList(); + } + virtual void grabbedKeyEvent(QKeyEvent *event) const; + + // mock methods + QWeakPointer createMockWindow(const QString &caption, WId id); + void closeWindow(TabBox::TabBoxClient *client); +private: + QList< QSharedPointer > m_windows; +}; +} // namespace KWin +#endif diff --git a/tabbox/tests/test_tabbox_clientmodel.cpp b/tabbox/tests/test_tabbox_clientmodel.cpp new file mode 100644 index 0000000000..cd2c963629 --- /dev/null +++ b/tabbox/tests/test_tabbox_clientmodel.cpp @@ -0,0 +1,47 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2012 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +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 General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#include "test_tabbox_clientmodel.h" +#include "mock_tabboxhandler.h" +#include "clientmodel.h" + +#include +using namespace KWin; + +void TestTabBoxClientModel::testLongestCaptionWithNullClient() +{ + MockTabBoxHandler tabboxhandler; + TabBox::ClientModel *clientModel = new TabBox::ClientModel(&tabboxhandler); + clientModel->createClientList(); + QCOMPARE(clientModel->longestCaption(), QString()); + // add a window to the mock + tabboxhandler.createMockWindow(QString("test"), 1); + clientModel->createClientList(); + QCOMPARE(clientModel->longestCaption(), QString("test")); + // delete the one client in the list + QModelIndex index = clientModel->index(0, 0); + QVERIFY(index.isValid()); + TabBox::TabBoxClient *client = static_cast(clientModel->data(index, TabBox::ClientModel::ClientRole).value()); + client->close(); + // internal model of ClientModel now contains a deleted pointer + // longestCaption should behave just as if the window were not in the list + QCOMPARE(clientModel->longestCaption(), QString()); +} + +QTEST_MAIN(TestTabBoxClientModel) diff --git a/tabbox/tests/test_tabbox_clientmodel.h b/tabbox/tests/test_tabbox_clientmodel.h new file mode 100644 index 0000000000..1bdf0d4aa4 --- /dev/null +++ b/tabbox/tests/test_tabbox_clientmodel.h @@ -0,0 +1,38 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2012 Martin Gräßlin + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU General Public License as published by +the Free Software Foundation; either version 2 of the License, or +(at your option) any later version. + +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 General Public License for more details. + +You should have received a copy of the GNU General Public License +along with this program. If not, see . +*********************************************************************/ +#ifndef TEST_TABBOX_CLIENT_MODEL_H +#define TEST_TABBOX_CLIENT_MODEL_H +#include + +class TestTabBoxClientModel : public QObject +{ + Q_OBJECT +private slots: + /** + * Tests that calculating the longest caption does not + * crash in case the internal m_clientList contains a weak + * pointer to a deleted TabBoxClient. + * + * See bug #303840 + **/ + void testLongestCaptionWithNullClient(); +}; + +#endif