feat(notification): add ActivationToken support for wayland#1627
Open
18202781743 wants to merge 1 commit into
Open
feat(notification): add ActivationToken support for wayland#162718202781743 wants to merge 1 commit into
18202781743 wants to merge 1 commit into
Conversation
Reviewer's GuideAdds Wayland XDG activation token support to the notification service by emitting a new ActivationToken D-Bus signal for non-extended actions and propagating the token to dde-am via XDG_ACTIVATION_TOKEN for extended actions, using an asynchronous token retrieval mechanism. Sequence diagram for Wayland activation token handling in notification actionssequenceDiagram
actor User
participant NotificationManager
participant XdgActivation
participant DbusAdaptor
participant DdeAm
rect rgb(230,230,255)
User ->> NotificationManager: actionInvoked(id, bubbleId, actionKey)
NotificationManager ->> XdgActivation: new XdgActivation
NotificationManager ->> XdgActivation: requestToken()
XdgActivation -->> NotificationManager: tokenReady(token)
alt [token not empty]
NotificationManager ->> DbusAdaptor: ActivationToken(bubbleId, token)
end
NotificationManager ->> DbusAdaptor: ActionInvoked(bubbleId, actionKey)
NotificationManager ->> DbusAdaptor: NotificationClosed(bubbleId, Closed)
end
rect rgb(230,255,230)
User ->> NotificationManager: doActionInvoked(entity, actionId, actionKey)
NotificationManager ->> XdgActivation: new XdgActivation
NotificationManager ->> XdgActivation: requestToken()
XdgActivation -->> NotificationManager: tokenReady(token)
NotificationManager ->> DdeAm: startDetached("dde-am", amArgs)
opt [token not empty]
NotificationManager ->> DdeAm: set env XDG_ACTIVATION_TOKEN
end
end
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:
- Both in
actionInvokedanddoActionInvokedthe asynchronousXdgActivationflow can indefinitely delay emittingActionInvoked/NotificationClosedor startingdde-amiftokenReadyis never emitted; consider adding an error/timeout path or a fallback that proceeds without a token. - You unconditionally create a new
XdgActivationinstance for every action, which may be unnecessary on non-Wayland sessions and could add overhead; consider guarding this logic with a Wayland/session check or reusing a shared helper instead of allocating per-action. - The
XdgActivationsetup and cleanup code is duplicated in two places; consider extracting a small helper (e.g.,requestActivationToken(callback)) to centralize token acquisition and ensure consistent behavior and resource management.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Both in `actionInvoked` and `doActionInvoked` the asynchronous `XdgActivation` flow can indefinitely delay emitting `ActionInvoked`/`NotificationClosed` or starting `dde-am` if `tokenReady` is never emitted; consider adding an error/timeout path or a fallback that proceeds without a token.
- You unconditionally create a new `XdgActivation` instance for every action, which may be unnecessary on non-Wayland sessions and could add overhead; consider guarding this logic with a Wayland/session check or reusing a shared helper instead of allocating per-action.
- The `XdgActivation` setup and cleanup code is duplicated in two places; consider extracting a small helper (e.g., `requestActivationToken(callback)`) to centralize token acquisition and ensure consistent behavior and resource management.
## Individual Comments
### Comment 1
<location path="panels/notification/server/notificationmanager.cpp" line_range="139" />
<code_context>
- Q_EMIT ActionInvoked(bubbleId, actionKey);
- Q_EMIT NotificationClosed(bubbleId, NotifyEntity::Closed);
+ // For non-extended actions, emit ActivationToken first if available
+ auto *activation = new DS_NAMESPACE::XdgActivation(this);
+ connect(activation, &DS_NAMESPACE::XdgActivation::tokenReady, this,
+ [this, bubbleId, actionKey, activation](const QString &token) {
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the repeated XdgActivation setup into a reusable helper that takes a handler functor to simplify control flow and avoid duplication in both call sites.
The `XdgActivation` usage is duplicated and adds nested lambdas and lifetime handling in both call sites. Extracting a small helper that owns `XdgActivation` and just calls a functor when the token is ready will keep both sites linear and remove the extra `amArgsCopy`.
Example helper (private method in `NotificationManager`):
```c++
class NotificationManager : public QObject
{
Q_OBJECT
// ...
private:
template <typename F>
void withActivationToken(F &&handler)
{
auto *activation = new DS_NAMESPACE::XdgActivation(this);
connect(activation, &DS_NAMESPACE::XdgActivation::tokenReady, this,
[activation, handler = std::forward<F>(handler)](const QString &token) mutable {
handler(token);
activation->deleteLater();
},
Qt::SingleShotConnection);
activation->requestToken();
}
};
```
Then `actionInvoked` becomes simpler and closer to the original structure:
```c++
void NotificationManager::actionInvoked(qint64 id, uint bubbleId, const QString &actionKey)
{
qInfo(notifyLog) << "Action invoked, bubbleId:" << bubbleId << ", id:" << id << ", actionKey" << actionKey;
actionInvoked(id, actionKey);
withActivationToken([this, bubbleId, actionKey](const QString &token) {
if (!token.isEmpty()) {
Q_EMIT ActivationToken(bubbleId, token);
qDebug(notifyLog) << "Emitted ActivationToken for non-extended action:" << token;
}
Q_EMIT ActionInvoked(bubbleId, actionKey);
Q_EMIT NotificationClosed(bubbleId, NotifyEntity::Closed);
});
}
```
And the extended action (`dde-am` launch) stays linear and avoids `amArgsCopy`:
```c++
if (!args.isEmpty()) {
QString cmd = args.takeFirst();
QStringList amArgs;
amArgs << "-c" << cmd;
if (!args.isEmpty()) {
amArgs << "--" << args;
}
withActivationToken([amArgs = std::move(amArgs)](const QString &token) {
QProcess pro;
pro.setProgram("dde-am");
pro.setArguments(amArgs);
QProcessEnvironment proEnv = QProcessEnvironment::systemEnvironment();
proEnv.remove("DSG_APP_ID");
if (!token.isEmpty()) {
proEnv.insert("XDG_ACTIVATION_TOKEN", token);
qDebug(notifyLog) << "Set XDG_ACTIVATION_TOKEN for extended action:" << token;
}
pro.setProcessEnvironment(proEnv);
pro.startDetached();
});
}
```
This keeps all behavior (including token emission, `ActivationToken` signal, and process environment setup) while reducing duplication, nesting, and manual `new`/`deleteLater` in the main logic.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
18202781743
force-pushed
the
agent/agent/975b667c
branch
from
6056b09 to
65c61fe
Compare
18202781743
force-pushed
the
agent/agent/975b667c
branch
from
65c61fe to
469d53b
Compare
deepin pr auto review我来对这段代码进行审查，并提出改进意见：
a) 代码质量：
b) 代码性能：
c) 代码安全：
d) 代码逻辑：
// 建议在 NotificationManager 类中添加 XdgActivation 对象池
QVector<XdgActivation*> m_activationPool;
// 优化后的 isExtended 实现
bool NotificationManager::isExtendedAction(qint64 id, const QString &actionId) const
{
static QCache<qint64, QMap<QString, QVariant>> entityHintCache(100);
auto *hints = entityHintCache.object(id);
if (!hints) {
auto entity = m_persistence->fetchEntity(id);
if (!entity.isValid()) {
return false;
}
hints = new QMap<QString, QVariant>(entity.hints());
entityHintCache.insert(id, hints);
}
return hints->contains("x-deepin-action-" + actionId);
}
// 抽取的 token 处理函数
void NotificationManager::handleActivationToken(uint bubbleId, const QString &actionKey,
std::function<void(const QString &)> callback)
{
auto *activation = m_activationPool.isEmpty() ?
new DS_NAMESPACE::XdgActivation(this) :
m_activationPool.takeFirst();
connect(activation, &DS_NAMESPACE::XdgActivation::tokenReady, this,
[this, activation, callback](const QString &token) {
if (!token.isEmpty() && token.length() <= 256) { // 添加长度限制
callback(token);
}
m_activationPool.append(activation);
}, Qt::SingleShotConnection);
activation->requestToken();
}
总体来说，这次更新增加了对 Wayland 激活令牌的支持，改进了通知系统的功能完整性。代码结构清晰，但仍有性能优化和安全性加强的空间。 |
1. Implement ActivationToken signal in notification DBus adapters 2. Add XdgActivation token request for non-extended actions 3. Integrate activation token into dde-am execution for extended actions 4. Add isExtendedAction helper to check for deepin-specific actions Log: Notifications now properly set XDG_ACTIVATION_TOKEN when launching actions Influence: 1. Verify notification action buttons trigger ActivationToken signal 2. Test extended actions (x-deepin-action-*) set XDG_ACTIVATION_TOKEN in environment 3. Confirm non-extended actions emit ActivationToken before ActionInvoked 4. Check token is passed to dde-am process for app launching 5. Verify backward compatibility with existing notification flows 6. Test multiple rapid notifications to ensure proper token handling feat: 通知支持 XdgActivation 协议 1. 在通知DBus适配器中实现ActivationToken信号 2. 为非扩展操作添加XdgActivation令牌请求 3. 将激活令牌集成到扩展操作的dde-am执行中 4. 添加isExtendedAction辅助方法检查深度特定操作 Log: 通知现在在启动操作时正确设置 XDG_ACTIVATION_TOKEN Influence: 1. 验证通知操作按钮触发ActivationToken信号 2. 测试扩展操作（x-deepin-action-*）在环境中设置XDG_ACTIVATION_TOKEN 3. 确认非扩展操作在ActionInvoked之前发出ActivationToken 4. 检查令牌是否正确传递给dde-am进程用于应用启动 5. 验证与现有通知流程的向后兼容性 6. 测试多个快速通知以确保令牌处理正确
18202781743
force-pushed
the
agent/agent/975b667c
branch
from
469d53b to
ad02d3f
Compare
BLumia
approved these changes
Jun 11, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 18202781743, BLumia 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.
Summary
为通知服务添加 wayland 激活令牌支持：
ActivationToken信号再发送ActionInvokedXDG_ACTIVATION_TOKEN传递令牌给dde-amChanges
panels/notification/server/notificationmanager.h: 添加ActivationToken信号panels/notification/server/notificationmanager.cpp: 实现异步令牌请求和传递逻辑panels/notification/server/dbusadaptor.h: 添加ActivationTokenD-Bus 信号Test Plan
XDG_ACTIVATION_TOKEN环境变量ActivationToken信号Summary by Sourcery
Add Wayland activation token support to notification actions and expose it via D-Bus.
New Features:
Enhancements: