Commit decf7c9d authored by Roman Gilg's avatar Roman Gilg

Take file descriptor only instead of whole KWayland Display

Summary:
The current KScreenLocker expects the Compositor to hand over the
KWayland::Server::Display pointer.

This has several disadvantages:
* In KScreenLocker functionality is duplicated which the compositor
  probably has setup already (creating client connections manually).
* The Display object has a way larger scope than what KScreenLocker
  actually needs.
* Ownership of the Display is unclear in regards to memory but also
  what the compositor is still allowed to do in comparision to KScreenLocker
  with the Display.
* KScreenLocker can only be integrated in compositors using KWayland for
  managing their wl_display protocol object.

Instead it is now enough to hand over a single file descriptor KScreenLocker
can use as its client endpoint to talk to the compositor.

Test Plan: Manually in Wayland session, autotest.

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: apol

Differential Revision: https://phabricator.kde.org/D28082
parent c893e333
......@@ -20,6 +20,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include <QtTest>
#include "../ksldapp.h"
#include <KWayland/Server/clientconnection.h>
#include <KWayland/Server/compositor_interface.h>
#include <KWayland/Server/datadevicemanager_interface.h>
#include <KWayland/Server/display.h>
......@@ -28,6 +29,8 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include <KWayland/Server/shell_interface.h>
#include <KWayland/Server/plasmashell_interface.h>
#include <sys/socket.h>
using namespace KWayland::Server;
class NoScreensTest : public QObject
......@@ -37,7 +40,7 @@ Q_SIGNALS:
void surfaceShown();
private Q_SLOTS:
void initTestCase();
void init();
void cleanup();
void testAllQScreensClose();
......@@ -49,12 +52,16 @@ private:
SeatInterface *m_seat = nullptr;
PlasmaShellInterface *m_plasma = nullptr;
SurfaceInterface *m_surface = nullptr;
ClientConnection *m_clientConnection = nullptr;
};
void NoScreensTest::initTestCase()
void NoScreensTest::init()
{
m_display = new Display(this);
m_display = new Display;
m_display->setAutomaticSocketNaming(true);
m_display->start(Display::StartMode::ConnectClientsOnly);
QVERIFY(m_display->isRunning());
m_compositor = m_display->createCompositor(m_display);
m_compositor->create();
m_shell = m_display->createShell(m_display);
......@@ -70,12 +77,40 @@ void NoScreensTest::initTestCase()
m_plasma->create();
QCoreApplication::instance()->setProperty("platformName", QStringLiteral("wayland"));
ScreenLocker::KSldApp::self();
ScreenLocker::KSldApp::self()->setWaylandDisplay(m_display);
auto *app = ScreenLocker::KSldApp::self();
QProcessEnvironment env = QProcessEnvironment::systemEnvironment();
env.insert(QStringLiteral("QT_QPA_PLATFORM"), QStringLiteral("wayland"));
ScreenLocker::KSldApp::self()->setGreeterEnvironment(env);
ScreenLocker::KSldApp::self()->initialize();
app->setGreeterEnvironment(env);
app->initialize();
connect(app, &ScreenLocker::KSldApp::aboutToLock,
this, [this, app] {
int sx[2];
QVERIFY(socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sx) >= 0);
m_clientConnection = m_display->createClient(sx[0]);
QVERIFY(m_clientConnection);
connect(m_clientConnection, &ClientConnection::disconnected, this, [this] {
// The API is currently ill-defined about the ownership of a client connection
// when calling createClient server-side.
m_clientConnection = nullptr;
});
app->setWaylandFd(sx[1]);
connect(m_seat, &SeatInterface::timestampChanged,
app, &ScreenLocker::KSldApp::userActivity);
});
connect(app, &ScreenLocker::KSldApp::unlocked,
this, [this, app] {
if (m_clientConnection) {
delete m_clientConnection;
m_clientConnection = nullptr;
}
disconnect(m_seat, &SeatInterface::timestampChanged,
app, &ScreenLocker::KSldApp::userActivity);
});
connect(m_shell, &ShellInterface::surfaceCreated, this,
[this] (ShellSurfaceInterface *surface) {
m_surface = surface->surface();
......@@ -93,7 +128,7 @@ void NoScreensTest::initTestCase()
void NoScreensTest::cleanup()
{
unlock();
delete m_display;
}
void NoScreensTest::unlock()
......@@ -116,8 +151,6 @@ void NoScreensTest::testAllQScreensClose()
}
QSignalSpy lockedStateSpy(ScreenLocker::KSldApp::self(), &ScreenLocker::KSldApp::lockStateChanged);
QVERIFY(lockedStateSpy.isValid());
QSignalSpy greeterConnectionSpy(ScreenLocker::KSldApp::self(), &ScreenLocker::KSldApp::greeterClientConnectionChanged);
QVERIFY(greeterConnectionSpy.isValid());
QSignalSpy surfaceShownSpy(this, &NoScreensTest::surfaceShown);
QVERIFY(surfaceShownSpy.isValid());
......@@ -131,11 +164,11 @@ void NoScreensTest::testAllQScreensClose()
ScreenLocker::KSldApp::self()->lock(ScreenLocker::EstablishLock::Immediate);
QCOMPARE(lockedStateSpy.count(), 1);
QCOMPARE(greeterConnectionSpy.count(), 1);
QCOMPARE(ScreenLocker::KSldApp::self()->lockState(), ScreenLocker::KSldApp::AcquiringLock);
QVERIFY(surfaceShownSpy.wait());
QCOMPARE(ScreenLocker::KSldApp::self()->lockState(), ScreenLocker::KSldApp::Locked);
QVERIFY(m_clientConnection);
// give some time to show the window
QTest::qWait(5000);
......
......@@ -38,10 +38,6 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include <KNotification>
#include <KGlobalAccel>
//kwayland
#include <KWayland/Server/display.h>
#include <KWayland/Server/clientconnection.h>
// Qt
#include <QAction>
#include <QKeyEvent>
......@@ -83,7 +79,6 @@ KSldApp::KSldApp(QObject * parent)
, m_lockProcess(nullptr)
, m_lockWindow(nullptr)
, m_waylandServer(new WaylandServer(this))
, m_waylandDisplay(nullptr)
, m_lockedTimer(QElapsedTimer())
, m_idleId(0)
, m_lockGrace(0)
......@@ -213,14 +208,9 @@ void KSldApp::initialize()
auto finishedSignal = static_cast<void (QProcess::*)(int, QProcess::ExitStatus)>(&QProcess::finished);
connect(m_lockProcess, finishedSignal, this,
[this](int exitCode, QProcess::ExitStatus exitStatus) {
qDebug() << "Greeter process exitted with status:"
<< exitStatus << "exit code:" << exitCode;
if (m_isWayland && m_waylandDisplay && m_greeterClientConnection) {
m_greeterClientConnection->destroy();
}
const bool regularExit = !exitCode && exitStatus == QProcess::NormalExit;
if (regularExit || s_graceTimeKill || s_logindExit) {
// unlock process finished successfully - we can remove the lock grab
......@@ -451,6 +441,9 @@ public:
bool KSldApp::establishGrab()
{
if (m_isWayland) {
return m_waylandFd >= 0;
}
if (!m_isX11) {
return true;
}
......@@ -584,33 +577,22 @@ bool KSldApp::isFdoPowerInhibited() const
return m_powerManagementInhibition->isInhibited();
}
void KSldApp::setWaylandFd(int fd)
{
m_waylandFd = fd;
}
void KSldApp::startLockProcess(EstablishLock establishLock)
{
QProcessEnvironment env = m_greeterEnv;
if (m_isWayland && m_waylandDisplay) {
int sx[2];
if (socketpair(AF_UNIX, SOCK_STREAM | SOCK_CLOEXEC, 0, sx) < 0) {
qWarning() << "Can not create socket";
emit m_lockProcess->errorOccurred(QProcess::FailedToStart);
return;
}
m_greeterClientConnection = m_waylandDisplay->createClient(sx[0]);
connect(m_greeterClientConnection, &QObject::destroyed, this,
[this] (QObject *destroyedObject) {
if (destroyedObject != m_greeterClientConnection) {
return;
}
m_greeterClientConnection = nullptr;
emit greeterClientConnectionChanged();
}
);
emit greeterClientConnectionChanged();
int socket = dup(sx[1]);
if (m_isWayland && m_waylandFd >= 0) {
int socket = dup(m_waylandFd);
if (socket >= 0) {
env.insert(QStringLiteral("WAYLAND_SOCKET"), QString::number(socket));
}
}
QStringList args;
if (establishLock == EstablishLock::Immediate) {
args << QStringLiteral("--immediateLock");
......@@ -647,32 +629,46 @@ void KSldApp::startLockProcess(EstablishLock establishLock)
close(fd);
}
void KSldApp::userActivity()
{
if (isGraceTime()) {
unlock();
}
if (m_lockWindow) {
m_lockWindow->userActivity();
}
}
void KSldApp::showLockWindow()
{
if (!m_lockWindow) {
if (m_isX11) {
m_lockWindow = new X11Locker(this);
connect(m_lockWindow, &AbstractLocker::userActivity, m_lockWindow,
[this]() {
if (isGraceTime()) {
unlock();
}
},
Qt::QueuedConnection
);
}
if (m_isWayland) {
m_lockWindow = new WaylandLocker(m_waylandDisplay, this);
m_lockWindow = new WaylandLocker(this);
}
if (!m_lockWindow) {
return;
}
m_lockWindow->setGlobalAccel(m_globalAccel);
connect(m_lockWindow, &AbstractLocker::userActivity, m_lockWindow,
[this]() {
if (isGraceTime()) {
unlock();
}
},
Qt::QueuedConnection
);
connect(m_lockWindow, &AbstractLocker::lockWindowShown, m_lockWindow,
[this] {
lockScreenShown();
}
, Qt::QueuedConnection);
connect(m_waylandServer, &WaylandServer::x11WindowAdded, m_lockWindow, &AbstractLocker::addAllowedWindow);
}
m_lockWindow->showLockWindow();
......@@ -738,13 +734,6 @@ void KSldApp::solidSuspend()
}
}
void KSldApp::setWaylandDisplay(KWayland::Server::Display *display)
{
if (m_waylandDisplay != display) {
m_waylandDisplay = display;
}
}
void KSldApp::lockScreenShown()
{
if (m_lockState == Locked) {
......
......@@ -35,14 +35,6 @@ class QTimer;
class KSldTest;
class PowerManagementInhibition;
namespace KWayland
{
namespace Server {
class Display;
class ClientConnection;
}
}
namespace ScreenLocker
{
......@@ -84,13 +76,11 @@ public:
void configure();
void userActivity();
bool isGraceTime() const;
void setWaylandDisplay(KWayland::Server::Display *display);
void setWaylandFd(int fd);
KWayland::Server::ClientConnection *greeterClientConnection() const {
return m_greeterClientConnection;
}
void setGreeterEnvironment(const QProcessEnvironment &env);
/**
......@@ -139,7 +129,6 @@ Q_SIGNALS:
void aboutToLock();
void locked();
void unlocked();
void greeterClientConnectionChanged();
void lockStateChanged();
private Q_SLOTS:
......@@ -163,8 +152,7 @@ private:
QProcess *m_lockProcess;
AbstractLocker *m_lockWindow;
WaylandServer *m_waylandServer;
KWayland::Server::Display *m_waylandDisplay;
KWayland::Server::ClientConnection *m_greeterClientConnection;
/**
* Timer to measure how long the screen is locked.
* This information is required by DBus Interface.
......@@ -196,6 +184,8 @@ private:
QProcessEnvironment m_greeterEnv;
PowerManagementInhibition *m_powerManagementInhibition;
int m_waylandFd = -1;
// for auto tests
friend KSldTest;
};
......
......@@ -23,19 +23,12 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
#include <QGuiApplication>
#include <QScreen>
#include <KWayland/Server/display.h>
#include <KWayland/Server/seat_interface.h>
namespace ScreenLocker
{
WaylandLocker::WaylandLocker(KWayland::Server::Display *display, QObject *parent)
WaylandLocker::WaylandLocker(QObject *parent)
: AbstractLocker(parent)
{
const auto seats = display->seats();
for (auto s : seats) {
connect(s, &KWayland::Server::SeatInterface::timestampChanged, this, &WaylandLocker::userActivity);
}
if (m_background) {
updateGeometryOfBackground();
const auto screens = qApp->screens();
......
......@@ -39,7 +39,7 @@ class WaylandLocker : public AbstractLocker
Q_OBJECT
public:
WaylandLocker(KWayland::Server::Display *display, QObject *parent);
WaylandLocker(QObject *parent);
~WaylandLocker() override;
void showLockWindow() override;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment