From 02fbeeae78188c6e7aa4dd90c582faef1d678c01 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Mon, 22 Jul 2024 15:11:25 +0300 Subject: [PATCH] Make Workspace::desktopResized() reassign outputs of uninitialized windows If an output is deleted, the Workspace::desktopResized() is going to re-assign windows to the new outputs. It is done so so the workspace re-arrangement procedure is deterministic and has concrete order. However, with the current Window lifecycle management, it's possible to encounter the follwing case: - xdg_toplevel gets created on output A - xdg_toplevel initial state is committed - output A is removed - a wl_buffer is attached to the xdg_toplevel, which results in a geometry change and an output change - Window::setMoveResizeOutput() is called, but the previous output is a dangling pointer CCBUG: 489632 --- autotests/integration/CMakeLists.txt | 2 + autotests/integration/workspace_test.cpp | 115 +++++++++++++++++++++++ src/workspace.cpp | 9 ++ 3 files changed, 126 insertions(+) create mode 100644 autotests/integration/workspace_test.cpp diff --git a/autotests/integration/CMakeLists.txt b/autotests/integration/CMakeLists.txt index 3a5a9b62af..7233a22d7d 100644 --- a/autotests/integration/CMakeLists.txt +++ b/autotests/integration/CMakeLists.txt @@ -164,6 +164,8 @@ integrationTest(NAME testXwaylandServerRestart SRCS xwaylandserver_restart_test. integrationTest(NAME testFakeInput SRCS fakeinput_test.cpp) integrationTest(NAME testSecurityContext SRCS security_context_test.cpp) integrationTest(NAME testStickyKeys SRCS sticky_keys_test.cpp) +integrationTest(NAME testWorkspace SRCS workspace_test.cpp) + if(KWIN_BUILD_X11) integrationTest(NAME testDontCrashEmptyDeco SRCS dont_crash_empty_deco.cpp LIBS KDecoration2::KDecoration) integrationTest(NAME testXwaylandSelections SRCS xwayland_selections_test.cpp) diff --git a/autotests/integration/workspace_test.cpp b/autotests/integration/workspace_test.cpp new file mode 100644 index 0000000000..270426abff --- /dev/null +++ b/autotests/integration/workspace_test.cpp @@ -0,0 +1,115 @@ +/* + SPDX-FileCopyrightText: 2024 Vlad Zahorodnii + + SPDX-License-Identifier: GPL-2.0-or-later +*/ + +#include "kwin_wayland_test.h" + +#include "core/outputconfiguration.h" +#include "pointer_input.h" +#include "wayland_server.h" +#include "workspace.h" + +#include + +using namespace KWin; + +static const QString s_socketName = QStringLiteral("wayland_test_kwin_workspace-0"); + +class WorkspaceTest : public QObject +{ + Q_OBJECT + +private Q_SLOTS: + void initTestCase(); + void init(); + void cleanup(); + + void evacuateMappedWindowFromRemovedOutput(); + void evacuateUnmappedWindowFromRemovedOutput(); +}; + +void WorkspaceTest::initTestCase() +{ + QSignalSpy applicationStartedSpy(kwinApp(), &Application::started); + QVERIFY(waylandServer()->init(s_socketName)); + kwinApp()->start(); + QVERIFY(applicationStartedSpy.wait()); +} + +void WorkspaceTest::init() +{ + Test::setOutputConfig({ + QRect(0, 0, 1280, 1024), + QRect(1280, 0, 1280, 1024), + }); + + QVERIFY(Test::setupWaylandConnection()); + + workspace()->setActiveOutput(QPoint(640, 512)); + input()->pointer()->warp(QPoint(640, 512)); +} + +void WorkspaceTest::cleanup() +{ + Test::destroyWaylandConnection(); +} + +void WorkspaceTest::evacuateMappedWindowFromRemovedOutput() +{ + // This test verifies that a window will be evacuated to another output if the output it is + // currently on has been either removed or disabled. + + const auto firstOutput = workspace()->outputs()[0]; + const auto secondOutput = workspace()->outputs()[1]; + + std::unique_ptr surface(Test::createSurface()); + std::unique_ptr shellSurface(Test::createXdgToplevelSurface(surface.get())); + auto window = Test::renderAndWaitForShown(surface.get(), QSize(100, 50), Qt::blue); + QCOMPARE(window->output(), firstOutput); + QCOMPARE(window->moveResizeOutput(), firstOutput); + + QSignalSpy outputChangedSpy(window, &Window::outputChanged); + { + OutputConfiguration config; + config.changeSet(firstOutput)->enabled = false; + workspace()->applyOutputConfiguration(config); + } + QCOMPARE(outputChangedSpy.count(), 1); + QCOMPARE(window->output(), secondOutput); + QCOMPARE(window->moveResizeOutput(), secondOutput); +} + +void WorkspaceTest::evacuateUnmappedWindowFromRemovedOutput() +{ + // This test verifies that a window, which is not fully managed by the Workspace yet, will be + // evacuated to another output if the output it is currently on has been withdrawn. + + const auto firstOutput = workspace()->outputs()[0]; + const auto secondOutput = workspace()->outputs()[1]; + + std::unique_ptr surface(Test::createSurface()); + std::unique_ptr shellSurface(Test::createXdgToplevelSurface(surface.get(), Test::CreationSetup::CreateOnly)); + + surface->commit(KWayland::Client::Surface::CommitFlag::None); + QVERIFY(Test::waylandSync()); + QCOMPARE(waylandServer()->windows().count(), 1); + Window *window = waylandServer()->windows().constFirst(); + QVERIFY(!window->readyForPainting()); + QCOMPARE(window->output(), firstOutput); + QCOMPARE(window->moveResizeOutput(), firstOutput); + + QSignalSpy outputChangedSpy(window, &Window::outputChanged); + { + OutputConfiguration config; + config.changeSet(firstOutput)->enabled = false; + workspace()->applyOutputConfiguration(config); + } + QCOMPARE(outputChangedSpy.count(), 1); + QCOMPARE(window->output(), secondOutput); + QCOMPARE(window->moveResizeOutput(), secondOutput); +} + +WAYLANDTEST_MAIN(WorkspaceTest) +#include "workspace_test.moc" diff --git a/src/workspace.cpp b/src/workspace.cpp index 35ffc23df3..7edf6faeaa 100644 --- a/src/workspace.cpp +++ b/src/workspace.cpp @@ -2153,6 +2153,15 @@ void Workspace::desktopResized() window->setOutput(outputAt(window->frameGeometry().center())); } + if (waylandServer()) { + // TODO: Track uninitialized windows in the Workspace too. + const auto windows = waylandServer()->windows(); + for (Window *window : windows) { + window->setMoveResizeOutput(outputAt(window->moveResizeGeometry().center())); + window->setOutput(outputAt(window->frameGeometry().center())); + } + } + // restore cursor position const auto oldCursorOutput = std::find_if(m_oldScreenGeometries.cbegin(), m_oldScreenGeometries.cend(), [](const auto &geometry) { return exclusiveContains(geometry, Cursors::self()->mouse()->pos());