[autotests] Use Xwayland class in WaylandTestApplication

Summary:
By using the new Xwayland class we can reduce code duplication.

An abstract parent class is introduced, that allows interfacing
with the Xwayland class from the test binaries.

Test Plan: Autotests still pass.

Reviewers: #kwin, davidedmundson

Reviewed By: #kwin, davidedmundson

Subscribers: kwin

Tags: #kwin

Differential Revision: https://phabricator.kde.org/D15022

autotests/integration/CMakeLists.txt

@@ -1,6 +1,6 @@
 add_subdirectory(helper)
 
-add_library(KWinIntegrationTestFramework STATIC kwin_wayland_test.cpp test_helpers.cpp)
+add_library(KWinIntegrationTestFramework STATIC ../../xwl/xwayland.cpp kwin_wayland_test.cpp test_helpers.cpp)
 target_link_libraries(KWinIntegrationTestFramework kwin Qt5::Test)
 
 function(integrationTest)

autotests/integration/kwin_wayland_test.cpp

@@ -24,6 +24,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 #include "../../wayland_server.h"
 #include "../../workspace.h"
 #include "../../xcbutils.h"
+#include "../../xwl/xwayland.h"
 
 #include <KPluginMetaData>
 
@@ -42,10 +43,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
 namespace KWin
 {
 
-static void readDisplay(int pipe);
-
 WaylandTestApplication::WaylandTestApplication(OperationMode mode, int &argc, char **argv)
-    : Application(mode, argc, argv)
+    : ApplicationWaylandAbstract(mode, argc, argv)
 {
     QStandardPaths::setTestModeEnabled(true);
     // TODO: add a test move to kglobalaccel instead?
@@ -72,6 +71,7 @@ WaylandTestApplication::WaylandTestApplication(OperationMode mode, int &argc, ch
     }
     initPlatform(plugins.first());
     WaylandServer::create(this);
+    setProcessStartupEnvironment(QProcessEnvironment::systemEnvironment());
 }
 
 WaylandTestApplication::~WaylandTestApplication()
@@ -85,24 +85,11 @@ WaylandTestApplication::~WaylandTestApplication()
     }
     destroyWorkspace();
     waylandServer()->dispatch();
-    disconnect(m_xwaylandFailConnection);
-    if (x11Connection()) {
-        Xcb::setInputFocus(XCB_INPUT_FOCUS_POINTER_ROOT);
-        destroyAtoms();
-        emit x11ConnectionAboutToBeDestroyed();
-        xcb_disconnect(x11Connection());
-        setX11Connection(nullptr);
-    }
-    if (m_xwaylandProcess) {
-        m_xwaylandProcess->terminate();
-        while (m_xwaylandProcess->state() != QProcess::NotRunning) {
-            processEvents(QEventLoop::WaitForMoreEvents);
-        }
-        waylandServer()->destroyXWaylandConnection();
-    }
     if (QStyle *s = style()) {
         s->unpolish(this);
     }
+    // kill Xwayland before terminating its connection
+    delete m_xwayland;
     waylandServer()->terminateClientConnections();
     destroyCompositor();
 }
@@ -142,7 +129,7 @@ void WaylandTestApplication::continueStartupWithScreens()
         return;
     }
     createCompositor();
-    connect(Compositor::self(), &Compositor::sceneCreated, this, &WaylandTestApplication::startXwaylandServer);
+    connect(Compositor::self(), &Compositor::sceneCreated, this, &WaylandTestApplication::continueStartupWithXwayland);
 }
 
 void WaylandTestApplication::continueStartupWithSceen()
@@ -151,165 +138,18 @@ void WaylandTestApplication::continueStartupWithSceen()
     createWorkspace();
 }
 
-void WaylandTestApplication::continueStartupWithX()
+void WaylandTestApplication::continueStartupWithXwayland()
 {
-    createX11Connection();
+    disconnect(Compositor::self(), &Compositor::sceneCreated, this, &WaylandTestApplication::continueStartupWithXwayland);
-    xcb_connection_t *c = x11Connection();
-    if (!c) {
-        // about to quit
-        return;
-    }
-    QSocketNotifier *notifier = new QSocketNotifier(xcb_get_file_descriptor(c), QSocketNotifier::Read, this);
-    auto processXcbEvents = [this, c] {
-        while (auto event = xcb_poll_for_event(c)) {
-            updateX11Time(event);
-            long result = 0;
-            if (QThread::currentThread()->eventDispatcher()->filterNativeEvent(QByteArrayLiteral("xcb_generic_event_t"), event, &result)) {
-                free(event);
-                continue;
-            }
-            if (Workspace::self()) {
-                Workspace::self()->workspaceEvent(event);
-            }
-            free(event);
-        }
-        xcb_flush(c);
-    };
-    connect(notifier, &QSocketNotifier::activated, this, processXcbEvents);
-    connect(QThread::currentThread()->eventDispatcher(), &QAbstractEventDispatcher::aboutToBlock, this, processXcbEvents);
-    connect(QThread::currentThread()->eventDispatcher(), &QAbstractEventDispatcher::awake, this, processXcbEvents);
 
-    // create selection owner for WM_S0 - magic X display number expected by XWayland
+    m_xwayland = new Xwl::Xwayland(this);
-    KSelectionOwner owner("WM_S0", c, x11RootWindow());
+    connect(m_xwayland, &Xwl::Xwayland::criticalError, this, [](int code) {
-    owner.claim(true);
+        // we currently exit on Xwayland errors always directly
-
+        // TODO: restart Xwayland
-    createAtoms();
+        std::cerr << "Xwayland had a critical error. Going to exit now." << std::endl;
-
+        exit(code);
-    setupEventFilters();
+    });
-
+    m_xwayland->init();
-    // Check  whether another windowmanager is running
-    const uint32_t maskValues[] = {XCB_EVENT_MASK_SUBSTRUCTURE_REDIRECT};
-    ScopedCPointer<xcb_generic_error_t> redirectCheck(xcb_request_check(connection(),
-                                                                        xcb_change_window_attributes_checked(connection(),
-                                                                                                                rootWindow(),
-                                                                                                                XCB_CW_EVENT_MASK,
-                                                                                                                maskValues)));
-    if (!redirectCheck.isNull()) {
-        ::exit(1);
-    }
-
-    createWorkspace();
-
-    Xcb::sync(); // Trigger possible errors, there's still a chance to abort
-}
-
-void WaylandTestApplication::createX11Connection()
-{
-    int screenNumber = 0;
-    xcb_connection_t *c = nullptr;
-    if (m_xcbConnectionFd == -1) {
-        c = xcb_connect(nullptr, &screenNumber);
-    } else {
-        c = xcb_connect_to_fd(m_xcbConnectionFd, nullptr);
-    }
-    if (int error = xcb_connection_has_error(c)) {
-        std::cerr << "FATAL ERROR: Creating connection to XServer failed: " << error << std::endl;
-        exit(1);
-        return;
-    }
-    setX11Connection(c);
-    // we don't support X11 multi-head in Wayland
-    setX11ScreenNumber(screenNumber);
-    setX11RootWindow(defaultScreen()->root);
-}
-
-void WaylandTestApplication::startXwaylandServer()
-{
-    disconnect(Compositor::self(), &Compositor::sceneCreated, this, &WaylandTestApplication::startXwaylandServer);
-    int pipeFds[2];
-    if (pipe(pipeFds) != 0) {
-        std::cerr << "FATAL ERROR failed to create pipe to start Xwayland " << std::endl;
-        exit(1);
-        return;
-    }
-    int sx[2];
-    if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sx) < 0) {
-        std::cerr << "FATAL ERROR: failed to open socket to open XCB connection" << std::endl;
-        exit(1);
-        return;
-    }
-    int fd = dup(sx[1]);
-    if (fd < 0) {
-        std::cerr << "FATAL ERROR: failed to open socket to open XCB connection" << std::endl;
-        exit(20);
-        return;
-    }
-
-    const int waylandSocket = waylandServer()->createXWaylandConnection();
-    if (waylandSocket == -1) {
-        std::cerr << "FATAL ERROR: failed to open socket for Xwayland" << std::endl;
-        exit(1);
-        return;
-    }
-    const int wlfd = dup(waylandSocket);
-    if (wlfd < 0) {
-        std::cerr << "FATAL ERROR: failed to open socket for Xwayland" << std::endl;
-        exit(20);
-        return;
-    }
-
-    m_xcbConnectionFd = sx[0];
-
-    m_xwaylandProcess = new QProcess(kwinApp());
-    m_xwaylandProcess->setProgram(QStringLiteral("Xwayland"));
-    QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
-    env.insert("WAYLAND_SOCKET", QByteArray::number(wlfd));
-    m_xwaylandProcess->setProcessEnvironment(env);
-    m_xwaylandProcess->setArguments({QStringLiteral("-displayfd"),
-                           QString::number(pipeFds[1]),
-                           QStringLiteral("-rootless"),
-                           QStringLiteral("-wm"),
-                           QString::number(fd)});
-    m_xwaylandFailConnection = connect(m_xwaylandProcess, static_cast<void (QProcess::*)(QProcess::ProcessError)>(&QProcess::error), this,
-        [] (QProcess::ProcessError error) {
-            if (error == QProcess::FailedToStart) {
-                std::cerr << "FATAL ERROR: failed to start Xwayland" << std::endl;
-            } else {
-                std::cerr << "FATAL ERROR: Xwayland failed, going to exit now" << std::endl;
-            }
-            exit(1);
-        }
-    );
-    const int xDisplayPipe = pipeFds[0];
-    connect(m_xwaylandProcess, &QProcess::started, this,
-        [this, xDisplayPipe] {
-            QFutureWatcher<void> *watcher = new QFutureWatcher<void>(this);
-            QObject::connect(watcher, &QFutureWatcher<void>::finished, this, &WaylandTestApplication::continueStartupWithX, Qt::QueuedConnection);
-            QObject::connect(watcher, &QFutureWatcher<void>::finished, watcher, &QFutureWatcher<void>::deleteLater, Qt::QueuedConnection);
-            watcher->setFuture(QtConcurrent::run(readDisplay, xDisplayPipe));
-        }
-    );
-    m_xwaylandProcess->start();
-    close(pipeFds[1]);
-}
-
-static void readDisplay(int pipe)
-{
-    QFile readPipe;
-    if (!readPipe.open(pipe, QIODevice::ReadOnly)) {
-        std::cerr << "FATAL ERROR failed to open pipe to start X Server" << std::endl;
-        exit(1);
-    }
-    QByteArray displayNumber = readPipe.readLine();
-
-    displayNumber.prepend(QByteArray(":"));
-    displayNumber.remove(displayNumber.size() -1, 1);
-    std::cout << "X-Server started on display " << displayNumber.constData() << std::endl;
-
-    setenv("DISPLAY", displayNumber.constData(), true);
-
-    // close our pipe
-    close(pipe);
 }
 
 }

