Commit 6bfa71d8 authored by Vlad Zahorodnii's avatar Vlad Zahorodnii
Browse files

Pass a dedicated fd to each keyboard for the xkb keymap

Summary:
To better isolate the clients from each other eachh KeyboardInterface
creates it's own dedicated temporary file and sends the fd for this
temporary file to the client. This means the memory for the keymap is no
longer shared between all clients, every client has an own copy.

To support this the existing API to set the keymap is deprecated and
replaced by a new method setKeymapData which takes the content of the
keymap as a byte array. The now deprecated method which takes a file
descriptor is changed to use the new setKeymapData method. For that it
reads the content of the file.

The implementation in KeyboardInterface to create the file descriptor is
based on the implementation of KWin. As I implemented the change in KWin
(see 3b4c508ee36ac74c37e77fcaa14d106397ad2994) it is not a problem from
GPL vs LGPL perspective.

The change includes test cases to verify that the content of the keymap
is properly passed to the client and that the memory is no longer shared.

BUG: 381674

Reviewers: #kwin, #frameworks, davidedmundson, zzag

Reviewed By: #kwin, zzag

Subscribers: plasma-devel, kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D14910
parent deb476e4
......@@ -77,7 +77,8 @@ private Q_SLOTS:
void testTouch();
void testDisconnect();
void testPointerEnterOnUnboundSurface();
// TODO: add test for keymap
void testKeymap();
void testKeymapThroughFd();
private:
KWayland::Server::Display *m_display;
......@@ -1699,7 +1700,6 @@ void TestWaylandSeat::testKeyboard()
m_seatInterface->setKeyRepeatInfo(30, 560);
m_seatInterface->setKeyRepeatInfo(25, 660);
m_seatInterface->updateKeyboardModifiers(5, 6, 7, 8);
m_seatInterface->setKeymap(open("/dev/null", O_RDONLY), 0);
m_seatInterface->setFocusedKeyboardSurface(nullptr);
m_seatInterface->setFocusedKeyboardSurface(serverSurface);
QCOMPARE(m_seatInterface->focusedKeyboardSurface(), serverSurface);
......@@ -2392,5 +2392,97 @@ void TestWaylandSeat::testPointerEnterOnUnboundSurface()
QVERIFY(!clientErrorSpy.wait(100));
}
void TestWaylandSeat::testKeymap()
{
using namespace KWayland::Client;
using namespace KWayland::Server;
m_seatInterface->setHasKeyboard(true);
QSignalSpy keyboardChangedSpy(m_seat, &Seat::hasKeyboardChanged);
QVERIFY(keyboardChangedSpy.isValid());
QVERIFY(keyboardChangedSpy.wait());
QScopedPointer<Keyboard> keyboard(m_seat->createKeyboard());
QSignalSpy keymapChangedSpy(keyboard.data(), &Keyboard::keymapChanged);
QVERIFY(keymapChangedSpy.isValid());
m_seatInterface->setKeymapData(QByteArrayLiteral("foo"));
QVERIFY(keymapChangedSpy.wait());
int fd = keymapChangedSpy.first().first().toInt();
QVERIFY(fd != -1);
QCOMPARE(keymapChangedSpy.first().last().value<quint32>(), 3u);
QFile file;
QVERIFY(file.open(fd, QIODevice::ReadOnly));
const char *address = reinterpret_cast<char*>(file.map(0, keymapChangedSpy.first().last().value<quint32>()));
QVERIFY(address);
QCOMPARE(qstrcmp(address, "foo"), 0);
file.close();
// change the keymap
keymapChangedSpy.clear();
m_seatInterface->setKeymapData(QByteArrayLiteral("bar"));
QVERIFY(keymapChangedSpy.wait());
fd = keymapChangedSpy.first().first().toInt();
QVERIFY(fd != -1);
QCOMPARE(keymapChangedSpy.first().last().value<quint32>(), 3u);
QVERIFY(file.open(fd, QIODevice::ReadWrite));address = reinterpret_cast<char*>(file.map(0, keymapChangedSpy.first().last().value<quint32>()));
QVERIFY(address);
QCOMPARE(qstrcmp(address, "bar"), 0);
}
void TestWaylandSeat::testKeymapThroughFd()
{
#if KWAYLANDSERVER_BUILD_DEPRECATED_SINCE(5, 69)
using namespace KWayland::Client;
using namespace KWayland::Server;
m_seatInterface->setHasKeyboard(true);
QSignalSpy keyboardChangedSpy(m_seat, &Seat::hasKeyboardChanged);
QVERIFY(keyboardChangedSpy.isValid());
QVERIFY(keyboardChangedSpy.wait());
QScopedPointer<Keyboard> keyboard(m_seat->createKeyboard());
QSignalSpy keymapChangedSpy(keyboard.data(), &Keyboard::keymapChanged);
QVERIFY(keymapChangedSpy.isValid());
QTemporaryFile serverFile;
QVERIFY(serverFile.open());
QByteArray data = QByteArrayLiteral("foobar");
QVERIFY(serverFile.resize(data.size() + 1));
uchar *serverAddress = serverFile.map(0, data.size() + 1);
QVERIFY(serverAddress);
QVERIFY(qstrncpy(reinterpret_cast<char *>(serverAddress), data.constData(), data.size() + 1));
m_seatInterface->setKeymap(serverFile.handle(), data.size());
QVERIFY(keymapChangedSpy.wait());
int fd = keymapChangedSpy.first().first().toInt();
QVERIFY(fd != -1);
QCOMPARE(keymapChangedSpy.first().last().value<quint32>(), 6u);
QFile file;
QVERIFY(file.open(fd, QIODevice::ReadOnly));
const char *address = reinterpret_cast<char*>(file.map(0, keymapChangedSpy.first().last().value<quint32>()));
QVERIFY(address);
QCOMPARE(qstrcmp(address, "foobar"), 0);
QScopedPointer<Keyboard> keyboard2(m_seat->createKeyboard());
QSignalSpy keymapChangedSpy2(keyboard2.data(), &Keyboard::keymapChanged);
QVERIFY(keymapChangedSpy2.isValid());
QVERIFY(keymapChangedSpy2.wait());
int fd2 = keymapChangedSpy2.first().first().toInt();
QVERIFY(fd2 != -1);
QCOMPARE(keymapChangedSpy2.first().last().value<quint32>(), 6u);
QFile file2;
QVERIFY(file2.open(fd2, QIODevice::ReadWrite));
char *address2 = reinterpret_cast<char*>(file2.map(0, keymapChangedSpy2.first().last().value<quint32>()));
QVERIFY(address2);
QCOMPARE(qstrcmp(address2, "foobar"), 0);
address2[0] = 'g';
QCOMPARE(qstrcmp(address2, "goobar"), 0);
QCOMPARE(qstrcmp(address, "foobar"), 0);
#endif
}
QTEST_GUILESS_MAIN(TestWaylandSeat)
#include "test_wayland_seat.moc"
......@@ -299,7 +299,7 @@ ecm_generate_export_header(KF5WaylandServer
GROUP_BASE_NAME KF
VERSION ${KF5_VERSION}
DEPRECATED_BASE_VERSION 0
DEPRECATION_VERSIONS 5.5 5.28 5.50 5.52
DEPRECATION_VERSIONS 5.5 5.28 5.50 5.52 5.69
)
# TODO: add support for EXCLUDE_DEPRECATED_BEFORE_AND_AT to all KWayland libs
# needs fixing of undeprecated API being still implemented using own deprecated API
......
......@@ -9,10 +9,13 @@
#include "seat_interface.h"
#include "surface_interface.h"
// Qt
#include <QTemporaryFile>
#include <QVector>
// Wayland
#include <wayland-server.h>
#include <unistd.h>
namespace KWayland
{
......@@ -76,6 +79,29 @@ void KeyboardInterface::setKeymap(int fd, quint32 size)
d->sendKeymap(fd, size);
}
void KeyboardInterface::setKeymap(const QByteArray &content)
{
QScopedPointer<QTemporaryFile> tmp{new QTemporaryFile(this)};
if (!tmp->open()) {
return;
}
unlink(tmp->fileName().toUtf8().constData());
if (!tmp->resize(content.size())) {
return;
}
uchar *address = tmp->map(0, content.size());
if (!address) {
return;
}
if (qstrncpy(reinterpret_cast<char*>(address), content.constData(), content.size() + 1) == nullptr) {
return;
}
tmp->unmap(address);
Q_D();
d->sendKeymap(tmp->handle(), content.size());
d->keymap.swap(tmp);
}
void KeyboardInterface::Private::sendKeymap(int fd, quint32 size)
{
if (!resource) {
......
......@@ -36,6 +36,7 @@ public:
private:
void setFocusedSurface(SurfaceInterface *surface, quint32 serial);
void setKeymap(int fd, quint32 size);
void setKeymap(const QByteArray &content);
void updateModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group, quint32 serial);
void keyPressed(quint32 key, quint32 serial);
void keyReleased(quint32 key, quint32 serial);
......
......@@ -10,6 +10,8 @@
#include <QPointer>
class QTemporaryFile;
namespace KWayland
{
namespace Server
......@@ -31,6 +33,7 @@ public:
SurfaceInterface *focusedSurface = nullptr;
QPointer<SurfaceInterface> focusedChildSurface;
QMetaObject::Connection destroyConnection;
QScopedPointer<QTemporaryFile> keymap;
private:
static const struct wl_keyboard_interface s_interface;
......
......@@ -14,6 +14,8 @@
#include "pointer_interface_p.h"
#include "surface_interface.h"
#include "textinput_interface_p.h"
// Qt
#include <QFile>
// Wayland
#ifndef WL_SEAT_NAME_SINCE_VERSION
#define WL_SEAT_NAME_SINCE_VERSION 2
......@@ -522,7 +524,7 @@ void SeatInterface::Private::getKeyboard(wl_client *client, wl_resource *resourc
}
keyboard->repeatInfo(keys.keyRepeat.charactersPerSecond, keys.keyRepeat.delay);
if (keys.keymap.xkbcommonCompatible) {
keyboard->setKeymap(keys.keymap.fd, keys.keymap.size);
keyboard->setKeymap(keys.keymap.content);
}
keyboards << keyboard;
if (keys.focus.surface && keys.focus.surface->client() == clientConnection) {
......@@ -1147,14 +1149,28 @@ void SeatInterface::setFocusedKeyboardSurface(SurfaceInterface *surface)
}
}
#if KWAYLANDSERVER_BUILD_DEPRECATED_SINCE(5, 69)
void SeatInterface::setKeymap(int fd, quint32 size)
{
QFile file;
if (!file.open(fd, QIODevice::ReadOnly)) {
return;
}
const char *address = reinterpret_cast<char*>(file.map(0, size));
if (!address) {
return;
}
setKeymapData(QByteArray(address, size));
}
#endif
void SeatInterface::setKeymapData(const QByteArray &content)
{
Q_D();
d->keys.keymap.xkbcommonCompatible = true;
d->keys.keymap.fd = fd;
d->keys.keymap.size = size;
d->keys.keymap.content = content;
for (auto it = d->keyboards.constBegin(); it != d->keyboards.constEnd(); ++it) {
(*it)->setKeymap(fd, size);
(*it)->setKeymap(content);
}
}
......@@ -1211,17 +1227,19 @@ bool SeatInterface::isKeymapXkbCompatible() const
return d->keys.keymap.xkbcommonCompatible;
}
#if KWAYLANDSERVER_BUILD_DEPRECATED_SINCE(5, 69)
int SeatInterface::keymapFileDescriptor() const
{
Q_D();
return d->keys.keymap.fd;
return -1;
}
#endif
#if KWAYLANDSERVER_BUILD_DEPRECATED_SINCE(5, 69)
quint32 SeatInterface::keymapSize() const
{
Q_D();
return d->keys.keymap.size;
return 0;
}
#endif
quint32 SeatInterface::depressedModifiers() const
{
......
......@@ -546,7 +546,19 @@ public:
* @name keyboard related methods
**/
///@{
#if KWAYLANDSERVER_ENABLE_DEPRECATED_SINCE(5, 69)
/**
* @deprecated since 5.69, use setKeymapData
**/
KWAYLANDSERVER_DEPRECATED_VERSION(5, 69, "Use SeatInterface::setKeymapData()")
void setKeymap(int fd, quint32 size);
#endif
/**
* Sets the xkb keymap with @p content for this Seat.
* The content gets sent to all registered KeyboardInterfaces
* @since 5.69
**/
void setKeymapData(const QByteArray &content);
void keyPressed(quint32 key);
void keyReleased(quint32 key);
void updateKeyboardModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group);
......@@ -568,8 +580,16 @@ public:
quint32 lockedModifiers() const;
quint32 groupModifiers() const;
quint32 lastModifiersSerial() const;
int keymapFileDescriptor() const;
quint32 keymapSize() const;
#if KWAYLANDSERVER_ENABLE_DEPRECATED_SINCE(5, 69)
/**
* @deprecated since 5.69
**/
int KWAYLANDSERVER_DEPRECATED keymapFileDescriptor() const;
/**
* @deprecated since 5.69
**/
quint32 KWAYLANDSERVER_DEPRECATED keymapSize() const;
#endif
bool isKeymapXkbCompatible() const;
QVector<quint32> pressedKeys() const;
/**
......
......@@ -86,9 +86,8 @@ public:
};
QHash<quint32, State> states;
struct Keymap {
int fd = -1;
quint32 size = 0;
bool xkbcommonCompatible = false;
QByteArray content;
};
Keymap keymap;
struct Modifiers {
......
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