feat: Upgrade treeland-ddm protocol to version 2#750
Open
calsys456 wants to merge 3 commits intolinuxdeepin:masterfrom
Open
feat: Upgrade treeland-ddm protocol to version 2#750calsys456 wants to merge 3 commits intolinuxdeepin:masterfrom
calsys456 wants to merge 3 commits intolinuxdeepin:masterfrom
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: calsys456 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Collaborator
Author
|
Paired with linuxdeepin/treeland-protocols#47 and linuxdeepin/ddm#85 |
Reviewer's GuideMigrates DDM integration from a custom QLocalSocket/v1 protocol to a new Wayland-based treeland-ddm v2 server interface, rewiring greeter, helper, and session management to use DDMInterfaceV2, and updating session IDs to be QString-based throughout. Sequence diagram for login flow via treeland-ddm v2sequenceDiagram
actor User
participant GreeterUI
participant GreeterProxy
participant Helper
participant DDMInterfaceV2
participant WaylandServer
participant DDMClient
User->>GreeterUI: Enter credentials and select session
GreeterUI->>GreeterProxy: login(user, password, sessionIndex)
GreeterProxy->>SessionModel: index(sessionIndex, 0)
SessionModel-->>GreeterProxy: QModelIndex
GreeterProxy->>SessionModel: data(index, TypeRole)
SessionModel-->>GreeterProxy: sessionType
GreeterProxy->>SessionModel: data(index, FileRole)
SessionModel-->>GreeterProxy: sessionFile
alt DDMInterfaceV2 is valid
GreeterProxy->>DDMInterfaceV2: login(user, password, sessionType, sessionFile)
DDMInterfaceV2->>WaylandServer: treeland_ddm_v2_send_login(...)
WaylandServer->>DDMClient: treeland-ddm-v2 login request
alt Authentication succeeds
DDMClient-->>WaylandServer: user_logged_in(username, session)
WaylandServer->>DDMInterfaceV2: user_logged_in(username, session)
DDMInterfaceV2-->>GreeterProxy: userLoggedIn(username, session)
GreeterProxy->>Helper: setLock(false)
else Authentication fails
DDMClient-->>WaylandServer: authentication_failed(error)
WaylandServer->>DDMInterfaceV2: authentication_failed(error)
DDMInterfaceV2-->>GreeterProxy: authenticationFailed(error)
GreeterProxy-->>GreeterUI: failedAttemptsChanged
end
else DDMInterfaceV2 not valid
GreeterProxy->>GreeterProxy: localValidation(user, password)
alt Local validation succeeds
GreeterProxy->>Helper: setLock(false)
else Local validation fails
GreeterProxy-->>GreeterUI: failedAttemptsChanged
end
end
Class diagram for DDMInterfaceV2 integration and session ID changesclassDiagram
direction LR
class QObject
class WServerInterface {
<<interface>>
+QByteArrayView interfaceName()
+void create(WServer server)
+void destroy(WServer server)
+wl_global* global()
}
class DDMInterfaceV2 {
+QByteArrayView interfaceName()
+void setHandle(wl_resource* handle)
+static QString authErrorToString(uint32_t error)
+void login(QString username, QString password, DDM_Session_Type sessionType, QString sessionFile)
+void logout(QString session)
+void lock(QString session)
+void unlock(QString session, QString password)
+void powerOff()
+void reboot()
+void suspend()
+void hibernate()
+void hybridSleep()
+void switchToVt(int vtnr)
+void connected()
+void disconnected()
+void capabilities(uint32_t capabilities)
+void userLoggedIn(QString username, QString session)
+void authenticationFailed(uint32_t error)
-wl_global* m_global
}
class GreeterProxy {
+GreeterProxy(QObject* parent)
+void connectDDM(DDMInterfaceV2* interface)
+void setShowShutdownView(bool show)
+void setShowAnimation(bool show)
+void setHasActiveSession(bool hasSession)
+void powerOff()
+void reboot()
+void suspend()
+void hibernate()
+void hybridSleep()
+void login(QString user, QString password, int sessionIndex)
+void lock()
+void unlock(QString user, QString password)
+void logout()
+void onSessionNew(QString id, QDBusObjectPath path)
+void onSessionRemoved(QString id, QDBusObjectPath path)
+void onSessionLock()
+void onSessionUnlock()
-void capabilities(uint32_t capabilities)
-void userLoggedIn(QString username, QString session)
-void authenticationFailed(uint32_t error)
-void setLock(bool isLocked)
-DDMInterfaceV2* m_ddmInterface
-LockScreen* m_lockScreen
-QString m_hostName
-bool m_canPowerOff
-bool m_canReboot
-bool m_canSuspend
-bool m_canHibernate
-bool m_canHybridSleep
}
class Helper {
+static Helper* instance()
+void init(Treeland_Treeland* treeland)
+UserModel* userModel()
+SessionModel* sessionModel()
+DDMInterfaceV2* ddmInterfaceV2() const
+void showLockScreen(bool animation)
+void activateSession()
+void deactivateSession()
+void enableRender()
+void disableRender()
+bool beforeDisposeEvent(WSeat* seat, QWindow* window, QInputEvent* event)
-GreeterProxy* m_greeterProxy
-DDMInterfaceV2* m_ddmInterfaceV2
-SessionManager* m_sessionManager
-UserModel* m_userModel
-SessionModel* m_sessionModel
}
class Session {
+~Session()
+QString id() const
+uid_t uid() const
+QString username() const
+WSocket* socket() const
-QString m_id
-uid_t m_uid
-QString m_username
-WSocket* m_socket
}
class SessionManager {
+bool activeSocketEnabled() const
+void setActiveSocketEnabled(bool newEnabled)
+void updateActiveUserSession(QString username, QString id)
+void removeSession(shared_ptr_Session session)
+shared_ptr_Session sessionForId(QString id) const
+shared_ptr_Session sessionForUid(uid_t uid) const
+shared_ptr_Session sessionForUser(QString username) const
+shared_ptr_Session sessionForXWayland(WXWayland* xwayland) const
+shared_ptr_Session sessionForSocket(WSocket* socket) const
+shared_ptr_Session activeSession() const
-shared_ptr_Session ensureSession(QString id, QString username)
-weak_ptr_Session m_activeSession
-QList~shared_ptr_Session~ m_sessions
}
class UserModel {
+QString currentUserName() const
+void setCurrentUserName(QString username)
+QVariant get(QString username) const
+User* getUser(QString username) const
+void updateUserLoginState(QString username, bool loggedIn)
+void userLoggedIn(QString username, QString session)
}
class SessionModel {
+QModelIndex index(int row, int column) const
+QVariant data(QModelIndex index, int role) const
<<enum>> TypeRole
<<enum>> FileRole
}
DDMInterfaceV2 --|> QObject
DDMInterfaceV2 --|> WServerInterface
Helper o--> GreeterProxy
Helper o--> DDMInterfaceV2
Helper o--> SessionManager
Helper o--> UserModel
Helper o--> SessionModel
GreeterProxy ..> DDMInterfaceV2 : uses
GreeterProxy ..> UserModel : uses
GreeterProxy ..> SessionModel : uses
GreeterProxy ..> SessionManager : via Helper
SessionManager o--> Session
UserModel ..> SessionManager : coordinates active session IDs
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
Session,QString m_id = 0;is a bit odd for a QString default value; consider default-constructing (QString m_id{}) to avoid relying on implicit conversions from integer literals. - In
DDMInterfaceV2::destroy,wl_resource_destroy(static_cast<struct wl_resource *>(m_handle));should guard againstm_handlebeing null, especially sincehandleResourceDestroysets the handle to nullptr when the client disconnects. ddminterfacev2.his missing include guards or#pragma once; adding one will prevent multiple-inclusion issues as this interface gets reused.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `Session`, `QString m_id = 0;` is a bit odd for a QString default value; consider default-constructing (`QString m_id{}`) to avoid relying on implicit conversions from integer literals.
- In `DDMInterfaceV2::destroy`, `wl_resource_destroy(static_cast<struct wl_resource *>(m_handle));` should guard against `m_handle` being null, especially since `handleResourceDestroy` sets the handle to nullptr when the client disconnects.
- `ddminterfacev2.h` is missing include guards or `#pragma once`; adding one will prevent multiple-inclusion issues as this interface gets reused.
## Individual Comments
### Comment 1
<location path="src/session/session.h" line_range="37" />
<code_context>
friend class SessionManager;
- int m_id = 0;
+ QString m_id = 0;
uid_t m_uid = 0;
QString m_username = {};
</code_context>
<issue_to_address>
**issue (bug_risk):** Initializing QString with literal 0 is unsafe; use default construction instead.
`QString m_id = 0;` constructs from a null pointer / integer, which is undefined and can lead to crashes or warnings. Use default initialization instead, e.g. `QString m_id{};` or `QString m_id;` to get an empty string.
</issue_to_address>
### Comment 2
<location path="src/modules/ddm/ddminterfacev2.cpp" line_range="180-183" />
<code_context>
+ handleBindingGlobal);
+}
+
+void DDMInterfaceV2::destroy([[maybe_unused]] WServer *server)
+{
+ wl_resource_destroy(static_cast<struct wl_resource *>(m_handle));
+ wl_global_destroy(m_global);
+ m_global = nullptr;
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Destroying m_handle unconditionally can double-destroy the wl_resource.
`handleResourceDestroy` already calls `setHandle(nullptr)` after the Wayland resource is destroyed. If `destroy()` is called afterwards, `wl_resource_destroy(static_cast<struct wl_resource *>(m_handle));` may receive a stale or null handle, which is undefined behavior. Guard the destroy with a null check and clear `m_handle` here as well, e.g.:
```cpp
void DDMInterfaceV2::destroy([[maybe_unused]] WServer *server)
{
if (m_handle) {
wl_resource_destroy(static_cast<struct wl_resource *>(m_handle));
m_handle = nullptr;
}
if (m_global) {
wl_global_destroy(m_global);
m_global = nullptr;
}
}
```
</issue_to_address>
### Comment 3
<location path="src/modules/ddm/ddminterfacev2.h" line_range="1-10" />
<code_context>
+// Copyright (C) 2026 UnionTech Software Technology Co., Ltd.
+// SPDX-License-Identifier: Apache-2.0 OR LGPL-3.0-only OR GPL-2.0-only OR GPL-3.0-only
+
</code_context>
<issue_to_address>
**nitpick (bug_risk):** Header is missing include guards or pragma once.
Please add an include guard or `#pragma once` to avoid multiple-definition or ODR issues when this header is included from multiple translation units.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates Treeland’s display manager (DDM) integration from the legacy socket-based / treeland-ddm v1 interface to the treeland-ddm v2 Wayland protocol, and updates session handling to use string session IDs consistent with logind.
（中文）本 PR 将 Treeland 的显示管理器（DDM）集成从旧的基于本地 socket 的 treeland-ddm v1 迁移到 treeland-ddm v2 Wayland 协议，并将会话 ID 统一升级为与 logind 一致的字符串类型。
Changes:
- Replace DDM v1 server wrapper with a new
DDMInterfaceV2Wayland server-side wrapper and wire it intoHelper/GreeterProxy. - Refactor
GreeterProxyto issue DDM actions via the v2 Wayland interface (with local-auth fallback when DDM is unavailable). - Change session IDs from
inttoQStringacrossSession/SessionManagerand greeter-related signals.
（中文）变更点：
- 用新的
DDMInterfaceV2Wayland 服务端封装替换 DDM v1，并接入Helper/GreeterProxy。 - 重构
GreeterProxy，通过 v2 Wayland 接口调用 DDM（DDM 不可用时保留本地认证兜底）。 - 将
Session/SessionManager及 greeter 相关信号中的 session id 从int改为QString。
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/session/session.h |
Switch Session/SessionManager APIs to QString session IDs. |
src/session/session.cpp |
Update implementations for QString session IDs and related lookups. |
src/seat/helper.h |
Replace v1 DDM interface member/accessor with v2. |
src/seat/helper.cpp |
Attach DDMInterfaceV2 and route VT switching through v2 when available. |
src/modules/ddm/ddminterfacev2.h |
Add new v2 DDM Wayland server wrapper API. |
src/modules/ddm/ddminterfacev2.cpp |
Implement v2 protocol binding, requests, and event wrappers. |
src/modules/ddm/ddminterfacev1.h |
Remove legacy v1 interface wrapper. |
src/modules/ddm/ddminterfacev1.cpp |
Remove legacy v1 interface wrapper implementation. |
src/modules/ddm/CMakeLists.txt |
Generate/build against treeland-ddm-v2.xml instead of v1. |
src/greeter/usermodel.h |
Update userLoggedIn signal to carry string session ID. |
src/greeter/greeterproxy.h |
Replace socket-based DDM plumbing with v2 interface hooks/signals. |
src/greeter/greeterproxy.cpp |
Refactor greeter operations (login/lock/logout/etc.) to use DDMInterfaceV2. |
|
TAG Bot New tag: 0.8.4 |
In this upgrade we rewrite all treeland-ddm interactions, includes those originally in SocketServer / GreeterProxy, into wayland communication. There's only one true treeland-ddm interaction which should on behalf of the Wayland protocol.
With the upgrade we did in previous commit, communication with ddm using QLocalSocket is not needed now, and we shall reform GreeterProxy, the communication method with our display manager the DDM, to use the newly formed Wayland protocol.
Although the logind session id is exactly an integer in current systemd-logind, but this is an UB according to systemd documentation. Systemd advertise us to treat it as a string and we shall adapt it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this upgrade we rewrite all treeland-ddm interactions, includes those originally in SocketServer / GreeterProxy, into wayland communication. There's only one true treeland-ddm interaction which should on behalf of the Wayland protocol.
Summary by Sourcery
Migrate display manager integration to the new treeland-ddm v2 Wayland protocol and remove the legacy socket-based/v1 interface.
New Features:
Enhancements:
Build: