Manage active selection as active DataSource than DataDevice

Summary:
A DataDevice will have zero or one active DataSource as the seclection.

In the existing code we track the current data device then update it to
the newest data device
when the source inside a data device changes.

If we store the active data source inside Seat instead of the device
everything becomes
somewhat simpler and safer.

An entire unit test vanishes as that case of an externally set
DataDevice with no source
can no longer happen.

There's also a lot of duplication that's been merged in this patch so we
have one path.

There are some technical behavioural changes in particular we do cleanup
when the
source vanishes rather than the data device, but if anything that seems
safer and more correct.

It's a precursor for introducing an abstraction class round the source
without needing to meddle
with too much code.

Test Plan: Relevant unit tests passed, ran with it for a while with no
issue.

Reviewers: #kwin

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

src/wayland/autotests/client/test_datadevice.cpp

@@ -184,9 +184,11 @@ void TestDataDevice::testCreate()
     QVERIFY(!deviceInterface->selection());
     QVERIFY(deviceInterface->parentResource());
 
+
+    // this will probably fail, we need to make a selection client side
     QVERIFY(!m_seatInterface->selection());
-    m_seatInterface->setSelection(deviceInterface);
-    QCOMPARE(m_seatInterface->selection(), deviceInterface);
+    m_seatInterface->setSelection(deviceInterface->selection());
+    QCOMPARE(m_seatInterface->selection(), deviceInterface->selection());
 
     // and destroy
     QSignalSpy destroyedSpy(deviceInterface, &QObject::destroyed);
@@ -404,7 +406,7 @@ void TestDataDevice::testSetSelection()
     // send selection to datadevice
     QSignalSpy selectionOfferedSpy(dataDevice.data(), SIGNAL(selectionOffered(KWayland::Client::DataOffer*)));
     QVERIFY(selectionOfferedSpy.isValid());
-    deviceInterface->sendSelection(deviceInterface);
+    deviceInterface->sendSelection(deviceInterface->selection());
     QVERIFY(selectionOfferedSpy.wait());
     QCOMPARE(selectionOfferedSpy.count(), 1);
     auto dataOffer = selectionOfferedSpy.first().first().value<DataOffer*>();
@@ -439,7 +441,7 @@ void TestDataDevice::testSetSelection()
     dataDevice.reset();
     QVERIFY(unboundSpy.wait());
     // send a selection to the unbound data device
-    deviceInterface->sendSelection(deviceInterface);
+    deviceInterface->sendSelection(deviceInterface->selection());
 }
 
 void TestDataDevice::testSendSelectionOnSeat()

src/wayland/autotests/client/test_wayland_seat.cpp

@@ -26,6 +26,7 @@
 #include "../../src/server/buffer_interface.h"
 #include "../../src/server/compositor_interface.h"
 #include "../../src/server/datadevicemanager_interface.h"
+#include "../../src/server/datasource_interface.h"
 #include "../../src/server/display.h"
 #include "../../src/server/keyboard_interface.h"
 #include "../../src/server/pointer_interface.h"
@@ -72,7 +73,6 @@ private Q_SLOTS:
     void testCast();
     void testDestroy();
     void testSelection();
-    void testSelectionNoDataSource();
     void testDataDeviceForKeyboardSurface();
     void testTouch();
     void testDisconnect();
@@ -1921,54 +1921,6 @@ void TestWaylandSeat::testSelection()
     QVERIFY(cancelledSpy.isValid());
     m_seatInterface->setSelection(ddi);
     QVERIFY(cancelledSpy.wait());
-
-    // Copy already cleared selection,  BUG 383054
-    ddi->sendSelection(ddi);
-}
-
-void TestWaylandSeat::testSelectionNoDataSource()
-{
-    // this test verifies that the server doesn't crash when using setSelection with
-    // a DataDevice which doesn't have a DataSource yet
-    using namespace KWayland::Client;
-    using namespace KWaylandServer;
-    // first create the DataDevice
-    QScopedPointer<DataDeviceManagerInterface> ddmi(m_display->createDataDeviceManager());
-    ddmi->create();
-    QSignalSpy ddiCreatedSpy(ddmi.data(), &DataDeviceManagerInterface::dataDeviceCreated);
-    QVERIFY(ddiCreatedSpy.isValid());
-    Registry registry;
-    QSignalSpy dataDeviceManagerSpy(&registry, &Registry::dataDeviceManagerAnnounced);
-    QVERIFY(dataDeviceManagerSpy.isValid());
-    registry.setEventQueue(m_queue);
-    registry.create(m_connection->display());
-    QVERIFY(registry.isValid());
-    registry.setup();
-
-    QVERIFY(dataDeviceManagerSpy.wait());
-    QScopedPointer<DataDeviceManager> ddm(registry.createDataDeviceManager(dataDeviceManagerSpy.first().first().value<quint32>(),
-                                                                           dataDeviceManagerSpy.first().last().value<quint32>()));
-    QVERIFY(ddm->isValid());
-
-    QScopedPointer<DataDevice> dd(ddm->getDataDevice(m_seat));
-    QVERIFY(dd->isValid());
-    QVERIFY(ddiCreatedSpy.wait());
-    auto ddi = ddiCreatedSpy.first().first().value<DataDeviceInterface*>();
-    QVERIFY(ddi);
-
-    // now create a surface and pass it keyboard focus
-    QSignalSpy surfaceCreatedSpy(m_compositorInterface, &CompositorInterface::surfaceCreated);
-    QVERIFY(surfaceCreatedSpy.isValid());
-    QScopedPointer<Surface> surface(m_compositor->createSurface());
-    QVERIFY(surface->isValid());
-    QVERIFY(surfaceCreatedSpy.wait());
-    auto serverSurface = surfaceCreatedSpy.first().first().value<SurfaceInterface*>();
-    QVERIFY(!m_seatInterface->selection());
-    m_seatInterface->setFocusedKeyboardSurface(serverSurface);
-    QCOMPARE(m_seatInterface->focusedKeyboardSurface(), serverSurface);
-
-    // now let's set the selection
-    m_seatInterface->setSelection(ddi);
 }
 
 void TestWaylandSeat::testDataDeviceForKeyboardSurface()
@@ -2021,7 +1973,7 @@ void TestWaylandSeat::testDataDeviceForKeyboardSurface()
     QVERIFY(ddiCreatedSpy.wait());
     auto ddi = ddiCreatedSpy.first().first().value<DataDeviceInterface*>();
     QVERIFY(ddi);
-    m_seatInterface->setSelection(ddi);
+    m_seatInterface->setSelection(ddi->selection());
 
     // switch to other client
     // create a surface and pass it keyboard focus

src/wayland/datadevice_interface.cpp

@@ -209,15 +209,10 @@ DataSourceInterface *DataDeviceInterface::selection() const
     return d->selection;
 }
 
-void DataDeviceInterface::sendSelection(DataDeviceInterface *other)
+void DataDeviceInterface::sendSelection(DataSourceInterface *other)
 {
     Q_D();
-    auto otherSelection = other->selection();
-    if (!otherSelection) {
-        sendClearSelection();
-        return;
-    }
-    auto r = d->createDataOffer(otherSelection);
+    auto r = d->createDataOffer(other);
     if (!r) {
         return;
     }

src/wayland/datadevice_interface.h

@@ -51,7 +51,7 @@ public:
 
     DataSourceInterface *selection() const;
 
-    void sendSelection(DataDeviceInterface *other);
+    void sendSelection(DataSourceInterface *other);
     void sendClearSelection();
     /**
      * The event is sent when a drag-and-drop operation is ended because the implicit grab is removed.

src/wayland/seat_interface.cpp

@@ -253,26 +253,17 @@ void SeatInterface::Private::registerDataDevice(DataDeviceInterface *dataDevice)
     auto dataDeviceCleanup = [this, dataDevice] {
         dataDevices.removeOne(dataDevice);
         keys.focus.selections.removeOne(dataDevice);
-
-        if (currentSelection == dataDevice) {
-            // current selection is cleared
-            currentSelection = nullptr;
-            emit q->selectionChanged(nullptr);
-            for (auto selection: keys.focus.selections) {
-                selection->sendClearSelection();
-            }
-        }
     };
     QObject::connect(dataDevice, &QObject::destroyed, q, dataDeviceCleanup);
     QObject::connect(dataDevice, &Resource::unbound, q, dataDeviceCleanup);
     QObject::connect(dataDevice, &DataDeviceInterface::selectionChanged, q,
         [this, dataDevice] {
-            updateSelection(dataDevice, true);
+            updateSelection(dataDevice);
         }
     );
     QObject::connect(dataDevice, &DataDeviceInterface::selectionCleared, q,
         [this, dataDevice] {
-            updateSelection(dataDevice, false);
+            updateSelection(dataDevice);
         }
     );
     QObject::connect(dataDevice, &DataDeviceInterface::dragStarted, q,
@@ -331,14 +322,13 @@ void SeatInterface::Private::registerDataDevice(DataDeviceInterface *dataDevice)
         // same client?
         if (keys.focus.surface->client() == dataDevice->client()) {
             keys.focus.selections.append(dataDevice);
-            if (currentSelection && currentSelection->selection()) {
+            if (currentSelection) {
                 dataDevice->sendSelection(currentSelection);
             }
         }
     }
 }
 
-
 void SeatInterface::Private::registerTextInput(TextInputInterface *ti)
 {
     // text input version 0 might call this multiple times
@@ -382,44 +372,13 @@ void SeatInterface::Private::endDrag(quint32 serial)
     emit q->dragEnded();
 }
 
-void SeatInterface::Private::cancelPreviousSelection(DataDeviceInterface *dataDevice)
+void SeatInterface::Private::updateSelection(DataDeviceInterface *dataDevice)
 {
-    if (!currentSelection) {
+    // if the update is from the focussed window we should inform the active client
+    if (!(keys.focus.surface && (keys.focus.surface->client() == dataDevice->client()))) {
         return;
     }
-    if (auto s = currentSelection->selection()) {
-        if (currentSelection != dataDevice) {
-            // only if current selection is not on the same device
-            // that would cancel the newly set source
-            s->cancel();
-        }
-    }
-}
-
-void SeatInterface::Private::updateSelection(DataDeviceInterface *dataDevice, bool set)
-{
-    bool selChanged = currentSelection != dataDevice;
-    if (keys.focus.surface && (keys.focus.surface->client() == dataDevice->client())) {
-        // cancel the previous selection
-        cancelPreviousSelection(dataDevice);
-        // new selection on a data device belonging to current keyboard focus
-        currentSelection = dataDevice;
-    }
-    if (dataDevice == currentSelection) {
-        // need to send out the selection
-        for (auto focussedDevice: qAsConst(keys.focus.selections)) {
-            if (set) {
-                focussedDevice->sendSelection(dataDevice);
-            } else {
-                focussedDevice->sendClearSelection();
-                currentSelection = nullptr;
-                selChanged = true;
-            }
-        }
-    }
-    if (selChanged) {
-        emit q->selectionChanged(currentSelection);
-    }
+    q->setSelection(dataDevice->selection());
 }
 
 void SeatInterface::setHasKeyboard(bool has)
@@ -1595,29 +1554,43 @@ TextInputInterface *SeatInterface::focusedTextInput() const
     return d->textInput.focus.textInput;
 }
 
-DataDeviceInterface *SeatInterface::selection() const
+DataSourceInterface *SeatInterface::selection() const
 {
     Q_D();
     return d->currentSelection;
 }
 
-void SeatInterface::setSelection(DataDeviceInterface *dataDevice)
+void SeatInterface::setSelection(DataSourceInterface *selection)
 {
     Q_D();
-    if (d->currentSelection == dataDevice) {
+    if (d->currentSelection == selection) {
         return;
     }
-    // cancel the previous selection
-    d->cancelPreviousSelection(dataDevice);
-    d->currentSelection = dataDevice;
-    for (auto focussedDevice: qAsConst(d->keys.focus.selections)) {
-        if (dataDevice && dataDevice->selection()) {
-            focussedDevice->sendSelection(dataDevice);
+
+    if (d->currentSelection) {
+        d->currentSelection->cancel();
+        disconnect(d->currentSelection, nullptr, this, nullptr);
+    }
+
+    if (selection) {
+        auto cleanup = [this]() {
+            setSelection(nullptr);
+        };
+        connect(selection, &DataSourceInterface::unbound, this, cleanup);
+        connect(selection, &QObject::destroyed, this, cleanup);
+    }
+
+    d->currentSelection = selection;
+
+    for (auto focussedSelection: qAsConst(d->keys.focus.selections)) {
+        if (selection) {
+            focussedSelection->sendSelection(selection);
         } else {
-            focussedDevice->sendClearSelection();
+            focussedSelection->sendClearSelection();
         }
     }
-    emit selectionChanged(dataDevice);
+
+    emit selectionChanged(selection);
 }
 
 }

src/wayland/seat_interface.h

@@ -23,6 +23,7 @@ namespace KWaylandServer
 {
 
 class DataDeviceInterface;
+class DataSourceInterface;
 class Display;
 class SurfaceInterface;
 class TextInputInterface;
@@ -692,8 +693,10 @@ public:
      * @since 5.24
      * @see selectionChanged
      * @see setSelection
+     * This may be null
      **/
-    DataDeviceInterface *selection() const;
+    DataSourceInterface *selection() const;
+
     /**
      * This method allows to manually set the @p dataDevice for the current clipboard selection.
      * The clipboard selection is handled automatically in SeatInterface.
@@ -707,7 +710,7 @@ public:
      * @see selectionChanged
      * @since 5.24
      **/
-    void setSelection(DataDeviceInterface *dataDevice);
+    void setSelection(DataSourceInterface *selection);
 
     static SeatInterface *get(wl_resource *native);
 
@@ -736,7 +739,7 @@ Q_SIGNALS:
      * @see selection
      * @see setSelection
      **/
-    void selectionChanged(DataDeviceInterface*);
+    void selectionChanged(DataSourceInterface*);
 
     /**
      * Emitted when a drag'n'drop operation is started

src/wayland/seat_interface_p.h

@@ -37,7 +37,6 @@ public:
     void registerDataDevice(DataDeviceInterface *dataDevice);
     void registerTextInput(TextInputInterface *textInput);
     void endDrag(quint32 serial);
-    void cancelPreviousSelection(DataDeviceInterface *newlySelectedDataDevice);
 
     QString name;
     bool pointer = false;
@@ -49,8 +48,11 @@ public:
     QVector<KeyboardInterface*> keyboards;
     QVector<TouchInterface*> touchs;
     QVector<DataDeviceInterface*> dataDevices;
+
     QVector<TextInputInterface*> textInputs;
-    DataDeviceInterface *currentSelection = nullptr;
+
+    // the last thing copied into the clipboard content
+    DataSourceInterface *currentSelection = nullptr;
 
     // Pointer related members
     struct Pointer {
@@ -165,7 +167,7 @@ private:
     void getPointer(wl_client *client, wl_resource *resource, uint32_t id);
     void getKeyboard(wl_client *client, wl_resource *resource, uint32_t id);
     void getTouch(wl_client *client, wl_resource *resource, uint32_t id);
-    void updateSelection(DataDeviceInterface *dataDevice, bool set);
+    void updateSelection(DataDeviceInterface *dataDevice);
     static Private *cast(wl_resource *r);
     static void unbind(wl_resource *r);