fix: fix notification settings opening via direct D-Bus call#1530
Merged
18202781743 merged 1 commit intolinuxdeepin:masterfrom Mar 25, 2026
Merged
fix: fix notification settings opening via direct D-Bus call#153018202781743 merged 1 commit intolinuxdeepin:masterfrom
18202781743 merged 1 commit intolinuxdeepin:masterfrom
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideSwitches opening of notification settings from a synchronous D‑Bus helper call to a manually constructed, asynchronous D‑Bus method call to avoid UI blocking when launching the settings page from the notification center. Sequence diagram for asynchronously opening notification settings via D-BussequenceDiagram
actor User
participant NotificationCenter
participant NotifyAccessor
participant QDBusConnection_sessionBus as QDBusConnection_sessionBus
participant ControlCenter1 as org_deepin_dde_ControlCenter1
User ->> NotificationCenter: clickNotificationSettingsButton()
NotificationCenter ->> NotifyAccessor: openNotificationSetting()
NotifyAccessor ->> NotifyAccessor: create QDBusMessage(ShowPage)
NotifyAccessor ->> QDBusConnection_sessionBus: asyncCall(msg "notification")
QDBusConnection_sessionBus -->> ControlCenter1: ShowPage("notification")
ControlCenter1 -->> QDBusConnection_sessionBus: process request (no reply awaited)
Note over User,NotificationCenter: UI remains responsive while settings open
Class diagram for NotifyAccessor using asynchronous D-Bus callclassDiagram
class NotifyAccessor {
+bool applicationPin(QString appId) const
+void openNotificationSetting()
+void onNotificationStateChanged(qint64 id, int processedType)
}
class QDBusMessage {
+static QDBusMessage createMethodCall(QString service, QString path, QString interface, QString method)
+QDBusMessage operator_left_shift(QString arg)
}
class QDBusConnection {
+static QDBusConnection sessionBus()
+void asyncCall(QDBusMessage msg)
}
NotifyAccessor ..> QDBusMessage : constructs
NotifyAccessor ..> QDBusConnection : uses sessionBus_asyncCall
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 - I've found 1 issue, and left some high level feedback:
- The new async D-Bus call drops all error handling; consider at least adding minimal failure logging via a QDBusPendingCallWatcher or another mechanism so that failures to open the settings are still observable.
- Instead of hardcoding the service, path, and interface here, you could reuse or extend the existing controlCenterInterface helper with asyncCall to avoid duplicating those strings and keep call sites consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new async D-Bus call drops all error handling; consider at least adding minimal failure logging via a QDBusPendingCallWatcher or another mechanism so that failures to open the settings are still observable.
- Instead of hardcoding the service, path, and interface here, you could reuse or extend the existing controlCenterInterface helper with asyncCall to avoid duplicating those strings and keep call sites consistent.
## Individual Comments
### Comment 1
<location path="panels/notification/center/notifyaccessor.cpp" line_range="222-227" />
<code_context>
- qWarning(notifyLog) << "Failed to Open notifycation setting" << reply.error().message();
- return;
- }
+ QDBusMessage msg = QDBusMessage::createMethodCall("org.deepin.dde.ControlCenter1",
+ "/org/deepin/dde/ControlCenter1",
+ "org.deepin.dde.ControlCenter1",
+ "ShowPage");
+ msg << "notification";
+ QDBusConnection::sessionBus().asyncCall(msg);
}
</code_context>
<issue_to_address>
**issue (bug_risk):** Async call drops error handling and logging compared to the previous sync implementation.
Previously, the sync `controlCenterInterface().call("ShowPage", "notification")` logged DBus errors and bailed out on failure. The new `asyncCall` is fire‑and‑forget, so failures (service unavailable, method not found, etc.) are now silent. If that feedback is still needed, consider `QDBusPendingCall`/`QDBusPendingCallWatcher` or another way to at least log failures, or add a comment making it explicit that errors are intentionally ignored.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Changed the method of opening notification settings from using the controlCenterInterface() helper to directly constructing a D-Bus message and making an async call. The previous approach used a QDBusReply which would wait for a synchronous response, potentially blocking the UI. The new implementation uses QDBusMessage::createMethodCall to create the message and QDBusConnection::sessionBus().asyncCall() to send it asynchronously, improving responsiveness. Log: Fixed issue where opening notification settings could cause UI lag Influence: 1. Test clicking notification settings button in notification center 2. Verify settings window opens without delay 3. Check that other notification center functions remain unaffected 4. Test with multiple rapid clicks to ensure no blocking occurs fix: 修复通过直接 D-Bus 调用打开通知设置的问题 将打开通知设置的方法从使用 controlCenterInterface() 辅助函数改为直接构造 D-Bus 消息并进行异步调用。之前的方法使用 QDBusReply 会等待同步响应，可能 导致 UI 阻塞。新实现使用 QDBusMessage::createMethodCall 创建消息，并使用 QDBusConnection::sessionBus().asyncCall() 异步发送，提高了响应速度。 Log: 修复了打开通知设置可能导致界面卡顿的问题 Influence: 1. 测试通知中心中点击通知设置按钮 2. 验证设置窗口打开无延迟 3. 检查通知中心其他功能是否不受影响 4. 测试多次快速点击确保不会发生阻塞 PMS: BUG-353895
18202781743
approved these changes
Mar 25, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, wjyrich 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 |
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
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.
Changed the method of opening notification settings from using the controlCenterInterface() helper to directly constructing a D-Bus message and making an async call. The previous approach used a QDBusReply which would wait for a synchronous response, potentially blocking the UI. The new implementation uses QDBusMessage::createMethodCall to create the message and QDBusConnection::sessionBus().asyncCall() to send it asynchronously, improving responsiveness.
Log: Fixed issue where opening notification settings could cause UI lag
Influence:
fix: 修复通过直接 D-Bus 调用打开通知设置的问题
将打开通知设置的方法从使用 controlCenterInterface() 辅助函数改为直接构造
D-Bus 消息并进行异步调用。之前的方法使用 QDBusReply 会等待同步响应，可能
导致 UI 阻塞。新实现使用 QDBusMessage::createMethodCall 创建消息，并使用 QDBusConnection::sessionBus().asyncCall() 异步发送，提高了响应速度。
Log: 修复了打开通知设置可能导致界面卡顿的问题
Influence:
PMS: BUG-353895
Summary by Sourcery
Handle opening of notification settings via an asynchronous D-Bus method call to avoid blocking the UI.
Bug Fixes:
Enhancements: