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,4 +1,5 @@
 add_subdirectory( qml )
+add_subdirectory( tests )
 
 # Install the KWin/WindowSwitcher service type
 install( FILES kwinwindowswitcher.desktop DESTINATION ${SERVICETYPES_INSTALL_DIR} )

tabbox/clientmodel.cpp

@@ -93,6 +93,9 @@ QString ClientModel::longestCaption() const
     QString caption;
     foreach (QWeakPointer<TabBoxClient> clientPointer, m_clientList) {
         QSharedPointer<TabBoxClient> client = clientPointer.toStrongRef();
+        if (!client) {
+            continue;
+        }
         if (client->caption().size() > caption.size()) {
             caption = client->caption();
         }

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} )

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 <mgraesslin@kde.org>
+
+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 <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#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"

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 <mgraesslin@kde.org>
+
+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 <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#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<MockTabBoxHandler*>(TabBox::tabBox)->closeWindow(this);
+}
+
+} // namespace KWin

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 <mgraesslin@kde.org>
+
+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 <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#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

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 <mgraesslin@kde.org>
+
+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 <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#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<TabBox::TabBoxClient>();
+}
+
+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

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 <mgraesslin@kde.org>
+
+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 <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#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<TabBox::TabBoxClient>();
+    }
+    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<Window>();
+    }
+    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<TabBox::TabBoxClient> createMockWindow(const QString &caption, WId id);
+    void closeWindow(TabBox::TabBoxClient *client);
+private:
+    QList< QSharedPointer<TabBox::TabBoxClient> > m_windows;
+};
+} // namespace KWin
+#endif

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 <mgraesslin@kde.org>
+
+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 <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#include "test_tabbox_clientmodel.h"
+#include "mock_tabboxhandler.h"
+#include "clientmodel.h"
+
+#include <QtTest/QtTest>
+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<TabBox::TabBoxClient *>(clientModel->data(index, TabBox::ClientModel::ClientRole).value<void*>());
+    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)

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 <mgraesslin@kde.org>
+
+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 <http://www.gnu.org/licenses/>.
+*********************************************************************/
+#ifndef TEST_TABBOX_CLIENT_MODEL_H
+#define TEST_TABBOX_CLIENT_MODEL_H
+#include <QObject>
+
+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