refactor: remove PowerManager dependency and use SessionManager #378
Conversation
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
Reviewer's GuideThis PR refactors the shutdown plugin to replace the PowerManager D-Bus interface with the SessionManager interface for all power operations. The change removes the old DBusPowerManager code, updates shutdown logic to use SessionManager over the sessionBus, and introduces new XML definitions and generated proxy files for SessionManager. Sequence diagram for shutdown plugin power operation checks using SessionManagersequenceDiagram
participant ShutdownPlugin
participant SessionManager
ShutdownPlugin->>SessionManager: CanSuspend()
SessionManager-->>ShutdownPlugin: bool (can suspend)
ShutdownPlugin->>SessionManager: CanHibernate()
SessionManager-->>ShutdownPlugin: bool (can hibernate)
Entity relationship diagram for SessionManager D-Bus interface methods and propertieserDiagram
SESSIONMANAGER {
bool CanSuspend
bool CanHibernate
bool CanLogout
bool CanReboot
bool CanShutdown
void ForceLogout
void ForceReboot
void ForceShutdown
void Logout
void PowerOffChoose
void Reboot
bool Register
void RequestHibernate
void RequestLock
void RequestLogout
void RequestReboot
void RequestShutdown
void RequestSuspend
void SetLocked
void Shutdown
void ToggleDebug
bool Locked
string CurrentUid
int Stage
object CurrentSessionPath
}
%% All methods and properties are part of the SessionManager entity
Class diagram for replacement of PowerManager with SessionManagerclassDiagram
class ShutdownPlugin {
- bool m_pluginLoaded
- QScopedPointer<CommonIconButton> m_dockIcon
- QScopedPointer<Dock::TipsWidget> m_tipsLabel
- SessionManager* m_sessionManagerInter
- QSharedPointer<DConfig> m_dconfig
- DConfig *m_lastoreDConfig
+ itemContextMenu(itemKey: QString): QString
}
class SessionManager {
+ AllowSessionDaemonRun(): QDBusPendingReply<bool>
+ CanHibernate(): QDBusPendingReply<bool>
+ CanLogout(): QDBusPendingReply<bool>
+ CanReboot(): QDBusPendingReply<bool>
+ CanShutdown(): QDBusPendingReply<bool>
+ CanSuspend(): QDBusPendingReply<bool>
+ ForceLogout(): QDBusPendingReply<>
+ ForceReboot(): QDBusPendingReply<>
+ ForceShutdown(): QDBusPendingReply<>
+ Logout(): QDBusPendingReply<>
+ PowerOffChoose(): QDBusPendingReply<>
+ Reboot(): QDBusPendingReply<>
+ Register(in0: QString): QDBusPendingReply<bool>
+ RequestHibernate(): QDBusPendingReply<>
+ RequestLock(): QDBusPendingReply<>
+ RequestLogout(): QDBusPendingReply<>
+ RequestReboot(): QDBusPendingReply<>
+ RequestShutdown(): QDBusPendingReply<>
+ RequestSuspend(): QDBusPendingReply<>
+ SetLocked(in0: bool): QDBusPendingReply<>
+ Shutdown(): QDBusPendingReply<>
+ ToggleDebug(): QDBusPendingReply<>
+ ~SessionManager()
+ currentSessionPath: QDBusObjectPath
+ currentUid: QString
+ locked: bool
+ stage: int
}
ShutdownPlugin --> SessionManager : uses
%% DBusPowerManager class removed
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The new SessionManager::CanSuspend/CanHibernate methods return QDBusPendingReply, so you should synchronously wait for the reply or switch to a QDBusReply to extract the actual boolean value before using it in your conditions.
- Add explicit error handling or fallback logic for when the SessionManager D-Bus interface is unavailable or returns an error, so the shutdown plugin can still gracefully handle missing session services.
- Since you switched to sessionBus for power operations, verify coverage on environments where the session bus might not expose power methods and consider reverting to systemBus or providing a fallback mechanism in those cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new SessionManager::CanSuspend/CanHibernate methods return QDBusPendingReply<bool>, so you should synchronously wait for the reply or switch to a QDBusReply to extract the actual boolean value before using it in your conditions.
- Add explicit error handling or fallback logic for when the SessionManager D-Bus interface is unavailable or returns an error, so the shutdown plugin can still gracefully handle missing session services.
- Since you switched to sessionBus for power operations, verify coverage on environments where the session bus might not expose power methods and consider reverting to systemBus or providing a fallback mechanism in those cases.
## Individual Comments
### Comment 1
<location> `plugins/dde-dock/shutdown/dbus/org_deepin_dde_sessionmanager.h:1` </location>
<code_context>
-//
-// SPDX-License-Identifier: LGPL-3.0-or-later
-
-/*
- * This file was generated by qdbusxml2cpp version 0.8
- * Command line was: qdbusxml2cpp -c PowerManager -p com_deepin_daemon_powermanager com.deepin.daemon.PowerManager.xml
</code_context>
<issue_to_address>
**issue (complexity):** Consider avoiding checking in large generated boilerplate by either hand-writing only needed D-Bus methods or generating proxies at build-time.
Here are two ways to avoid checking in that 200-line generated boilerplate while keeping the exact same D-Bus calls:
1) Use QDBusInterface (or wrap it) and only hand-write the few methods you actually call:
```cpp
// SessionManager.h
#include <QtDBus/QtDBus>
class SessionManager : QObject {
Q_OBJECT
QDBusInterface iface_;
public:
explicit SessionManager(QObject* parent = nullptr)
: QObject(parent)
, iface_("org.deepin.dde.SessionManager1",
"/org/deepin/dde/SessionManager1",
"org.deepin.dde.SessionManager1",
QDBusConnection::sessionBus(), this)
{}
// helper to reduce repetition
template<typename... Args>
auto call(const char* method, Args&&... args) {
return iface_.asyncCall(method, std::forward<Args>(args)...);
}
// only implement what you use:
QDBusPendingReply<bool> CanHibernate() { return call("CanHibernate"); }
QDBusPendingReply<bool> CanReboot() { return call("CanReboot"); }
QDBusPendingReply<> Logout() { return call("Logout"); }
// …etc.
};
```
This removes all the 30+ identical inline stubs and cuts boilerplate.
2) Or keep using qdbusxml2cpp but generate the proxy at build-time instead of checking it into git:
```cmake
add_custom_command(
OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/SessionManager.h
${CMAKE_CURRENT_BINARY_DIR}/SessionManager.cpp
COMMAND qdbusxml2cpp
-p SessionManager
${CMAKE_CURRENT_SOURCE_DIR}/org.deepin.dde.SessionManager.xml
DEPENDS org.deepin.dde.SessionManager.xml
)
target_include_directories(your_target PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
target_sources(your_target PRIVATE
${CMAKE_CURRENT_BINARY_DIR}/SessionManager.cpp
)
```
This way you get the full proxy without inflating your repo or manually editing generated code.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -0,0 +1,200 @@ | |||
| /* | |||
There was a problem hiding this comment.
issue (complexity): Consider avoiding checking in large generated boilerplate by either hand-writing only needed D-Bus methods or generating proxies at build-time.
Here are two ways to avoid checking in that 200-line generated boilerplate while keeping the exact same D-Bus calls:
- Use QDBusInterface (or wrap it) and only hand-write the few methods you actually call:
// SessionManager.h
#include <QtDBus/QtDBus>
class SessionManager : QObject {
Q_OBJECT
QDBusInterface iface_;
public:
explicit SessionManager(QObject* parent = nullptr)
: QObject(parent)
, iface_("org.deepin.dde.SessionManager1",
"/org/deepin/dde/SessionManager1",
"org.deepin.dde.SessionManager1",
QDBusConnection::sessionBus(), this)
{}
// helper to reduce repetition
template<typename... Args>
auto call(const char* method, Args&&... args) {
return iface_.asyncCall(method, std::forward<Args>(args)...);
}
// only implement what you use:
QDBusPendingReply<bool> CanHibernate() { return call("CanHibernate"); }
QDBusPendingReply<bool> CanReboot() { return call("CanReboot"); }
QDBusPendingReply<> Logout() { return call("Logout"); }
// …etc.
};This removes all the 30+ identical inline stubs and cuts boilerplate.
- Or keep using qdbusxml2cpp but generate the proxy at build-time instead of checking it into git:
add_custom_command(
OUTPUT ${CMAKE_CURRENT_BINARY_DIR}/SessionManager.h
${CMAKE_CURRENT_BINARY_DIR}/SessionManager.cpp
COMMAND qdbusxml2cpp
-p SessionManager
${CMAKE_CURRENT_SOURCE_DIR}/org.deepin.dde.SessionManager.xml
DEPENDS org.deepin.dde.SessionManager.xml
)
target_include_directories(your_target PRIVATE ${CMAKE_CURRENT_BINARY_DIR})
target_sources(your_target PRIVATE
${CMAKE_CURRENT_BINARY_DIR}/SessionManager.cpp
)This way you get the full proxy without inflating your repo or manually editing generated code.
18202781743
previously approved these changes
Oct 30, 2025
1. Removed the PowerManager D-Bus interface dependency from the shutdown plugin 2. Replaced PowerManager with SessionManager for power-related operations 3. Deleted the auto-generated DBusPowerManager files (cpp and header) 4. Added new SessionManager D-Bus interface XML definition and generated implementation files 5. Updated shutdown plugin to use SessionManager for CanSuspend() and CanHibernate() checks 6. Changed D-Bus connection from systemBus to sessionBus for SessionManager This change simplifies the dependency chain by using SessionManager which provides the same power management functionality that was previously handled by PowerManager. The SessionManager interface is more comprehensive and better integrated with the desktop session management. Log: Power management operations now use SessionManager instead of PowerManager Influence: 1. Test suspend functionality to ensure CanSuspend() works correctly 2. Test hibernate functionality to verify CanHibernate() returns correct values 3. Verify shutdown menu displays correct options based on system capabilities 4. Test power operations (suspend/hibernate) actually work when selected 5. Verify D-Bus connection to SessionManager is established properly 6. Test error handling when SessionManager is unavailable 重构：移除 PowerManager 依赖并使用 SessionManager 1. 从关机插件中移除了 PowerManager D-Bus 接口依赖 2. 使用 SessionManager 替代 PowerManager 进行电源相关操作 3. 删除了自动生成的 DBusPowerManager 文件（cpp 和头文件） 4. 添加了新的 SessionManager D-Bus 接口 XML 定义和生成的实现文件 5. 更新关机插件使用 SessionManager 进行 CanSuspend() 和 CanHibernate() 检查 6. 将 D-Bus 连接从 systemBus 改为 sessionBus 以连接 SessionManager 此更改通过使用 SessionManager 简化了依赖链，SessionManager 提供了与之前 PowerManager 处理的相同电源管理功能。SessionManager 接口更全面，能更好地 与桌面会话管理集成。 Log: 电源管理操作现在使用 SessionManager 而不是 PowerManager Influence: 1. 测试挂起功能确保 CanSuspend() 正常工作 2. 测试休眠功能验证 CanHibernate() 返回正确值 3. 验证关机菜单根据系统功能显示正确选项 4. 测试选择的电源操作（挂起/休眠）实际可用 5. 验证与 SessionManager 的 D-Bus 连接正确建立 6. 测试 SessionManager 不可用时错误处理
deepin pr auto review我来对这个diff进行代码审查：
总体来说，这次改动主要是将PowerManager接口替换为SessionManager接口，代码质量良好，但建议在安全性方面进行加强。 |
18202781743
approved these changes
Oct 30, 2025
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, fly602 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 |
Contributor
Author
|
/forcemerge |
|
This pr force merged! (status: blocked) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
This change simplifies the dependency chain by using SessionManager which provides the same power management functionality that was previously handled by PowerManager. The SessionManager interface is more comprehensive and better integrated with the desktop session management.
Log: Power management operations now use SessionManager instead of PowerManager
Influence:
重构：移除 PowerManager 依赖并使用 SessionManager
此更改通过使用 SessionManager 简化了依赖链，SessionManager 提供了与之前 PowerManager 处理的相同电源管理功能。SessionManager 接口更全面，能更好地 与桌面会话管理集成。
Log: 电源管理操作现在使用 SessionManager 而不是 PowerManager
Influence:
Summary by Sourcery
Refactor the shutdown plugin to replace the PowerManager D-Bus interface with the SessionManager interface for power operations, removing the old DBusPowerManager files and updating the plugin to use sessionBus connections.
Enhancements: