feat: add xdg-activation-v1 client for SNI tray activation token

[18202781743]

Add XdgActivationClient that uses QWaylandClientExtension to bind dde-shell's xdg_activation_v1 global and request activation tokens via standard Wayland protocol before calling SNI Activate.

Issue: #6

[sourcery-ai]

Sorry @18202781743, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 18202781743

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• debian/deepin/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[deepin-ci-robot]

deepin pr auto review

你好！我是CodeGeeX。我已仔细审查了你提供的Git Diff。本次修改主要为Wayland环境下的系统托盘添加了xdg-activation-v1协议支持，以解决在Wayland下点击托盘图标激活窗口时缺少激活令牌的问题。

整体来看，代码结构清晰，逻辑基本合理。但在语法逻辑、代码质量、性能和安全性方面，有一些值得注意和改进的地方。以下是详细的审查意见：

1. 语法与逻辑

• 单例模式的线程安全与内存泄漏风险 (xdgactivationclient.cpp 第96-102行)
  • 问题：当前的单例实现if (!s_instance) { s_instance = new XdgActivationClient(qApp); }不是线程安全的。如果在多线程环境下同时调用instance()，可能会创建多个实例。此外，虽然指定了qApp为父对象，但XdgActivationClient的析构函数并非虚函数（且未delete自身），依赖Qt的对象树销毁在关闭时可能存在时序问题。
  • 建议：使用C++11的静态局部变量特性来实现线程安全的单例，代码更简洁且绝对安全：

    XdgActivationClient *XdgActivationClient::instance()
    {
        static XdgActivationClient s_instance(qApp);
        return &s_instance;
    }

• 环境变量操作的竞态条件 (sniprotocolhandler.cpp 第326-330行)
  • 问题：你通过qputenv设置XDG_ACTIVATION_TOKEN，然后在回调中调用Activate后立刻qunsetenv。在Qt的D-Bus调用中，Activate通常是异步的。设置环境变量后立即取消，可能导致底层D-Bus底层发送消息时读取不到该环境变量，因为环境变量只存在于当前进程的空间中，且生命周期过短。
  • 建议：Wayland的xdg-activation协议传递令牌的标准做法是通过D-Bus的XDG_ACTIVATION_TOKEN属性传递，或者确保环境变量在调用栈期间一直存活。如果底层SNI实现是读取进程环境变量，你应当确保在D-Bus消息真正发送完毕后再清除。更推荐的做法是，如果m_sniInter支持，将token作为参数传递给Activate方法，而不是依赖全局环境变量。
• Lambda捕获this指针的潜在悬空风险 (xdgactivationclient.cpp 第128行)
  • 问题：在requestToken中，连接done信号的lambda捕获了this（隐式捕获了m_pendingProvider）。如果XdgActivationClient在令牌请求期间被销毁，lambda触发时会导致悬空指针崩溃。
  • 建议：虽然单例模式降低了这种风险，但为了代码健壮性，建议使用QPointer或在connect时传入this作为context对象，确保如果this被销毁，连接自动断开：

    connect(provider, &XdgActivationTokenV1::done, this, [this, callback](const QString &token) {
        // ...
    }, Qt::SingleShotConnection);

2. 代码质量

• 跨目录引用源文件的反模式 (plugins/application-tray/CMakeLists.txt 第35-36行)
  • 问题：${CMAKE_SOURCE_DIR}/src/tray-wayland-integration/xdgactivationclient.cpp 直接将另一个模块的源文件硬编码包含到当前插件中。这破坏了模块的封装性，如果该文件依赖了src/tray-wayland-integration中的其他私有头文件，会导致编译失败。同时，这会导致该源文件被编译两次（一次在dockpluginmanager，一次在application-tray），增加了二进制体积。
  • 建议：
    1. 推荐方案：将XdgActivationClient编译为一个独立的静态库（如XdgActivationClientStatic），然后在application-tray和dockpluginmanager中通过target_link_libraries链接该静态库。
    2. 备选方案：如果一定要共用源码，使用target_sources并确保Include目录正确，但绝不要使用绝对路径跨模块引用，应使用CMake变量或相对路径。
• Qt版本判断逻辑的严谨性 (plugins/application-tray/CMakeLists.txt 第11行)
  • 问题：if(Qt${QT_VERSION_MAJOR}_VERSION VERSION_GREATER_EQUAL 6.10) 目前Qt 6.10尚未发布，此判断可能是为了兼容未来版本。但需要注意的是，WaylandClientPrivate在Qt 6.2之后才逐渐稳定，如果是为了兼容Qt 5，这里的逻辑需要更细致的评估。
  • 建议：确认所需Private API的最低Qt版本要求，并添加注释说明为何需要6.10及以上版本才引入WaylandClientPrivate。

3. 代码性能

