Move PlasmaWindowInterface lifespan management to the caller

PlasmaWindowInterface is a "Server-managed multicasting resources". We
no longer need our wrapper to outlive objects so we can tidy that up.

It's weird to have a method call to an object delete the object, so
memory management is moved to the caller to be consistent.

src/wayland/autotests/client/test_plasma_window_model.cpp

@@ -286,7 +286,7 @@ void PlasmaWindowModelTest::testAddRemoveRows()
     // now let's remove that again
     QSignalSpy rowRemovedSpy(model, &PlasmaWindowModel::rowsRemoved);
     QVERIFY(rowRemovedSpy.isValid());
-    w->unmap();
+    w->deleteLater();
     QVERIFY(rowRemovedSpy.wait());
     QCOMPARE(rowRemovedSpy.count(), 1);
     QVERIFY(!rowRemovedSpy.first().at(0).toModelIndex().isValid());
@@ -296,10 +296,6 @@ void PlasmaWindowModelTest::testAddRemoveRows()
     // now the model is empty again
     QCOMPARE(model->rowCount(), 0);
     QVERIFY(!model->index(0).isValid());
-
-    QSignalSpy wDestroyedSpy(w, &QObject::destroyed);
-    QVERIFY(wDestroyedSpy.isValid());
-    QVERIFY(wDestroyedSpy.wait());
 }
 
 void PlasmaWindowModelTest::testDefaultData_data()
@@ -826,8 +822,7 @@ void PlasmaWindowModelTest::testCreateWithUnmappedWindow()
     QVERIFY(unmappedSpy.isValid());
     QSignalSpy destroyedSpy(window, &PlasmaWindow::destroyed);
     QVERIFY(destroyedSpy.isValid());
-    // unmap should be triggered, but not yet the destroyed
-    w->unmap();
+    w->deleteLater();
     QVERIFY(unmappedSpy.wait());
     QVERIFY(destroyedSpy.isEmpty());
 

src/wayland/autotests/client/test_wayland_windowmanagement.cpp

@@ -241,15 +241,12 @@ void TestWindowManagement::testUseAfterUnmap()
     QVERIFY(unmappedSpy.isValid());
     QSignalSpy destroyedSpy(m_window, &QObject::destroyed);
     QVERIFY(destroyedSpy.isValid());
-    m_windowInterface->unmap();
+    m_windowInterface->deleteLater();
+    m_windowInterface = nullptr;
     m_window->requestClose();
     QVERIFY(unmappedSpy.wait());
     QVERIFY(destroyedSpy.wait());
     m_window = nullptr;
-    QSignalSpy serverDestroyedSpy(m_windowInterface, &QObject::destroyed);
-    QVERIFY(serverDestroyedSpy.isValid());
-    QVERIFY(serverDestroyedSpy.wait());
-    m_windowInterface = nullptr;
 }
 
 void TestWindowManagement::testServerDelete()
@@ -283,14 +280,11 @@ void TestWindowManagement::testActiveWindowOnUnmapped()
     QVERIFY(destroyedSpy.isValid());
     QSignalSpy serverDestroyedSpy(m_windowInterface, &QObject::destroyed);
     QVERIFY(serverDestroyedSpy.isValid());
-    m_windowManagementInterface->unmapWindow(m_windowInterface);
+    delete m_windowInterface;
+    m_windowInterface = nullptr;
     QVERIFY(activeWindowChangedSpy.wait());
     QVERIFY(!m_windowManagement->activeWindow());
-    QVERIFY(destroyedSpy.wait());
-    QCOMPARE(destroyedSpy.count(), 1);
-    m_window = nullptr;
-    QVERIFY(serverDestroyedSpy.wait());
-    m_windowInterface = nullptr;
+
 }
 
 void TestWindowManagement::testDeleteActiveWindow()
@@ -318,14 +312,16 @@ void TestWindowManagement::testCreateAfterUnmap()
 {
     // this test verifies that we don't get a protocol error on client side when creating an already unmapped window.
     QCOMPARE(m_windowManagement->children().count(), 1);
+
+    QSignalSpy windowAddedSpy(m_windowManagement, &KWayland::Client::PlasmaWindowManagement::windowCreated);
+
     // create and unmap in one go
     // client will first handle the create, the unmap will be sent once the server side is bound
-    auto serverWindow = m_windowManagementInterface->createWindow(this, QUuid::createUuid());
-    serverWindow->unmap();
+    auto serverWindow = m_windowManagementInterface->createWindow(m_windowManagementInterface, QUuid::createUuid());
+    delete serverWindow;
     QCOMPARE(m_windowManagementInterface->children().count(), 0);
-    QCoreApplication::instance()->processEvents();
-    QCoreApplication::instance()->processEvents(QEventLoop::WaitForMoreEvents);
-    QTRY_COMPARE(m_windowManagement->children().count(), 2);
+
+    windowAddedSpy.wait();
     auto window = dynamic_cast<KWayland::Client::PlasmaWindow*>(m_windowManagement->children().last());
     QVERIFY(window);
     // now this is not yet on the server, on the server it will be after next roundtrip
@@ -334,15 +330,8 @@ void TestWindowManagement::testCreateAfterUnmap()
     QSignalSpy clientDestroyedSpy(window, &QObject::destroyed);
     QVERIFY(clientDestroyedSpy.isValid());
     QVERIFY(clientDestroyedSpy.wait());
+    // Verify that any wrappers created for our temporary window are now gone
     QCOMPARE(m_windowManagement->children().count(), 1);
-    // the server side created a helper PlasmaWindowInterface with PlasmaWindowManagementInterface as parent
-    // it emitted unmapped so we can be sure it will be destroyed directly
-    QCOMPARE(m_windowManagementInterface->children().count(), 1);
-    auto helperWindow = qobject_cast<KWaylandServer::PlasmaWindowInterface*>(m_windowManagementInterface->children().first());
-    QVERIFY(helperWindow);
-    QSignalSpy helperDestroyedSpy(helperWindow, &QObject::destroyed);
-    QVERIFY(helperDestroyedSpy.isValid());
-    QVERIFY(helperDestroyedSpy.wait());
 }
 
 void TestWindowManagement::testRequests_data()
@@ -515,7 +504,7 @@ void TestWindowManagement::testParentWindow()
     // now let's create a second window
     QSignalSpy windowAddedSpy(m_windowManagement, &PlasmaWindowManagement::windowCreated);
     QVERIFY(windowAddedSpy.isValid());
-    auto serverTransient = m_windowManagementInterface->createWindow(this, QUuid::createUuid());
+    QScopedPointer<KWaylandServer::PlasmaWindowInterface> serverTransient(m_windowManagementInterface->createWindow(this, QUuid::createUuid()));
     serverTransient->setParentWindow(m_windowInterface);
     QVERIFY(windowAddedSpy.wait());
     auto transient = windowAddedSpy.first().first().value<PlasmaWindow*>();
@@ -534,7 +523,7 @@ void TestWindowManagement::testParentWindow()
     QCOMPARE(transient->parentWindow().data(), parentWindow);
 
     // now let's try to unmap the parent
-    m_windowInterface->unmap();
+    m_windowInterface->deleteLater();
     m_window = nullptr;
     m_windowInterface = nullptr;
     QVERIFY(parentWindowChangedSpy.wait());
@@ -628,8 +617,6 @@ void TestWindowManagement::testPid()
     QScopedPointer<PlasmaWindow> newWindow( windowSpy.first().first().value<KWayland::Client::PlasmaWindow *>());
     QVERIFY(newWindow);
     QVERIFY(newWindow->pid() == 0);
-
-
 }
 
 void TestWindowManagement::testApplicationMenu()

src/wayland/plasmawindowmanagement_interface.cpp

@@ -73,7 +73,6 @@ public:
     PlasmaWindowManagementInterface *wm;
 
     bool unmapped = false;
-    bool destroyed = false;
     PlasmaWindowInterface *parentWindow = nullptr;
     QMetaObject::Connection parentWindowDestroyConnection;
     QStringList plasmaVirtualDesktops;
@@ -92,7 +91,6 @@ public:
 
 protected:
     void org_kde_plasma_window_bind_resource(Resource *resource) override;
-    void org_kde_plasma_window_destroy_resource(Resource *resource) override;
     void org_kde_plasma_window_set_state(Resource *resource, uint32_t flags, uint32_t state) override;
     void org_kde_plasma_window_set_virtual_desktop(Resource *resource, uint32_t number) override;
     void org_kde_plasma_window_set_minimized_geometry(Resource *resource, wl_resource *panel, uint32_t x, uint32_t y, uint32_t width, uint32_t height) override;
@@ -199,10 +197,9 @@ void PlasmaWindowManagementInterfacePrivate::org_kde_plasma_window_management_ge
             return;
         }
     }
-    // create a temp window just for the resource and directly send an unmapped
-    PlasmaWindowInterface *window = new PlasmaWindowInterface(q, q);
-    window->d->unmapped = true;
-    window->d->add(resource->client(), id, resource->version());
+    // create a temp window just for the resource, bind then immediately delete it, sending an unmap event
+    PlasmaWindowInterface window(q, q);
+    window.d->add(resource->client(), id, resource->version());
 }
 
 void PlasmaWindowManagementInterfacePrivate::org_kde_plasma_window_management_get_window_by_uuid(Resource *resource, uint32_t id, const QString &internal_window_uuid)
@@ -214,10 +211,9 @@ void PlasmaWindowManagementInterfacePrivate::org_kde_plasma_window_management_ge
     );
     if (it == windows.constEnd()) {
         qCWarning(KWAYLAND_SERVER) << "Could not find window with uuid" << internal_window_uuid;
-        // create a temp window just for the resource and directly send an unmapped
-        PlasmaWindowInterface *window = new PlasmaWindowInterface(q, q);
-        window->d->unmapped = true;
-        window->d->add(resource->client(), id, resource->version());
+        // create a temp window just for the resource, bind then immediately delete it, sending an unmap event
+        PlasmaWindowInterface window(q, q);
+        window.d->add(resource->client(), id, resource->version());
         return;
     }
     (*it)->d->add(resource->client(), id, resource->version());
