Client: Add test for wl_data_offer leaks

Also refactors the mocking code for data device manager and implements its
isClean method to verify there are no leaks after each test.

MockCompositor::DataDevice now persists across get_data_device requests.

Task-number: QTBUG-73825
Change-Id: Ib5866e0c54d021e12557f9a96f27950010f2611b
Reviewed-by: Paul Olav Tvete <paul.tvete@qt.io>

4 files changed, 153 insertions, 30 deletions

diff --git a/tests/auto/client/datadevicev1/tst_datadevicev1.cpp b/tests/auto/client/datadevicev1/tst_datadevicev1.cpp
index a85b10c74..fe68d520d 100644
--- a/tests/auto/client/datadevicev1/tst_datadevicev1.cpp
+++ b/tests/auto/client/datadevicev1/tst_datadevicev1.cpp
@@ -47,7 +47,7 @@ public:
             add<DataDeviceManager>(dataDeviceVersion);
         });
     }
-    DataDevice *dataDevice() { return get<Seat>()->dataDevice(); }
+    DataDevice *dataDevice() { return get<DataDeviceManager>()->deviceFor(get<Seat>()); }
 };
 
 class tst_datadevicev1 : public QObject, private DataDeviceCompositor
@@ -58,6 +58,8 @@ private slots:
     void initTestCase();
     void pasteAscii();
     void pasteUtf8();
+    void destroysPreviousSelection();
+    void destroysSelectionWithSurface();
 };
 
 void tst_datadevicev1::initTestCase()
@@ -69,7 +71,8 @@ void tst_datadevicev1::initTestCase()
     QCOMPOSITOR_TRY_VERIFY(keyboard());
 
     QCOMPOSITOR_TRY_VERIFY(dataDevice());
-    QCOMPOSITOR_TRY_COMPARE(dataDevice()->resource()->version(), dataDeviceVersion);
+    QCOMPOSITOR_TRY_VERIFY(dataDevice()->resourceMap().contains(client()));
+    QCOMPOSITOR_TRY_COMPARE(dataDevice()->resourceMap().value(client())->version(), dataDeviceVersion);
 }
 
 void tst_datadevicev1::pasteAscii()
@@ -86,7 +89,8 @@ void tst_datadevicev1::pasteAscii()
 
     QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
     exec([&] {
-        auto *offer = new DataOffer(client(), dataDeviceVersion); // Cleaned up by destroy_resource
+        auto *client = xdgSurface()->resource()->client();
+        auto *offer = dataDevice()->sendDataOffer(client, {"text/plain"});
         connect(offer, &DataOffer::receive, [](QString mimeType, int fd) {
             QFile file;
             file.open(fd, QIODevice::WriteOnly, QFile::FileHandleFlag::AutoCloseHandle);
@@ -94,16 +98,14 @@ void tst_datadevicev1::pasteAscii()
             file.write(QByteArray("normal ascii"));
             file.close();
         });
-        dataDevice()->sendDataOffer(offer);
-        offer->send_offer("text/plain");
         dataDevice()->sendSelection(offer);
 
         auto *surface = xdgSurface()->m_surface;
         keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol
 
         pointer()->sendEnter(surface, {32, 32});
-        pointer()->sendButton(client(), BTN_LEFT, 1);
-        pointer()->sendButton(client(), BTN_LEFT, 0);
+        pointer()->sendButton(client, BTN_LEFT, 1);
+        pointer()->sendButton(client, BTN_LEFT, 0);
     });
     QTRY_COMPARE(window.m_text, "normal ascii");
 }
@@ -122,7 +124,8 @@ void tst_datadevicev1::pasteUtf8()
 
     QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
     exec([&] {
-        auto *offer = new DataOffer(client(), dataDeviceVersion); // Cleaned up by destroy_resource
+        auto *client = xdgSurface()->resource()->client();
+        auto *offer = dataDevice()->sendDataOffer(client, {"text/plain", "text/plain;charset=utf-8"});
         connect(offer, &DataOffer::receive, [](QString mimeType, int fd) {
             QFile file;
             file.open(fd, QIODevice::WriteOnly, QFile::FileHandleFlag::AutoCloseHandle);
@@ -130,20 +133,81 @@ void tst_datadevicev1::pasteUtf8()
             file.write(QByteArray("face with tears of joy: 😂"));
             file.close();
         });
-        dataDevice()->sendDataOffer(offer);
-        offer->send_offer("text/plain");
-        offer->send_offer("text/plain;charset=utf-8");
         dataDevice()->sendSelection(offer);
 
         auto *surface = xdgSurface()->m_surface;
         keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol
 
         pointer()->sendEnter(surface, {32, 32});
-        pointer()->sendButton(client(), BTN_LEFT, 1);
-        pointer()->sendButton(client(), BTN_LEFT, 0);
+        pointer()->sendButton(client, BTN_LEFT, 1);
+        pointer()->sendButton(client, BTN_LEFT, 0);
     });
     QTRY_COMPARE(window.m_text, "face with tears of joy: 😂");
 }
 
+void tst_datadevicev1::destroysPreviousSelection()
+{
+    QRasterWindow window;
+    window.resize(64, 64);
+    window.show();
+    QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
+
+    // When the client receives a selection event, it is required to destroy the previous offer
+    exec([&] {
+        QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 0);
+        auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"});
+        dataDevice()->sendSelection(offer);
+        auto *surface = xdgSurface()->m_surface;
+        keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol
+        QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 1);
+    });
+
+    exec([&] {
+        auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"});
+        dataDevice()->sendSelection(offer);
+        QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 2);
+    });
+
+    // Verify the first offer gets destroyed
+    QCOMPOSITOR_TRY_COMPARE(dataDevice()->m_sentSelectionOffers.size(), 1);
+
+    exec([&] {
+        auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"});
+        dataDevice()->sendSelection(offer);
+        auto *surface = xdgSurface()->m_surface;
+        keyboard()->sendLeave(surface);
+    });
+
+    // Clients are required to destroy their offer when losing keyboard focus
+    QCOMPOSITOR_TRY_COMPARE(dataDevice()->m_sentSelectionOffers.size(), 0);
+}
+
+void tst_datadevicev1::destroysSelectionWithSurface()
+{
+    auto *window = new QRasterWindow;
+    window->resize(64, 64);
+    window->show();
+
+    QCOMPOSITOR_TRY_VERIFY(xdgSurface() && xdgSurface()->m_committedConfigureSerial);
+
+    // When the client receives a selection event, it is required to destroy the previous offer
+    exec([&] {
+        QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 0);
+        auto *offer = dataDevice()->sendDataOffer(client(), {"text/plain"});
+        dataDevice()->sendSelection(offer);
+        auto *surface = xdgSurface()->m_surface;
+        keyboard()->sendEnter(surface); // Need to set keyboard focus according to protocol
+        QCOMPARE(dataDevice()->m_sentSelectionOffers.size(), 1);
+    });
+
+    // Ping to make sure we receive the wl_keyboard enter and leave events, before destroying the
+    // surface. Otherwise, the client will receive enter and leave events with a destroyed (null)
+    // surface, which is not what we are trying to test for here.
+    xdgPingAndWaitForPong();
+    window->destroy();
+
+    QCOMPOSITOR_TRY_COMPARE(dataDevice()->m_sentSelectionOffers.size(), 0);
+}
+
 QCOMPOSITOR_TEST_MAIN(tst_datadevicev1)
 #include "tst_datadevicev1.moc"
diff --git a/tests/auto/client/shared/coreprotocol.h b/tests/auto/client/shared/coreprotocol.h
index 699dcbded..5cef476c8 100644
--- a/tests/auto/client/shared/coreprotocol.h
+++ b/tests/auto/client/shared/coreprotocol.h
@@ -250,9 +250,6 @@ public:
     Keyboard* m_keyboard = nullptr;
     QVector<Keyboard *> m_oldKeyboards;
 
-    DataDevice *dataDevice() { return m_dataDevice.data(); }
-    QScopedPointer<DataDevice> m_dataDevice;
-
     uint m_capabilities = 0;
 
 protected:
diff --git a/tests/auto/client/shared/datadevice.cpp b/tests/auto/client/shared/datadevice.cpp
index c136c7596..dfa18952c 100644
--- a/tests/auto/client/shared/datadevice.cpp
+++ b/tests/auto/client/shared/datadevice.cpp
@@ -30,29 +30,60 @@
 
 namespace MockCompositor {
 
+bool DataDeviceManager::isClean()
+{
+    for (auto *device : qAsConst(m_dataDevices)) {
+        // The client should not leak selection offers, i.e. if this fails, there is a missing
+        // data_offer.destroy request
+        if (!device->m_sentSelectionOffers.empty())
+            return false;
+    }
+    return true;
+}
+
+DataDevice *DataDeviceManager::deviceFor(Seat *seat)
+{
+    Q_ASSERT(seat);
+    if (auto *device = m_dataDevices.value(seat, nullptr))
+        return device;
+
+    auto *device = new DataDevice(this, seat);
+    m_dataDevices[seat] = device;
+    return device;
+}
+
 void DataDeviceManager::data_device_manager_get_data_device(Resource *resource, uint32_t id, wl_resource *seatResource)
 {
     auto *seat = fromResource<Seat>(seatResource);
     QVERIFY(seat);
-    QVERIFY(!seat->m_dataDevice);
-    seat->m_dataDevice.reset(new DataDevice(resource->client(), id, resource->version()));
+    auto *device = deviceFor(seat);
+    device->add(resource->client(), id, resource->version());
 }
 
 DataDevice::~DataDevice()
 {
-    // If the client hasn't deleted the wayland object, just ignore subsequent events
-    if (auto *r = resource()->handle)
-        wl_resource_set_implementation(r, nullptr, nullptr, nullptr);
+    // If the client(s) hasn't deleted the wayland object, just ignore subsequent events
+    for (auto *r : resourceMap())
+        wl_resource_set_implementation(r->handle, nullptr, nullptr, nullptr);
 }
 
-void DataDevice::sendDataOffer(DataOffer *offer)
+DataOffer *DataDevice::sendDataOffer(wl_client *client, const QStringList &mimeTypes)
 {
-    wl_data_device::send_data_offer(offer->resource()->handle);
+    Q_ASSERT(client);
+    auto *offer = new DataOffer(this, client, m_manager->m_version);
+    for (auto *resource : resourceMap().values(client))
+        wl_data_device::send_data_offer(resource->handle, offer->resource()->handle);
+    for (const auto &mimeType : mimeTypes)
+        offer->send_offer(mimeType);
+    return offer;
 }
 
 void DataDevice::sendSelection(DataOffer *offer)
 {
-    wl_data_device::send_selection(offer->resource()->handle);
+    auto *client = offer->resource()->client();
+    for (auto *resource : resourceMap().values(client))
+        wl_data_device::send_selection(resource->handle, offer->resource()->handle);
+    m_sentSelectionOffers << offer;
 }
 
 void DataOffer::data_offer_destroy_resource(Resource *resource)
@@ -67,4 +98,11 @@ void DataOffer::data_offer_receive(Resource *resource, const QString &mime_type,
     emit receive(mime_type, fd);
 }
 
+void DataOffer::data_offer_destroy(QtWaylandServer::wl_data_offer::Resource *resource)
+{
+    bool removed = m_dataDevice->m_sentSelectionOffers.removeOne(this);
+    QVERIFY(removed);
+    wl_resource_destroy(resource->handle);
+}
+
 } // namespace MockCompositor
