From bd158a632100ae8186abdcc5853bb888d02105de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= <mgraesslin@kde.org>
Date: Sun, 23 Jul 2017 16:18:09 +0200
Subject: [PATCH] Reset last_active_client when a ShellClient is removed

Summary:
The last_active_client is set when an AbstractClient gets activated. For
the X11 case the last_active_client was getting reset to nullptr when
the last_active_client gets destroyed. But for the ShellClient that did
not yet happen. This could result in a crash.

This change addresses the problem and adds a test case which triggered
the crash. The condition of the crash are difficult to generate though -
it took me about an hour to write the test for the crash.

1. Wayland client must be active
2. Explicit focus to null (no active client)
3. destroy Wayland window
4. X11 client which sets focus on itself without interaction with window
  manager

Test Plan: test case no longer crashes

Reviewers: #kwin, #plasma

Subscribers: plasma-devel, kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D6852
---
 autotests/integration/x11_client_test.cpp | 61 +++++++++++++++++++++++
 workspace.cpp                             |  3 ++
 2 files changed, 64 insertions(+)

diff --git a/autotests/integration/x11_client_test.cpp b/autotests/integration/x11_client_test.cpp
index e44d06af18..5d65bb2ac9 100644
--- a/autotests/integration/x11_client_test.cpp
+++ b/autotests/integration/x11_client_test.cpp
@@ -49,6 +49,7 @@ private Q_SLOTS:
 
     void testCaptionSimplified();
     void testFullscreenLayerWithActiveWaylandWindow();
+    void testFocusInWithWaylandLastActiveWindow();
 };
 
 void X11ClientTest::initTestCase()
@@ -59,6 +60,7 @@ void X11ClientTest::initTestCase()
     QVERIFY(workspaceCreatedSpy.isValid());
     kwinApp()->platform()->setInitialWindowSize(QSize(1280, 1024));
     QVERIFY(waylandServer()->init(s_socketName.toLocal8Bit()));
+    kwinApp()->setConfig(KSharedConfig::openConfig(QString(), KConfig::SimpleConfig));
 
     kwinApp()->start();
     QVERIFY(workspaceCreatedSpy.wait());
@@ -212,5 +214,64 @@ void X11ClientTest::testFullscreenLayerWithActiveWaylandWindow()
     xcb_flush(c.data());
 }
 
+void X11ClientTest::testFocusInWithWaylandLastActiveWindow()
+{
+    // this test verifies that Workspace::allowClientActivation does not crash if last client was a Wayland client
+
+    // create an X11 window
+    QScopedPointer<xcb_connection_t, XcbConnectionDeleter> c(xcb_connect(nullptr, nullptr));
+    QVERIFY(!xcb_connection_has_error(c.data()));
+    const QRect windowGeometry(0, 0, 100, 200);
+    xcb_window_t w = xcb_generate_id(c.data());
+    xcb_create_window(c.data(), XCB_COPY_FROM_PARENT, w, rootWindow(),
+                      windowGeometry.x(),
+                      windowGeometry.y(),
+                      windowGeometry.width(),
+                      windowGeometry.height(),
+                      0, XCB_WINDOW_CLASS_INPUT_OUTPUT, XCB_COPY_FROM_PARENT, 0, nullptr);
+    xcb_size_hints_t hints;
+    memset(&hints, 0, sizeof(hints));
+    xcb_icccm_size_hints_set_position(&hints, 1, windowGeometry.x(), windowGeometry.y());
+    xcb_icccm_size_hints_set_size(&hints, 1, windowGeometry.width(), windowGeometry.height());
+    xcb_icccm_set_wm_normal_hints(c.data(), w, &hints);
+    xcb_map_window(c.data(), w);
+    xcb_flush(c.data());
+
+    // we should get a client for it
+    QSignalSpy windowCreatedSpy(workspace(), &Workspace::clientAdded);
+    QVERIFY(windowCreatedSpy.isValid());
+    QVERIFY(windowCreatedSpy.wait());
+    Client *client = windowCreatedSpy.first().first().value<Client*>();
+    QVERIFY(client);
+    QCOMPARE(client->window(), w);
+    QVERIFY(client->isActive());
+
+    // create Wayland window
+    QScopedPointer<Surface> surface(Test::createSurface());
+    QScopedPointer<ShellSurface> shellSurface(Test::createShellSurface(surface.data()));
+    auto waylandClient = Test::renderAndWaitForShown(surface.data(), QSize(100, 50), Qt::blue);
+    QVERIFY(waylandClient);
+    QVERIFY(waylandClient->isActive());
+    // activate no window
+    workspace()->setActiveClient(nullptr);
+    QVERIFY(!waylandClient->isActive());
+    QVERIFY(!workspace()->activeClient());
+    // and close Wayland window again
+    shellSurface.reset();
+    surface.reset();
+    QVERIFY(Test::waitForWindowDestroyed(waylandClient));
+
+    // and try to activate the x11 client through X11 api
+    const auto cookie = xcb_set_input_focus_checked(c.data(), XCB_INPUT_FOCUS_NONE, w, XCB_CURRENT_TIME);
+    auto error = xcb_request_check(c.data(), cookie);
+    QVERIFY(!error);
+    // this accesses last_active_client on trying to activate
+    QTRY_VERIFY(client->isActive());
+
+    // and destroy the window again
+    xcb_unmap_window(c.data(), w);
+    xcb_flush(c.data());
+}
+
 WAYLANDTEST_MAIN(X11ClientTest)
 #include "x11_client_test.moc"
diff --git a/workspace.cpp b/workspace.cpp
index 5f60a6af9e..b0a6dae828 100644
--- a/workspace.cpp
+++ b/workspace.cpp
@@ -428,6 +428,9 @@ void Workspace::init()
                 if (c == delayfocus_client) {
                     cancelDelayFocus();
                 }
+                if (c == last_active_client) {
+                    last_active_client = nullptr;
+                }
                 clientHidden(c);
                 emit clientRemoved(c);
                 markXStackingOrderAsDirty();