From e0716c2306cc805c8f166b06b2607a8a6eb0b09c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Gr=C3=A4=C3=9Flin?= Date: Tue, 5 Jul 2016 10:56:22 +0200 Subject: [PATCH] [server] Properly send a selection clear prior to keyboard focus enter Summary: When setting the keyboard focus the server needs also to send the current selection to the client. So far KWayland only sent the selection if it was set. That is if the last focused client cleared the selection it was not updated and the client might have had an outdated selection. To prevent this situation the server now explicitly sends the clear to the client on enter if there is no selection. Also if the selection is cleared, the SeatInterface now unsets it's current selection to make sure that the next focused keyboard will get the clear selection sent. Test Plan: Existing test case adjusted and a new test case added which simulates the interaction of two clients. Reviewers: #plasma_on_wayland Subscribers: plasma-devel Tags: #plasma_on_wayland Differential Revision: https://phabricator.kde.org/D2091 --- src/wayland/autotests/client/CMakeLists.txt | 11 + .../autotests/client/test_selection.cpp | 274 ++++++++++++++++++ .../autotests/client/test_wayland_seat.cpp | 8 +- src/wayland/seat_interface.cpp | 3 + 4 files changed, 291 insertions(+), 5 deletions(-) create mode 100644 src/wayland/autotests/client/test_selection.cpp diff --git a/src/wayland/autotests/client/CMakeLists.txt b/src/wayland/autotests/client/CMakeLists.txt index 08d852c3dd..cb2d29d827 100644 --- a/src/wayland/autotests/client/CMakeLists.txt +++ b/src/wayland/autotests/client/CMakeLists.txt @@ -319,3 +319,14 @@ add_executable(testError ${testError_SRCS}) target_link_libraries( testError Qt5::Test Qt5::Gui KF5::WaylandClient KF5::WaylandServer Wayland::Client) add_test(kwayland-testError testError) ecm_mark_as_test(testError) + +######################################################## +# Test Selection +######################################################## +set( testSelection_SRCS + test_selection.cpp + ) +add_executable(testSelection ${testSelection_SRCS}) +target_link_libraries( testSelection Qt5::Test Qt5::Gui KF5::WaylandClient KF5::WaylandServer Wayland::Client) +add_test(kwayland-testSelection testSelection) +ecm_mark_as_test(testSelection) diff --git a/src/wayland/autotests/client/test_selection.cpp b/src/wayland/autotests/client/test_selection.cpp new file mode 100644 index 0000000000..18dd85bb12 --- /dev/null +++ b/src/wayland/autotests/client/test_selection.cpp @@ -0,0 +1,274 @@ +/******************************************************************** +Copyright 2016 Martin Gräßlin + +This library is free software; you can redistribute it and/or +modify it under the terms of the GNU Lesser General Public +License as published by the Free Software Foundation; either +version 2.1 of the License, or (at your option) version 3, or any +later version accepted by the membership of KDE e.V. (or its +successor approved by the membership of KDE e.V.), which shall +act as a proxy defined in Section 6 of version 3 of the license. + +This library is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +Lesser General Public License for more details. + +You should have received a copy of the GNU Lesser General Public +License along with this library. If not, see . +*********************************************************************/ +// Qt +#include +// client +#include "../../src/client/connection_thread.h" +#include "../../src/client/compositor.h" +#include "../../src/client/datadevice.h" +#include "../../src/client/datadevicemanager.h" +#include "../../src/client/datasource.h" +#include "../../src/client/event_queue.h" +#include "../../src/client/keyboard.h" +#include "../../src/client/registry.h" +#include "../../src/client/seat.h" +#include "../../src/client/surface.h" +// server +#include "../../src/server/compositor_interface.h" +#include "../../src/server/datadevicemanager_interface.h" +#include "../../src/server/display.h" +#include "../../src/server/seat_interface.h" + +using namespace KWayland::Client; +using namespace KWayland::Server; + + +class SelectionTest : public QObject +{ + Q_OBJECT +private Q_SLOTS: + void init(); + void cleanup(); + void testClearOnEnter(); + +private: + Display *m_display = nullptr; + CompositorInterface *m_compositorInterface = nullptr; + SeatInterface *m_seatInterface = nullptr; + DataDeviceManagerInterface *m_ddmInterface = nullptr; + + struct Connection { + ConnectionThread *connection = nullptr; + QThread *thread = nullptr; + EventQueue *queue = nullptr; + Compositor *compositor = nullptr; + Seat *seat = nullptr; + DataDeviceManager *ddm = nullptr; + Keyboard *keyboard = nullptr; + DataDevice *dataDevice = nullptr; + }; + bool setupConnection(Connection *c); + void cleanupConnection(Connection *c); + + Connection m_client1; + Connection m_client2; +}; + +static const QString s_socketName = QStringLiteral("kwayland-test-selection-0"); + +void SelectionTest::init() +{ + delete m_display; + m_display = new Display(this); + m_display->setSocketName(s_socketName); + m_display->start(); + QVERIFY(m_display->isRunning()); + m_display->createShm(); + m_compositorInterface = m_display->createCompositor(m_display); + m_compositorInterface->create(); + m_seatInterface = m_display->createSeat(m_display); + m_seatInterface->setHasKeyboard(true); + m_seatInterface->create(); + m_ddmInterface = m_display->createDataDeviceManager(m_display); + m_ddmInterface->create(); + + // setup connection + setupConnection(&m_client1); + setupConnection(&m_client2); +} + +bool SelectionTest::setupConnection(Connection* c) +{ + c->connection = new ConnectionThread; + QSignalSpy connectedSpy(c->connection, &ConnectionThread::connected); + if (!connectedSpy.isValid()) { + return false; + } + c->connection->setSocketName(s_socketName); + + c->thread = new QThread(this); + c->connection->moveToThread(c->thread); + c->thread->start(); + + c->connection->initConnection(); + if (!connectedSpy.wait()) { + return false; + } + + c->queue = new EventQueue(this); + c->queue->setup(c->connection); + + Registry registry; + QSignalSpy interfacesAnnouncedSpy(®istry, &Registry::interfacesAnnounced); + if (!interfacesAnnouncedSpy.isValid()) { + return false; + } + registry.setEventQueue(c->queue); + registry.create(c->connection); + if (!registry.isValid()) { + return false; + } + registry.setup(); + if (!interfacesAnnouncedSpy.wait()) { + return false; + } + + c->compositor = registry.createCompositor(registry.interface(Registry::Interface::Compositor).name, + registry.interface(Registry::Interface::Compositor).version, + this); + if (!c->compositor->isValid()) { + return false; + } + c->ddm = registry.createDataDeviceManager(registry.interface(Registry::Interface::DataDeviceManager).name, + registry.interface(Registry::Interface::DataDeviceManager).version, + this); + if (!c->ddm->isValid()) { + return false; + } + c->seat = registry.createSeat(registry.interface(Registry::Interface::Seat).name, + registry.interface(Registry::Interface::Seat).version, + this); + if (!c->seat->isValid()) { + return false; + } + QSignalSpy keyboardSpy(c->seat, &Seat::hasKeyboardChanged); + if (!keyboardSpy.isValid()) { + return false; + } + if (!keyboardSpy.wait()) { + return false; + } + if (!c->seat->hasKeyboard()) { + return false; + } + c->keyboard = c->seat->createKeyboard(c->seat); + if (!c->keyboard->isValid()) { + return false; + } + c->dataDevice = c->ddm->getDataDevice(c->seat, this); + if (!c->dataDevice->isValid()) { + return false; + } + + return true; +} + +void SelectionTest::cleanup() +{ + cleanupConnection(&m_client1); + cleanupConnection(&m_client2); +#define CLEANUP(variable) \ + delete variable; \ + variable = nullptr; + + CLEANUP(m_ddmInterface) + CLEANUP(m_seatInterface) + CLEANUP(m_compositorInterface) + CLEANUP(m_display) +#undef CLEANUP +} + +void SelectionTest::cleanupConnection(Connection *c) +{ + delete c->dataDevice; + c->dataDevice = nullptr; + delete c->keyboard; + c->keyboard = nullptr; + delete c->ddm; + c->ddm = nullptr; + delete c->seat; + c->seat = nullptr; + delete c->compositor; + c->compositor = nullptr; + delete c->queue; + c->queue = nullptr; + if (c->connection) { + c->connection->deleteLater(); + c->connection = nullptr; + } + if (c->thread) { + c->thread->quit(); + c->thread->wait(); + delete c->thread; + c->thread = nullptr; + } +} + +void SelectionTest::testClearOnEnter() +{ + // this test verifies that the selection is cleared prior to keyboard enter if there is no current selection + QSignalSpy selectionClearedClient1Spy(m_client1.dataDevice, &DataDevice::selectionCleared); + QVERIFY(selectionClearedClient1Spy.isValid()); + QSignalSpy keyboardEnteredClient1Spy(m_client1.keyboard, &Keyboard::entered); + QVERIFY(keyboardEnteredClient1Spy.isValid()); + + // now create a Surface + QSignalSpy surfaceCreatedSpy(m_compositorInterface, &CompositorInterface::surfaceCreated); + QVERIFY(surfaceCreatedSpy.isValid()); + QScopedPointer s1(m_client1.compositor->createSurface()); + QVERIFY(surfaceCreatedSpy.wait()); + auto serverSurface1 = surfaceCreatedSpy.first().first().value(); + QVERIFY(serverSurface1); + + // pass this surface keyboard focus + m_seatInterface->setFocusedKeyboardSurface(serverSurface1); + // should get a clear + QVERIFY(selectionClearedClient1Spy.wait()); + + // let's set a selection + QScopedPointer dataSource(m_client1.ddm->createDataSource()); + dataSource->offer(QStringLiteral("text/plain")); + m_client1.dataDevice->setSelection(keyboardEnteredClient1Spy.first().first().value(), dataSource.data()); + + // now let's bring in client 2 + QSignalSpy selectionOfferedClient2Spy(m_client2.dataDevice, &DataDevice::selectionOffered); + QVERIFY(selectionOfferedClient2Spy.isValid()); + QSignalSpy selectionClearedClient2Spy(m_client2.dataDevice, &DataDevice::selectionCleared); + QVERIFY(selectionClearedClient2Spy.isValid()); + QSignalSpy keyboardEnteredClient2Spy(m_client2.keyboard, &Keyboard::entered); + QVERIFY(keyboardEnteredClient2Spy.isValid()); + QScopedPointer s2(m_client2.compositor->createSurface()); + QVERIFY(surfaceCreatedSpy.wait()); + auto serverSurface2 = surfaceCreatedSpy.last().first().value(); + QVERIFY(serverSurface2); + + // entering that surface should give a selection + m_seatInterface->setFocusedKeyboardSurface(serverSurface2); + QVERIFY(selectionOfferedClient2Spy.wait()); + QVERIFY(selectionClearedClient2Spy.isEmpty()); + // now unset the selection + m_client2.dataDevice->clearSelection(keyboardEnteredClient2Spy.first().first().value()); + QVERIFY(selectionClearedClient2Spy.wait()); + // set a data source but without offers + QScopedPointer dataSource2(m_client2.ddm->createDataSource()); + m_client2.dataDevice->setSelection(keyboardEnteredClient2Spy.first().first().value(), dataSource2.data()); + QVERIFY(selectionOfferedClient2Spy.wait()); + // and clear again + m_client2.dataDevice->clearSelection(keyboardEnteredClient2Spy.first().first().value()); + QVERIFY(selectionClearedClient2Spy.wait()); + + // now pass focus to first surface + m_seatInterface->setFocusedKeyboardSurface(serverSurface1); + // we should get a clear + QVERIFY(selectionClearedClient1Spy.wait()); +} + +QTEST_GUILESS_MAIN(SelectionTest) +#include "test_selection.moc" diff --git a/src/wayland/autotests/client/test_wayland_seat.cpp b/src/wayland/autotests/client/test_wayland_seat.cpp index cdae94456d..8c9cff3fad 100644 --- a/src/wayland/autotests/client/test_wayland_seat.cpp +++ b/src/wayland/autotests/client/test_wayland_seat.cpp @@ -1409,11 +1409,10 @@ void TestWaylandSeat::testSelection() m_seatInterface->setFocusedKeyboardSurface(serverSurface); QCOMPARE(m_seatInterface->focusedKeyboardSurface(), serverSurface); QVERIFY(!m_seatInterface->focusedKeyboard()); - serverSurface->client()->flush(); - QCoreApplication::processEvents(); - QCoreApplication::processEvents(); + QVERIFY(selectionClearedSpy.wait()); QVERIFY(selectionSpy.isEmpty()); - QVERIFY(selectionClearedSpy.isEmpty()); + QVERIFY(!selectionClearedSpy.isEmpty()); + selectionClearedSpy.clear(); QVERIFY(!m_seatInterface->selection()); // now let's try to set a selection - we have keyboard focus, so it should be sent to us @@ -1425,7 +1424,6 @@ void TestWaylandSeat::testSelection() QCOMPARE(selectionSpy.count(), 1); auto ddi = m_seatInterface->selection(); QVERIFY(ddi); - QVERIFY(selectionClearedSpy.isEmpty()); auto df = selectionSpy.first().first().value(); QCOMPARE(df->offeredMimeTypes().count(), 1); QCOMPARE(df->offeredMimeTypes().first().name(), QStringLiteral("text/plain")); diff --git a/src/wayland/seat_interface.cpp b/src/wayland/seat_interface.cpp index ca0c53ea53..4eefde4145 100644 --- a/src/wayland/seat_interface.cpp +++ b/src/wayland/seat_interface.cpp @@ -328,6 +328,7 @@ void SeatInterface::Private::updateSelection(DataDeviceInterface *dataDevice, bo keys.focus.selection->sendSelection(dataDevice); } else { keys.focus.selection->sendClearSelection(); + currentSelection = nullptr; } } } @@ -859,6 +860,8 @@ void SeatInterface::setFocusedKeyboardSurface(SurfaceInterface *surface) if (d->keys.focus.selection) { if (d->currentSelection) { d->keys.focus.selection->sendSelection(d->currentSelection); + } else { + d->keys.focus.selection->sendClearSelection(); } } }