diff --git a/tests/auto/client/shared/datadevice.h b/tests/auto/client/shared/datadevice.h
index 4613db776..a96da86f0 100644
--- a/tests/auto/client/shared/datadevice.h
+++ b/tests/auto/client/shared/datadevice.h
@@ -42,8 +42,14 @@ class DataDeviceManager : public Global, public QtWaylandServer::wl_data_device_
 public:
     explicit DataDeviceManager(CoreCompositor *compositor, int version = 1)
         : QtWaylandServer::wl_data_device_manager(compositor->m_display, version)
+        , m_version(version)
     {}
-    QVector<DataDevice *> m_dataDevices;
+    ~DataDeviceManager() override { qDeleteAll(m_dataDevices); }
+    bool isClean() override;
+    DataDevice *deviceFor(Seat *seat);
+
+    int m_version = 1; // TODO: remove on libwayland upgrade
+    QMap<Seat *, DataDevice *> m_dataDevices;
 
 protected:
     void data_device_manager_get_data_device(Resource *resource, uint32_t id, ::wl_resource *seatResource) override;
@@ -52,24 +58,42 @@ protected:
 class DataDevice : public QtWaylandServer::wl_data_device
 {
 public:
-    explicit DataDevice(::wl_client *client, int id, int version)
-        : QtWaylandServer::wl_data_device(client, id, version)
+    explicit DataDevice(DataDeviceManager *manager, Seat *seat)
+        : m_manager(manager)
+        , m_seat(seat)
     {}
     ~DataDevice() override;
     void send_data_offer(::wl_resource *resource) = delete;
-    void sendDataOffer(DataOffer *offer);
+    DataOffer *sendDataOffer(::wl_client *client, const QStringList &mimeTypes = {});
+    DataOffer *sendDataOffer(const QStringList &mimeTypes = {});
+
     void send_selection(::wl_resource *resource) = delete;
     void sendSelection(DataOffer *offer);
+
+    DataDeviceManager *m_manager = nullptr;
+    Seat *m_seat = nullptr;
+    QVector<DataOffer *> m_sentSelectionOffers;
+
+protected:
+    void data_device_release(Resource *resource) override
+    {
+        int removed = m_manager->m_dataDevices.remove(m_seat);
+        QVERIFY(removed);
+        wl_resource_destroy(resource->handle);
+    }
 };
 
 class DataOffer : public QObject, public QtWaylandServer::wl_data_offer
 {
     Q_OBJECT
 public:
-    explicit DataOffer(::wl_client *client, int version)
+    explicit DataOffer(DataDevice *dataDevice, ::wl_client *client, int version)
         : QtWaylandServer::wl_data_offer (client, 0, version)
+        , m_dataDevice(dataDevice)
     {}
 
+    DataDevice *m_dataDevice = nullptr;
+
 signals:
     void receive(QString mimeType, int fd);
 
@@ -77,7 +101,7 @@ protected:
     void data_offer_destroy_resource(Resource *resource) override;
     void data_offer_receive(Resource *resource, const QString &mime_type, int32_t fd) override;
 //    void data_offer_accept(Resource *resource, uint32_t serial, const QString &mime_type) override;
-//    void data_offer_destroy(Resource *resource) override;
+    void data_offer_destroy(Resource *resource) override;
 };
 
 } // namespace MockCompositor