refactor(server): create client through separate function

Do not implicitly create a client with a call to getClient. Instead use a separate function for that. This makes the API more explicit. BREAKING CHANGE: `Display::getClient` doesn't create clients implicitly
winft · Jan 6, 2024 · 21b56ca · 21b56ca
1 parent da46a25
commit 21b56ca
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 9 deletions.
diff --git a/autotests/server/display.cpp b/autotests/server/display.cpp
@@ -136,8 +136,12 @@ void TestServerDisplay::testClientConnection()
 
     QVERIFY(connectedSpy.isEmpty());
     QVERIFY(display.clients().empty());
+
     auto connection = display.getClient(wlClient);
+    QVERIFY(!connection);
+    connection = display.createClient(wlClient);
     QVERIFY(connection);
+
     QCOMPARE(connection->native(), wlClient);
 
     if (getuid() == 0) {

diff --git a/server/display.cpp b/server/display.cpp
@@ -98,9 +98,6 @@ Wayland::Client* Private::castClientImpl(Server::Client* client)
 
 Client* Private::createClientHandle(wl_client* wlClient)
 {
-    if (auto* client = getClient(wlClient)) {
-        return client->handle;
-    }
     auto* clientHandle = new Client(wlClient, q_ptr);
     setupClient(clientHandle->d_ptr.get());
     return clientHandle;
@@ -180,9 +177,12 @@ wl_display* Display::native() const
     return d_ptr->native();
 }
 
-Client* Display::getClient(wl_client* wlClient)
+Client* Display::getClient(wl_client* wlClient) const
 {
-    return d_ptr->createClientHandle(wlClient);
+    if (auto client = d_ptr->getClient(wlClient)) {
+        return client->handle;
+    }
+    return nullptr;
 }
 
 std::vector<Client*> Display::clients() const
@@ -194,9 +194,16 @@ std::vector<Client*> Display::clients() const
     return ret;
 }
 
+Client* Display::createClient(wl_client* wlClient)
+{
+    assert(!getClient(wlClient));
+    return d_ptr->createClientHandle(wlClient);
+}
+
 Client* Display::createClient(int fd)
 {
-    return getClient(d_ptr->createClient(fd));
+    auto wlClient = d_ptr->createClient(fd);
+    return createClient(wlClient);
 }
 
 void Display::setEglDisplay(void* display)

diff --git a/server/display.h b/server/display.h
@@ -108,10 +108,12 @@ class WRAPLANDSERVER_EXPORT Display : public QObject
     void dispatch();
     void flush();
 
-    Client* createClient(int fd);
-    Client* getClient(wl_client* client);
+    Client* getClient(wl_client* client) const;
     std::vector<Client*> clients() const;
 
+    Client* createClient(wl_client* client);
+    Client* createClient(int fd);
+
     void setEglDisplay(void* display);
     void* eglDisplay() const;
 

diff --git a/server/filtered_display.cpp b/server/filtered_display.cpp
@@ -54,6 +54,11 @@ bool FilteredDisplay::Private::filterCallback(wl_client const* wlClient,
     // respective clang-tidy check.
     // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
     auto client = priv->q_ptr->getClient(const_cast<wl_client*>(wlClient));
+    if (!client) {
+        // NOLINTNEXTLINE(cppcoreguidelines-pro-type-const-cast)
+        client = priv->q_ptr->createClient(const_cast<wl_client*>(wlClient));
+    }
+
     auto interface = wl_global_get_interface(wlGlobal);
     auto name = QByteArray::fromRawData(interface->name, static_cast<int>(strlen(interface->name)));
 

diff --git a/server/wayland/nucleus.h b/server/wayland/nucleus.h
@@ -145,7 +145,7 @@ class Nucleus : public BasicNucleus
         }
         // Client not yet known to the server.
         // TODO(romangg): Create backend representation only?
-        nucleus->display->handle->getClient(wlClient);
+        nucleus->display->handle->createClient(wlClient);
 
         // Now the client must be available.
         // TODO(romangg): otherwise send protocol error (oom)