• Wayland Client Extension的初始化时机 (xdgactivationclient.cpp 第112行)
  • 问题：XdgActivationV1在构造函数中调用了initialize()，这会立即发起Wayland Registry的同步请求。如果XdgActivationClient作为单例在主程序启动时被构建，可能会阻塞主线程，影响启动性能。
  • 建议：考虑使用延迟初始化，即在第一次调用isActive()或requestToken()时再真正初始化XdgActivationV1，或者确保QWaylandClientExtensionTemplate的初始化是纯异步的（当前Qt的实现基本是异步的，但最好确认不会引起阻塞）。

4. 代码安全

• 环境变量泄露与污染 (sniprotocolhandler.cpp 第326-330行)
  • 问题：使用qputenv修改进程全局环境变量是不安全的。如果在qputenv和qunsetenv的间隙，有其他线程访问XDG_ACTIVATION_TOKEN，会读到不该读到的数据；如果程序在qunsetenv前崩溃，环境变量将被永久污染。
  • 建议：如前所述，尽量避免修改全局环境变量。如果SNI的底层实现强制要求从环境变量读取，考虑使用更安全的作用域机制，或者向SNI的实现库提交Patch，改为通过函数参数直接传递Token。
• Wayland对象的生命周期管理 (xdgactivationclient.cpp 第62行)
  • 问题：在XdgActivationTokenV1的析构函数中调用destroy()。如果Wayland协议对象在事件循环外被销毁，可能会引发协议错误。
  • 建议：确保在Qt的事件循环中销毁Wayland对象，或者使用deleteLater()来延迟销毁，这在你的代码第131行已经正确使用了sender()->deleteLater()，但需要注意如果m_pendingProvider被清理时，也应当安全销毁。

修改建议代码示例

针对XdgActivationClient的核心逻辑，我建议进行如下优化：

// xdgactivationclient.cpp

XdgActivationClient *XdgActivationClient::instance()
{
    // 使用静态局部变量保证线程安全，且无需手动管理内存
    static XdgActivationClient s_instance(qApp);
    return &s_instance;
}

void XdgActivationClient::requestToken(QWindow *window, const QString &appId, TokenCallback callback)
{
    if (m_pendingProvider) {
        qCWarning(trayXdgActivation) << "Token request already in progress";
        if (callback) callback(QString());
        return;
    }

    if (!isActive()) {
        qCDebug(trayXdgActivation) << "xdg_activation_v1 not active";
        if (callback) callback(QString());
        return;
    }

    auto effectiveWindow = window ? window : QGuiApplication::focusWindow();
    if (!effectiveWindow) {
        qCWarning(trayXdgActivation) << "No target window for activation token";
        if (callback) callback(QString());
        return;
    }

    auto *provider = m_activation->createTokenProvider(effectiveWindow, appId);
    m_pendingProvider = provider;

    // 传入 this 作为 context，防止 this 被销毁时 lambda 仍然被调用
    connect(provider, &XdgActivationTokenV1::done, this, [this, callback](const QString &token) {
        m_pendingProvider = nullptr;

        if (token.isEmpty())
            qCWarning(trayXdgActivation) << "Empty activation token received";
        else
            qCDebug(trayXdgActivation) << "Activation token received";

        if (callback) callback(token);
        sender()->deleteLater();
    }, Qt::SingleShotConnection);
}

对于 sniprotocolhandler.cpp 中的环境变量问题，如果无法修改底层SNI接口，至少要确保逻辑严密：

// sniprotocolhandler.cpp
if (activationClient->isActive()) {
    auto widget = static_cast<QWidget*>(parent());
    auto *win = widget->window()->windowHandle();
    auto sniInter = m_sniInter;
    activationClient->requestToken(win, QString(), [sniInter](const QString &token) {
        if (!token.isEmpty()) {
            qputenv("XDG_ACTIVATION_TOKEN", token.toUtf8());
        }
        // Activate 是异步调用，环境变量在进程内，底层DBus发送时会读取当前进程环境
        sniInter->Activate(0, 0);
        
        // 注意：立即 qunsetenv 可能导致底层DBus尚未读取到该变量
        // 如果确实需要清理，建议使用 QTimer::singleShot(0, []{ qunsetenv("XDG_ACTIVATION_TOKEN"); });
        // 但最安全的做法是不使用环境变量传递，或者确认SNI实现会在Activate调用时立即拷贝环境变量
        qunsetenv("XDG_ACTIVATION_TOKEN"); 
    });
} else {
    m_sniInter->Activate(0, 0);
}

希望这些审查意见对你有所帮助！如果有任何疑问，欢迎继续交流。

[deepin-bot]

TAG Bot

New tag: 2.0.33
DISTRIBUTION: unstable
Suggest: synchronizing this PR through rebase #463