autotests/integration/kwin_wayland_test.h

@@ -52,11 +52,15 @@ class XdgDecorationManager;
 
 namespace KWin
 {
+namespace Xwl
+{
+class Xwayland;
+}
 
 class AbstractClient;
 class ShellClient;
 
-class WaylandTestApplication : public Application
+class WaylandTestApplication : public ApplicationWaylandAbstract
 {
     Q_OBJECT
 public:
@@ -68,15 +72,11 @@ protected:
 
 private:
     void createBackend();
-    void createX11Connection();
     void continueStartupWithScreens();
     void continueStartupWithSceen();
-    void continueStartupWithX();
+    void continueStartupWithXwayland();
-    void startXwaylandServer();
 
-    int m_xcbConnectionFd = -1;
+    Xwl::Xwayland *m_xwayland = nullptr;
-    QProcess *m_xwaylandProcess = nullptr;
-    QMetaObject::Connection m_xwaylandFailConnection;
 };
 
 namespace Test

main.cpp

@@ -456,5 +456,14 @@ void Application::initPlatform(const KPluginMetaData &plugin)
     }
 }
 
+ApplicationWaylandAbstract::ApplicationWaylandAbstract(OperationMode mode, int &argc, char **argv)
+    : Application(mode, argc, argv)
+{
+}
+
+ApplicationWaylandAbstract::~ApplicationWaylandAbstract()
+{
+}
+
 } // namespace
 

main.h

@@ -268,6 +268,27 @@ inline static Application *kwinApp()
     return static_cast<Application*>(QCoreApplication::instance());
 }
 
+namespace Xwl
+{
+class Xwayland;
+}
+
+class KWIN_EXPORT ApplicationWaylandAbstract : public Application
+{
+    Q_OBJECT
+public:
+    virtual ~ApplicationWaylandAbstract() = 0;
+protected:
+    friend class Xwl::Xwayland;
+
+    ApplicationWaylandAbstract(OperationMode mode, int &argc, char **argv);
+    virtual void setProcessStartupEnvironment(const QProcessEnvironment &environment) {
+        Q_UNUSED(environment);
+    }
+    virtual void startSession() {}
+};
+
+
 } // namespace
 
 #endif

