diff --git a/autotests/integration/CMakeLists.txt b/autotests/integration/CMakeLists.txt index f977e9970d..b4a56a3fa6 100644 --- a/autotests/integration/CMakeLists.txt +++ b/autotests/integration/CMakeLists.txt @@ -57,6 +57,7 @@ integrationTest(WAYLAND_ONLY NAME testShellClientRules SRCS shell_client_rules_t integrationTest(WAYLAND_ONLY NAME testIdleInhibition SRCS idle_inhibition_test.cpp) integrationTest(WAYLAND_ONLY NAME testColorCorrectNightColor SRCS colorcorrect_nightcolor_test.cpp) integrationTest(WAYLAND_ONLY NAME testDontCrashCursorPhysicalSizeEmpty SRCS dont_crash_cursor_physical_size_empty.cpp) +integrationTest(WAYLAND_ONLY NAME testDontCrashReinitializeCompositor SRCS dont_crash_reinitialize_compositor.cpp) if (XCB_ICCCM_FOUND) integrationTest(NAME testMoveResize SRCS move_resize_window_test.cpp LIBS XCB::ICCCM) diff --git a/autotests/integration/dont_crash_reinitialize_compositor.cpp b/autotests/integration/dont_crash_reinitialize_compositor.cpp new file mode 100644 index 0000000000..f09e2705c6 --- /dev/null +++ b/autotests/integration/dont_crash_reinitialize_compositor.cpp @@ -0,0 +1,150 @@ +/******************************************************************** +KWin - the KDE window manager +This file is part of the KDE project. + +Copyright (C) 2018 Vlad Zagorodniy + +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 "kwin_wayland_test.h" + +#include "abstract_client.h" +#include "composite.h" +#include "deleted.h" +#include "effects.h" +#include "platform.h" +#include "screens.h" +#include "shell_client.h" +#include "wayland_server.h" +#include "workspace.h" + +#include +#include + +namespace KWin +{ + +static const QString s_socketName = QStringLiteral("wayland_test_kwin_dont_crash_reinitialize_compositor-0"); + +class DontCrashReinitializeCompositorTest : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void initTestCase(); + void init(); + void cleanup(); + + void testReinitializeCompositor_data(); + void testReinitializeCompositor(); +}; + +void DontCrashReinitializeCompositorTest::initTestCase() +{ + qputenv("XDG_DATA_DIRS", QCoreApplication::applicationDirPath().toUtf8()); + + qRegisterMetaType(); + qRegisterMetaType(); + qRegisterMetaType(); + QSignalSpy workspaceCreatedSpy(kwinApp(), &Application::workspaceCreated); + QVERIFY(workspaceCreatedSpy.isValid()); + kwinApp()->platform()->setInitialWindowSize(QSize(1280, 1024)); + QMetaObject::invokeMethod(kwinApp()->platform(), "setVirtualOutputs", Qt::DirectConnection, Q_ARG(int, 2)); + QVERIFY(waylandServer()->init(s_socketName.toLocal8Bit())); + + qputenv("KWIN_COMPOSE", QByteArrayLiteral("O2")); + qputenv("KWIN_EFFECTS_FORCE_ANIMATIONS", QByteArrayLiteral("1")); + + kwinApp()->start(); + QVERIFY(workspaceCreatedSpy.wait()); + QCOMPARE(screens()->count(), 2); + QCOMPARE(screens()->geometry(0), QRect(0, 0, 1280, 1024)); + QCOMPARE(screens()->geometry(1), QRect(1280, 0, 1280, 1024)); + waylandServer()->initWorkspace(); +} + +void DontCrashReinitializeCompositorTest::init() +{ + QVERIFY(Test::setupWaylandConnection()); +} + +void DontCrashReinitializeCompositorTest::cleanup() +{ + Test::destroyWaylandConnection(); +} + +void DontCrashReinitializeCompositorTest::testReinitializeCompositor_data() +{ + QTest::addColumn("effectName"); + + QTest::newRow("Fade") << QStringLiteral("kwin4_effect_fade"); + QTest::newRow("Glide") << QStringLiteral("glide"); + QTest::newRow("Scale") << QStringLiteral("kwin4_effect_scale"); +} + +void DontCrashReinitializeCompositorTest::testReinitializeCompositor() +{ + // This test verifies that KWin doesn't crash when the compositor settings + // have been changed while a scripted effect animates the disappearing of + // a window. + + // Make sure that we have the right effects ptr. + auto effectsImpl = qobject_cast(effects); + QVERIFY(effectsImpl); + + // Unload all effects. + effectsImpl->unloadAllEffects(); + QVERIFY(effectsImpl->loadedEffects().isEmpty()); + + // Create the test client. + using namespace KWayland::Client; + + QScopedPointer surface(Test::createSurface()); + QVERIFY(!surface.isNull()); + QScopedPointer shellSurface(Test::createXdgShellStableSurface(surface.data())); + QVERIFY(!shellSurface.isNull()); + ShellClient *client = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue); + QVERIFY(client); + + // Make sure that only the test effect is loaded. + QFETCH(QString, effectName); + QVERIFY(effectsImpl->loadEffect(effectName)); + QCOMPARE(effectsImpl->loadedEffects().count(), 1); + QCOMPARE(effectsImpl->loadedEffects().first(), effectName); + Effect *effect = effectsImpl->findEffect(effectName); + QVERIFY(effect); + QVERIFY(!effect->isActive()); + + // Close the test client. + QSignalSpy windowClosedSpy(client, &ShellClient::windowClosed); + QVERIFY(windowClosedSpy.isValid()); + shellSurface.reset(); + surface.reset(); + QVERIFY(windowClosedSpy.wait()); + + // The test effect should start animating the test client. Is there a better + // way to verify that the test effect actually animates the test client? + QVERIFY(effect->isActive()); + + // Re-initialize the compositor, effects will be destroyed and created again. + Compositor::self()->slotReinitialize(); + + // By this time, KWin should still be alive. +} + +} // namespace KWin + +WAYLANDTEST_MAIN(KWin::DontCrashReinitializeCompositorTest) +#include "dont_crash_reinitialize_compositor.moc" diff --git a/composite.cpp b/composite.cpp index 932c914530..16b3b6a644 100644 --- a/composite.cpp +++ b/composite.cpp @@ -368,6 +368,13 @@ void Compositor::finish() return; m_finishing = true; m_releaseSelectionTimer.start(); + + // Some effects might need access to effect windows when they are about to + // be destroyed, for example to unreference deleted windows, so we have to + // make sure that effect windows outlive effects. + delete effects; + effects = nullptr; + if (Workspace::self()) { foreach (Client * c, Workspace::self()->clientList()) m_scene->windowClosed(c, NULL); @@ -403,8 +410,6 @@ void Compositor::finish() c->finishCompositing(); } } - delete effects; - effects = NULL; delete m_scene; m_scene = NULL; compositeTimer.stop(); diff --git a/effects.cpp b/effects.cpp index 4df33fd541..f787c2748a 100644 --- a/effects.cpp +++ b/effects.cpp @@ -1654,6 +1654,19 @@ KSharedConfigPtr EffectsHandlerImpl::inputConfig() const return kwinApp()->inputConfig(); } +Effect *EffectsHandlerImpl::findEffect(const QString &name) const +{ + auto it = std::find_if(loaded_effects.constBegin(), loaded_effects.constEnd(), + [name] (const EffectPair &pair) { + return pair.first == name; + } + ); + if (it == loaded_effects.constEnd()) { + return nullptr; + } + return (*it).second; +} + //**************************************** // EffectWindowImpl //**************************************** diff --git a/effects.h b/effects.h index e7edf04553..b678b77423 100644 --- a/effects.h +++ b/effects.h @@ -262,6 +262,15 @@ public: void windowToDesktops(EffectWindow *w, const QVector &desktops); + /** + * Finds an effect with the given name. + * + * @param name The name of the effect. + * @returns The effect with the given name @p name, or nullptr if there + * is no such effect loaded. + **/ + Effect *findEffect(const QString &name) const; + public Q_SLOTS: void slotCurrentTabAboutToChange(EffectWindow* from, EffectWindow* to); void slotTabAdded(EffectWindow* from, EffectWindow* to);