@@ -269,16 +265,6 @@ QList<PlasmaWindowInterface*> PlasmaWindowManagementInterface::windows() const
     return d->windows;
 }
 
-void PlasmaWindowManagementInterface::unmapWindow(PlasmaWindowInterface *window)
-{
-    if (!window) {
-        return;
-    }
-    d->windows.removeOne(window);
-    Q_ASSERT(!d->windows.contains(window));
-    window->d->unmap();
-}
-
 void PlasmaWindowManagementInterface::setStackingOrder(const QVector<quint32>& stackingOrder)
 {
     if (d->stackingOrder == stackingOrder) {
@@ -311,14 +297,7 @@ PlasmaWindowInterfacePrivate::PlasmaWindowInterfacePrivate(PlasmaWindowManagemen
 
 PlasmaWindowInterfacePrivate::~PlasmaWindowInterfacePrivate()
 {
-    destroyed = true;
-    const auto clientResources = resourceMap();
-    for (auto resource : clientResources) {
-        if(!unmapped) {
-            send_unmapped(resource->handle);
-        }
-        wl_resource_destroy(resource->handle);
-    }
+    unmap();
 }
 
 void PlasmaWindowInterfacePrivate::org_kde_plasma_window_destroy(Resource *resource)
@@ -326,15 +305,6 @@ void PlasmaWindowInterfacePrivate::org_kde_plasma_window_destroy(Resource *resou
     wl_resource_destroy(resource->handle);
 }
 
-void PlasmaWindowInterfacePrivate::org_kde_plasma_window_destroy_resource(Resource *resource)
-{
-    Q_UNUSED(resource)
-    // Auto destruct when all resources gone
-    if (unmapped && resourceMap().isEmpty() && !destroyed) { 
-        delete q;
-    }
-}
-
 void PlasmaWindowInterfacePrivate::org_kde_plasma_window_bind_resource(Resource *resource)
 {
 
@@ -366,10 +336,6 @@ void PlasmaWindowInterfacePrivate::org_kde_plasma_window_bind_resource(Resource
 
     send_parent_window(resource->handle, resourceForParent(parentWindow, resource));
 
-    if (unmapped) {
-        send_unmapped(resource->handle);
-    }
-
     if (geometry.isValid() && resource->version() >= ORG_KDE_PLASMA_WINDOW_GEOMETRY_SINCE_VERSION) {
         send_geometry(resource->handle, geometry.x(), geometry.y(), geometry.width(), geometry.height());
     }
@@ -493,16 +459,15 @@ void PlasmaWindowInterfacePrivate::setVirtualDesktop(quint32 desktop)
 
 void PlasmaWindowInterfacePrivate::unmap()
 {
+    if (unmapped) {
+        return;
+    }
     unmapped = true;
     const auto clientResources = resourceMap();
 
     for (auto resource : clientResources) {
         send_unmapped(resource->handle);
     }
-
-    if (clientResources.isEmpty()) {
-        delete q;
-    }
 }
 
 void PlasmaWindowInterfacePrivate::setState(org_kde_plasma_window_management_state flag, bool set)
@@ -751,7 +716,7 @@ void PlasmaWindowInterface::setVirtualDesktop(quint32 desktop)
 
 void PlasmaWindowInterface::unmap()
 {
-    d->wm->unmapWindow(this);
+    d->unmap();
 }
 
 QHash<SurfaceInterface*, QRect>  PlasmaWindowInterface::minimizedGeometries() const

src/wayland/plasmawindowmanagement_interface.h

@@ -40,23 +40,6 @@ public:
     PlasmaWindowInterface *createWindow(QObject *parent, const QUuid &uuid);
     QList<PlasmaWindowInterface*> windows() const;
 
-    /**
-     * Unmaps the @p window previously created with {@link createWindow}.
-     * The window will be unmapped and removed from the list of {@link windows}.
-     *
-     * Unmapping a @p window indicates to the client that it should destroy the
-     * resource created for the window. Once all resources for the @p window are
-     * destroyed, the @p window will get deleted automatically. There is no need
-     * to manually delete the @p window. A manual delete will trigger the unmap
-     * and resource destroy at the same time and can result in protocol errors on
-     * client side if it still accesses the resource before receiving the unmap event.
-     *
-     * @see createWindow
-     * @see windows
-     * @since 5.23
-     **/
-    void unmapWindow(PlasmaWindowInterface *window);
-
     /**
      * Associate a PlasmaVirtualDesktopManagementInterface to this window management.
      * It's necessary to associate one in orderto use the plasma virtual desktop features
@@ -149,10 +132,9 @@ public:
 
     /**
      * This method removes the Window and the Client is supposed to release the resource
-     * bound for this Window. Once all resources are released the Window gets deleted.
+     * bound for this Window.
      *
-     * Prefer using {@link PlasmaWindowManagementInterface::unmapWindow}.
-     * @see PlasmaWindowManagementInterface::unmapWindow
+     * No more events should be sent afterwards.
      **/
     void unmap();