Attachment 132375 Details for Bug 101595 – Patch for KWayland

[patch] Patch for KWayland

0001-server-Pass-a-dedicated-fd-to-each-keyboard-for-the-.patch (text/plain), 12.94 KB, created by Martin Flöser on 2017-06-30 18:33:22 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Flöser

Created: 2017-06-30 18:33:22 UTC

Size: 12.94 KB

patch

obsolete

>From b740083d9a2edd725a69e274e8d32874e7425405 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Martin=20Fl=C3=B6ser?= <mgraesslin@kde.org>
>Date: Fri, 30 Jun 2017 20:10:49 +0200
>Subject: [PATCH] [server] Pass a dedicated fd to each keyboard for the xkb
> keymap
>
>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
>---
> autotests/client/test_wayland_seat.cpp | 96 +++++++++++++++++++++++++++++++++-
> src/server/keyboard_interface.cpp      | 26 +++++++++
> src/server/keyboard_interface.h        |  1 +
> src/server/keyboard_interface_p.h      |  3 ++
> src/server/seat_interface.cpp          | 32 +++++++++---
> src/server/seat_interface.h            | 27 ++++++++--
> src/server/seat_interface_p.h          |  3 +-
> 7 files changed, 173 insertions(+), 15 deletions(-)
>
>diff --git a/autotests/client/test_wayland_seat.cpp b/autotests/client/test_wayland_seat.cpp
>index 26e2152..0099ce9 100644
>--- a/autotests/client/test_wayland_seat.cpp
>+++ b/autotests/client/test_wayland_seat.cpp
>@@ -90,7 +90,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;
>@@ -1595,7 +1596,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);
>@@ -2285,5 +2285,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()
>+{
>+#ifndef KWAYLANDSERVER_NO_DEPRECATED
>+    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"
>diff --git a/src/server/keyboard_interface.cpp b/src/server/keyboard_interface.cpp
>index a5ffd8a..464c533 100644
>--- a/src/server/keyboard_interface.cpp
>+++ b/src/server/keyboard_interface.cpp
>@@ -23,10 +23,13 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
> #include "seat_interface.h"
> #include "surface_interface.h"
> // Qt
>+#include <QTemporaryFile>
> #include <QVector>
> // Wayland
> #include <wayland-server.h>
> 
>+#include <unistd.h>
>+
> namespace KWayland
> {
> 
>@@ -90,6 +93,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) {
>diff --git a/src/server/keyboard_interface.h b/src/server/keyboard_interface.h
>index 17d1d20..099b2cd 100644
>--- a/src/server/keyboard_interface.h
>+++ b/src/server/keyboard_interface.h
>@@ -50,6 +50,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);
>diff --git a/src/server/keyboard_interface_p.h b/src/server/keyboard_interface_p.h
>index c7fba23..3bdeda1 100644
>--- a/src/server/keyboard_interface_p.h
>+++ b/src/server/keyboard_interface_p.h
>@@ -24,6 +24,8 @@ License along with this library.  If not, see <http://www.gnu.org/licenses/>.
> 
> #include <QPointer>
> 
>+class QTemporaryFile;
>+
> namespace KWayland
> {
> namespace Server
>@@ -45,6 +47,7 @@ public:
>     SurfaceInterface *focusedSurface = nullptr;
>     QPointer<SurfaceInterface> focusedChildSurface;
>     QMetaObject::Connection destroyConnection;
>+    QScopedPointer<QTemporaryFile> keymap;
> 
> private:
>     static const struct wl_keyboard_interface s_interface;
>diff --git a/src/server/seat_interface.cpp b/src/server/seat_interface.cpp
>index 3b17982..94dfa3a 100644
>--- a/src/server/seat_interface.cpp
>+++ b/src/server/seat_interface.cpp
>@@ -491,7 +491,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) {
>@@ -1087,14 +1087,28 @@ void SeatInterface::setFocusedKeyboardSurface(SurfaceInterface *surface)
>     }
> }
> 
>+#ifndef KWAYLANDSERVER_NO_DEPRECATED
> 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);
>     }
> }
> 
>@@ -1151,17 +1165,19 @@ bool SeatInterface::isKeymapXkbCompatible() const
>     return d->keys.keymap.xkbcommonCompatible;
> }
> 
>+#ifndef KWAYLANDSERVER_NO_DEPRECATED
> int SeatInterface::keymapFileDescriptor() const
> {
>-    Q_D();
>-    return d->keys.keymap.fd;
>+    return -1;
> }
>+#endif
> 
>+#ifndef KWAYLANDSERVER_NO_DEPRECATED
> quint32 SeatInterface::keymapSize() const
> {
>-    Q_D();
>-    return d->keys.keymap.size;
>+    return 0;
> }
>+#endif
> 
> quint32 SeatInterface::depressedModifiers() const
> {
>diff --git a/src/server/seat_interface.h b/src/server/seat_interface.h
>index bf11680..9dd4f16 100644
>--- a/src/server/seat_interface.h
>+++ b/src/server/seat_interface.h
>@@ -518,7 +518,18 @@ public:
>      * @name keyboard related methods
>      **/
>     ///@{
>-    void setKeymap(int fd, quint32 size);
>+    /**
>+     * @deprecated Since 5.KEYMAPFIX use setKeymapData
>+     **/
>+#ifndef KWAYLANDSERVER_NO_DEPRECATED
>+    void KWAYLANDSERVER_DEPRECATED 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.KEYMAPFIX
>+     **/
>+    void setKeymapData(const QByteArray &content);
>     void keyPressed(quint32 key);
>     void keyReleased(quint32 key);
>     void updateKeyboardModifiers(quint32 depressed, quint32 latched, quint32 locked, quint32 group);
>@@ -540,8 +551,18 @@ public:
>     quint32 lockedModifiers() const;
>     quint32 groupModifiers() const;
>     quint32 lastModifiersSerial() const;
>-    int keymapFileDescriptor() const;
>-    quint32 keymapSize() const;
>+    /**
>+     * @deprecated since 5.KEYMAPFIX
>+     **/
>+#ifndef KWAYLANDSERVER_NO_DEPRECATED
>+    int KWAYLANDSERVER_DEPRECATED keymapFileDescriptor() const;
>+#endif
>+    /**
>+     * @deprecated since 5.KEYMAPFIX
>+     **/
>+#ifndef KWAYLANDSERVER_NO_DEPRECATED
>+    quint32 KWAYLANDSERVER_DEPRECATED keymapSize() const;
>+#endif
>     bool isKeymapXkbCompatible() const;
>     QVector<quint32> pressedKeys() const;
>     /**
>diff --git a/src/server/seat_interface_p.h b/src/server/seat_interface_p.h
>index f4c4496..d26fffc 100644
>--- a/src/server/seat_interface_p.h
>+++ b/src/server/seat_interface_p.h
>@@ -99,9 +99,8 @@ public:
>         };
>         QHash<quint32, State> states;
>         struct Keymap {
>-            int fd = -1;
>-            quint32 size = 0;
>             bool xkbcommonCompatible = false;
>+            QByteArray content;
>         };
>         Keymap keymap;
>         struct Modifiers {
>-- 
>2.7.4
>

Actions: View | Splinter Review

Attachments on bug 101595: 132248 | 132254 | 132291 | 132307 | 132308 | 132327 | 132375

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.