main_wayland.cpp

@@ -110,7 +110,7 @@ void gainRealTime(RealTimeFlags flags = RealTimeFlags::DontReset)
 //************************************
 
 ApplicationWayland::ApplicationWayland(int &argc, char **argv)
-    : Application(OperationModeWaylandOnly, argc, argv)
+    : ApplicationWaylandAbstract(OperationModeWaylandOnly, argc, argv)
 {
 }
 
@@ -213,7 +213,7 @@ void ApplicationWayland::startSession()
     if (!m_inputMethodServerToStart.isEmpty()) {
         int socket = dup(waylandServer()->createInputMethodConnection());
         if (socket >= 0) {
-            QProcessEnvironment environment = m_environment;
+            QProcessEnvironment environment = processStartupEnvironment();
             environment.insert(QStringLiteral("WAYLAND_SOCKET"), QByteArray::number(socket));
             environment.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("wayland"));
             environment.remove("DISPLAY");
@@ -239,7 +239,7 @@ void ApplicationWayland::startSession()
     if (!m_sessionArgument.isEmpty()) {
         QProcess *p = new Process(this);
         p->setProcessChannelMode(QProcess::ForwardedErrorChannel);
-        p->setProcessEnvironment(m_environment);
+        p->setProcessEnvironment(processStartupEnvironment());
         auto finishedSignal = static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished);
         connect(p, finishedSignal, this, &ApplicationWayland::quit);
         p->start(m_sessionArgument);
@@ -251,7 +251,7 @@ void ApplicationWayland::startSession()
             // this is going to happen anyway as we are the wayland and X server the app connects to
             QProcess *p = new Process(this);
             p->setProcessChannelMode(QProcess::ForwardedErrorChannel);
-            p->setProcessEnvironment(m_environment);
+            p->setProcessEnvironment(processStartupEnvironment());
             p->start(application);
         }
     }

main_wayland.h

@@ -29,7 +29,7 @@ namespace Xwl
 class Xwayland;
 }
 
-class ApplicationWayland : public Application
+class ApplicationWayland : public ApplicationWaylandAbstract
 {
     Q_OBJECT
 public:
@@ -45,7 +45,7 @@ public:
     void setInputMethodServerToStart(const QString &inputMethodServer) {
         m_inputMethodServerToStart = inputMethodServer;
     }
-    void setProcessStartupEnvironment(const QProcessEnvironment &environment) {
+    void setProcessStartupEnvironment(const QProcessEnvironment &environment) override {
         m_environment = environment;
     }
     void setSessionArgument(const QString &session) {
@@ -60,13 +60,11 @@ protected:
     void performStartup() override;
 
 private:
-    friend class Xwl::Xwayland;
-
     void createBackend();
     void continueStartupWithScreens();
     void continueStartupWithSceen();
     void continueStartupWithXwayland();
-    void startSession();
+    void startSession() override;
 
     bool m_startXWayland = false;
     QStringList m_applicationsToStart;

xwl/xwayland.cpp

@@ -69,7 +69,7 @@ namespace KWin {
 namespace Xwl
 {
 
-Xwayland::Xwayland(ApplicationWayland *app, QObject *parent)
+Xwayland::Xwayland(ApplicationWaylandAbstract *app, QObject *parent)
     : QObject(parent),
       m_app(app)
 {

xwl/xwayland.h

@@ -28,7 +28,7 @@ class xcb_screen_t;
 
 namespace KWin
 {
-class ApplicationWayland;
+class ApplicationWaylandAbstract;
 
 namespace Xwl
 {
@@ -37,7 +37,7 @@ class Xwayland : public QObject
 {
     Q_OBJECT
 public:
-    Xwayland(ApplicationWayland *app, QObject *parent = nullptr);
+    Xwayland(ApplicationWaylandAbstract *app, QObject *parent = nullptr);
     virtual ~Xwayland();
 
     void init();
@@ -53,7 +53,7 @@ private:
     QProcess *m_xwaylandProcess = nullptr;
     QMetaObject::Connection m_xwaylandFailConnection;
 
-    ApplicationWayland *m_app;
+    ApplicationWaylandAbstract *m_app;
 };
 
 }