Commit 34056bcf authored by David Edmundson's avatar David Edmundson Committed by Vlad Zahorodnii
Browse files

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.
parent 83d6a64a
Pipeline #53818 canceled with stage
......@@ -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());
......
......@@ -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()
......
......@@ -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
......
......@@ -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();
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment