Add globalRemove to QWaylandClientExtension

Adds support for handling of removal of the relevant global to
QtWaylandClientExtension. The user is responsible for destroying the object
once it becomes inactive to match the behavior on destruction.
Two signals for globals and their removal are added to QWaylandDisplay
to make it a bit nicer to use in a more "Qt-way". The addRregistryListener
function is kept for now until other places are ported.

Change-Id: I4ccbaa32e18a5ae15871aa23639e2b4a372cc34e
Reviewed-by: David Edmundson <davidedmundson@kde.org>

src/plugins/platforms/wayland/global/qwaylandclientextension.cpp

@@ -1,6 +1,7 @@
 /****************************************************************************
 **
 ** Copyright (C) 2017 Erik Larsson.
+** Copyright (C) 2021 David Redondo <qt@david-redondo.de>
 ** Contact: https://www.qt.io/licensing/
 **
 ** This file is part of the QtWaylandCompositor module of the Qt Toolkit.
@@ -48,6 +49,8 @@
 
 QT_BEGIN_NAMESPACE
 
+using RegistryGlobal = QtWaylandClient::QWaylandDisplay::RegistryGlobal;
+
 QWaylandClientExtensionPrivate::QWaylandClientExtensionPrivate()
 {
     // Keep the possibility to use a custom waylandIntegration as a plugin,
@@ -60,23 +63,41 @@ QWaylandClientExtensionPrivate::QWaylandClientExtensionPrivate()
         qWarning() << "This application requires a Wayland platform plugin";
 }
 
-void QWaylandClientExtensionPrivate::handleRegistryGlobal(void *data, ::wl_registry *registry, uint32_t id,
-                                                          const QString &interface, uint32_t version)
+void QWaylandClientExtensionPrivate::globalAdded(const RegistryGlobal &global)
 {
-    QWaylandClientExtension *extension = static_cast<QWaylandClientExtension *>(data);
-    if (interface == QLatin1String(extension->extensionInterface()->name) && !extension->d_func()->active) {
-        extension->bind(registry, id, version);
-        extension->d_func()->active = true;
-        emit extension->activeChanged();
+    Q_Q(QWaylandClientExtension);
+    if (!active && global.interface == QLatin1String(q->extensionInterface()->name)) {
+        q->bind(global.registry, global.id, global.version);
+        active = true;
+        emit q->activeChanged();
+    }
+}
+
+void QWaylandClientExtensionPrivate::globalRemoved(const RegistryGlobal &global)
+{
+    Q_Q(QWaylandClientExtension);
+    if (active && global.interface == QLatin1String(q->extensionInterface()->name)) {
+        active = false;
+        emit q->activeChanged();
     }
 }
 
 void QWaylandClientExtension::initialize()
 {
     Q_D(QWaylandClientExtension);
-    if (!d->registered) {
-        d->waylandIntegration->display()->addRegistryListener(&QWaylandClientExtensionPrivate::handleRegistryGlobal, this);
-        d->registered = true;
+    if (d->active) {
+        return;
+    }
+    const QtWaylandClient::QWaylandDisplay *display = d->waylandIntegration->display();
+    const auto globals = display->globals();
+    auto global =
+            std::find_if(globals.cbegin(), globals.cend(), [this](const RegistryGlobal &global) {
+                return global.interface == QLatin1String(extensionInterface()->name);
+            });
+    if (global != globals.cend()) {
+        bind(global->registry, global->id, global->version);
+        d->active = true;
+        emit activeChanged();
     }
 }
 
@@ -85,17 +106,17 @@ QWaylandClientExtension::QWaylandClientExtension(const int ver)
 {
     Q_D(QWaylandClientExtension);
     d->version = ver;
-
-    // The registry listener uses virtual functions and we don't want it to be called from
-    // the constructor.
+    auto display = d->waylandIntegration->display();
+    QObjectPrivate::connect(display, &QtWaylandClient::QWaylandDisplay::globalAdded, d,
+                            &QWaylandClientExtensionPrivate::globalAdded);
+    QObjectPrivate::connect(display, &QtWaylandClient::QWaylandDisplay::globalRemoved, d,
+                            &QWaylandClientExtensionPrivate::globalRemoved);
+    // This function uses virtual functions and we don't want it to be called from the constructor.
     QMetaObject::invokeMethod(this, "initialize", Qt::QueuedConnection);
 }
 
 QWaylandClientExtension::~QWaylandClientExtension()
 {
-    Q_D(QWaylandClientExtension);
-    if (d->registered && !QCoreApplication::closingDown())
-        d->waylandIntegration->display()->removeListener(&QWaylandClientExtensionPrivate::handleRegistryGlobal, this);
 }
 
 QtWaylandClient::QWaylandIntegration *QWaylandClientExtension::integration() const

src/plugins/platforms/wayland/global/qwaylandclientextension_p.h

@@ -53,22 +53,23 @@
 
 #include <QtCore/private/qobject_p.h>
 #include <QtWaylandClient/QWaylandClientExtension>
+#include <QtWaylandClient/private/qwaylanddisplay_p.h>
 #include <QtWaylandClient/private/qwaylandintegration_p.h>
 
 QT_BEGIN_NAMESPACE
 
 class Q_WAYLAND_CLIENT_EXPORT QWaylandClientExtensionPrivate : public QObjectPrivate
 {
-    Q_DECLARE_PUBLIC(QWaylandClientExtension)
 public:
+    Q_DECLARE_PUBLIC(QWaylandClientExtension)
     QWaylandClientExtensionPrivate();
-    static void handleRegistryGlobal(void *data, ::wl_registry *registry, uint32_t id,
-                                     const QString &interface, uint32_t version);
+
+    void globalAdded(const QtWaylandClient::QWaylandDisplay::RegistryGlobal &global);
+    void globalRemoved(const QtWaylandClient::QWaylandDisplay::RegistryGlobal &global);
 
     QtWaylandClient::QWaylandIntegration *waylandIntegration = nullptr;
     int version = -1;
     bool active = false;
-    bool registered = false;
 };
 
 class Q_WAYLAND_CLIENT_EXPORT QWaylandClientExtensionTemplatePrivate : public QWaylandClientExtensionPrivate

src/plugins/platforms/wayland/qwaylanddisplay.cpp

@@ -634,6 +634,7 @@ void QWaylandDisplay::registry_global(uint32_t id, const QString &interface, uin
     }
 
     mGlobals.append(RegistryGlobal(id, interface, version, registry));
+    emit globalAdded(mGlobals.back());
 
     const auto copy = mRegistryListeners; // be prepared for listeners unregistering on notification
     for (Listener l : copy)
@@ -690,7 +691,7 @@ void QWaylandDisplay::registry_global_remove(uint32_t id)
                     inputDevice->setTextInputMethod(nullptr);
                 mWaylandIntegration->reconfigureInputContext();
             }
-            mGlobals.removeAt(i);
+            emit globalRemoved(mGlobals.takeAt(i));
             break;
         }
     }

src/plugins/platforms/wayland/qwaylanddisplay_p.h

@@ -228,6 +228,10 @@ public slots:
     void blockingReadEvents();
     void flushRequests();
 
+signals:
+    void globalAdded(const RegistryGlobal &global);
+    void globalRemoved(const RegistryGlobal &global);
+
 private:
     void handleWaylandSync();
     void requestWaylandSync();

tests/auto/wayland/CMakeLists.txt

@@ -3,6 +3,7 @@
 add_subdirectory(shared)
 
 add_subdirectory(client)
+add_subdirectory(clientextension)
 add_subdirectory(datadevicev1)
 add_subdirectory(fullscreenshellv1)
 add_subdirectory(iviapplication)

tests/auto/wayland/clientextension/CMakeLists.txt

@@ -0,0 +1,14 @@
+qt_internal_add_test(tst_clientextension
+    SOURCES
+        tst_clientextension.cpp
+    PUBLIC_LIBRARIES
+        SharedClientTest
+)
+
+qt6_generate_wayland_protocol_client_sources(tst_clientextension
+    FILES ${CMAKE_CURRENT_SOURCE_DIR}/test.xml
+)
+
+qt6_generate_wayland_protocol_server_sources(tst_clientextension
+    FILES ${CMAKE_CURRENT_SOURCE_DIR}/test.xml
+)

tests/auto/wayland/clientextension/test.xml

@@ -0,0 +1,6 @@
+<protocol name="test">
+    <interface name="test_interface" version="1">
+          <request name="release" type="destructor" />
+    </interface>
+</protocol>
+

tests/auto/wayland/clientextension/tst_clientextension.cpp

@@ -0,0 +1,152 @@
+/****************************************************************************
+**
+** Copyright (C) 2021 David Redondo <qt@david-redondo.de>
+** Contact: https://www.qt.io/licensing/
+**
+** This file is part of the test suite of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:GPL-EXCEPT$
+** Commercial License Usage
+** Licensees holding valid commercial Qt licenses may use this file in
+** accordance with the commercial license agreement provided with the
+** Software or, alternatively, in accordance with the terms contained in
+** a written agreement between you and The Qt Company. For licensing terms
+** and conditions see https://www.qt.io/terms-conditions. For further
+** information use the contact form at https://www.qt.io/contact-us.
+**
+** GNU General Public License Usage
+** Alternatively, this file may be used under the terms of the GNU
+** General Public License version 3 as published by the Free Software
+** Foundation with exceptions as appearing in the file LICENSE.GPL3-EXCEPT
+** included in the packaging of this file. Please review the following
+** information to ensure the GNU General Public License requirements will
+** be met: https://www.gnu.org/licenses/gpl-3.0.html.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#include <QSignalSpy>
+#include <QtGui/private/qguiapplication_p.h>
+#include <QtWaylandClient/private/qwayland-wayland.h>
+#include <QtWaylandClient/private/qwaylanddisplay_p.h>
+#include <QtWaylandClient/private/qwaylandintegration_p.h>
+#include <QtWaylandClient/qwaylandclientextension.h>
+#include <qwayland-server-test.h>
+#include <qwayland-test.h>
+#include "mockcompositor.h"
+#include "coreprotocol.h"
+
+using namespace MockCompositor;
+
+class TestExtension : public QWaylandClientExtensionTemplate<TestExtension>,
+                      public QtWayland::test_interface
+{
+public:
+    TestExtension() : QWaylandClientExtensionTemplate<TestExtension>(1) { }
+    ~TestExtension()
+    {
+        if (object()) {
+            release();
+        }
+    }
+    void initialize() { QWaylandClientExtension::initialize(); }
+};
+
+class TestGlobal : public Global, public QtWaylandServer::test_interface
+{
+    Q_OBJECT
+public:
+    explicit TestGlobal(CoreCompositor *compositor)
+        : QtWaylandServer::test_interface(compositor->m_display, 1)
+    {
+    }
+};
+
+class tst_clientextension : public QObject, private CoreCompositor
+{
+    Q_OBJECT
+private:
+    QtWaylandClient::QWaylandDisplay *display()
+    {
+        return static_cast<QtWaylandClient::QWaylandIntegration *>(
+                       QGuiApplicationPrivate::platformIntegration())
+                ->display();
+    }
+private slots:
+    void cleanup()
+    {
+        display()->flushRequests();
+        dispatch();
+        exec([this] { removeAll<TestGlobal>(); });
+        QTRY_COMPARE(display()->globals().size(), 0);
+    }
+    void createWithoutGlobal();
+    void createWithGlobalAutomatic();
+    void createWithGlobalManual();
+    void globalBecomesAvailable();
+    void globalRemoved();
+};
+
+void tst_clientextension::createWithoutGlobal()
+{
+    TestExtension extension;
+    QSignalSpy spy(&extension, &QWaylandClientExtension::activeChanged);
+    QVERIFY(spy.isValid());
+    QVERIFY(!extension.isActive());
+    QCOMPARE(spy.count(), 0);
+    extension.initialize();
+    QVERIFY(!extension.isActive());
+    QCOMPARE(spy.count(), 0);
+}
+
+void tst_clientextension::createWithGlobalAutomatic()
+{
+    exec([this] { add<TestGlobal>(); });
+    QTRY_COMPARE(display()->globals().size(), 1);
+    TestExtension extension;
+    QSignalSpy spy(&extension, &QWaylandClientExtension::activeChanged);
+    QVERIFY(spy.isValid());
+    QTRY_VERIFY(extension.isActive());
+    QCOMPARE(spy.count(), 1);
+}
+
+void tst_clientextension::createWithGlobalManual()
+{
+    exec([this] { add<TestGlobal>(); });
+    QTRY_COMPARE(display()->globals().size(), 1);
+    // Wait for the display to have the global
+    TestExtension extension;
+    QSignalSpy spy(&extension, &QWaylandClientExtension::activeChanged);
+    QVERIFY(spy.isValid());
+    extension.initialize();
+    QVERIFY(extension.isActive());
+    QCOMPARE(spy.count(), 1);
+}
+
+void tst_clientextension::globalBecomesAvailable()
+{
+    TestExtension extension;
+    QSignalSpy spy(&extension, &QWaylandClientExtension::activeChanged);
+    QVERIFY(spy.isValid());
+    exec([this] { add<TestGlobal>(); });
+    QTRY_VERIFY(extension.isActive());
+    QCOMPARE(spy.count(), 1);
+}
+
+void tst_clientextension::globalRemoved()
+{
+    exec([this] { add<TestGlobal>(); });
+    TestExtension extension;
+    QTRY_VERIFY(extension.isActive());
+    QSignalSpy spy(&extension, &QWaylandClientExtension::activeChanged);
+    QVERIFY(spy.isValid());
+    QCOMPOSITOR_TRY_COMPARE(get<TestGlobal>()->resourceMap().size(), 1);
+
+    exec([this] { removeAll<TestGlobal>(); });
+    QTRY_VERIFY(!extension.isActive());
+    QCOMPARE(spy.count(), 1);
+}
+
+QCOMPOSITOR_TEST_MAIN(tst_clientextension)
+#include "tst_clientextension.moc"