fix: disable xembed tray selection manager registeration

[BLumia]

The current FdoSelectionManager is for preparing the legacy xembed2sni solution. We currently no longer plans to go with such solution, but the current code will register _NET_SYSTEM_TRAY_S0. This patch disabled related code path so it won't gets registered. We should do registeration from dde-daemon instead.

注:此patch并不直接解决下面关联的PMS问题单,只是有相关性,是相关工作的
一部分.

PMS: BUG-360349

[sourcery-ai]

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

Please try again later or upgrade to continue using Sourcery

[deepin-ci-robot]

deepin pr auto review

你好！我是CodeGeeX，你的智能编程助手。我已经仔细审查了你提供的 Git Diff。

这次修改的主要目的是通过宏 _DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER 来条件编译 FdoSelectionManager 相关的代码，以便在特定环境下（如由 dde-daemon 处理时）关闭该功能。同时更新了版权声明。

以下是我从语法逻辑、代码质量、代码性能和代码安全四个维度提出的审查意见和改进建议：

1. 语法与逻辑

• 宏的命名与定义规范：
  • 问题：代码中使用 #define _DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER 0 来定义宏，并在后续使用 #if _DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER。在 C++ 中，以单下划线加大写字母（如 _DDE_...）开头的标识符是保留给编译器和标准库实现的。这可能会导致不可预见的宏冲突或未定义行为。
  • 建议：将宏名更改为非保留命名，例如 DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER，或者使用项目统一的前缀（如 DDE_TRAY_...）。
• 宏的定义位置与可见性：
  • 问题：该宏直接定义在头文件 xembedprotocolhandler.h 中。这意味着所有包含此头文件的代码都会受到这个硬编码宏的影响。如果未来需要根据不同的编译目标动态切换此功能，修改头文件是不够灵活的。
  • 建议：最佳实践是通过构建系统（如 CMake 的 add_definitions 或 configure_file）来传递此类特性宏，而不是在头文件中硬编码为 0。如果在当前架构下必须在头文件中定义，请确保它被放置在头文件保护符之内，并且考虑是否应该使用 #ifndef 来允许编译命令覆盖它。
  • 改进代码示例：

    // 允许编译参数优先，如果没有定义则默认为 0
    #ifndef DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER
    #define DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER 0
    #endif

• 潜在的悬空指针风险（逻辑漏洞）：
  • 问题：在头文件中，class FdoSelectionManager; 的前置声明依然存在。如果在 .cpp 中因为宏为 0 而没有包含 fdoselectionmanager.h，且未来有人在没有宏保护的情况下误用了 FdoSelectionManager 的具体类型（例如调用其方法），会导致编译失败。更严重的是，如果 m_selectionManager 成员变量（虽然 diff 中未展示，但推测存在）没有被宏保护，在宏为 0 时，该指针将永远不会被初始化，如果其他未受保护的代码解引用了它，将引发崩溃。
  • 建议：确保类成员变量 FdoSelectionManager *m_selectionManager; 也必须被 #if 包裹，或者在宏为 0 时将其定义为 nullptr 且不实例化。

2. 代码质量

• 注释风格：
  • 问题：头文件中新增的注释使用了 C 风格的 /* ... */ 或普通行注释，解释了为什么不需要这个宏。这对于理解代码上下文非常有帮助，但注释稍微有些冗长。
  • 建议：注释内容很好，建议保留。但可以稍微精简，使其更易于维护者阅读。
  • 改进示例：

    // FDO selection manager registration is handled by dde-daemon.
    // If enabled here, tray context menu positioning must also be handled correctly.
    // Consider moving FdoSelectionManager to dde-shell in the future.
    #define DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER 0

• 环境检测逻辑的完整性：
  • 问题：在 .cpp 的条件判断中：if ((qgetenv("XDG_SESSION_TYPE") == "wayland") && UTIL->isXAvaliable())。这里 isXAvaliable 存在拼写错误（应为 isXAvailable，多了一个 'i'，虽然这是旧代码，但既然审查到，建议顺手修复）。
  • 建议：修正拼写错误 isXAvaliable -> isXAvailable。

3. 代码性能

• 环境变量的获取：
  • 问题：qgetenv("XDG_SESSION_TYPE") 会在堆上分配 QByteArray 来比较字符串。在构造函数中调用一次对性能影响微乎其微，但如果该类被频繁实例化，或者未来这段逻辑被移动到循环中，就会产生不必要的内存分配和字符串比较开销。
  • 建议：当前实现可接受。若后续有性能优化需求，可考虑使用 qEnvironmentVariableIsEmpty 或 qEnvironmentVariableIntValue（如果环境变量是整数标识），或者使用 X11 的原生 API 检测会话类型，避免 QByteArray 的构造。

4. 代码安全

• 宏为 0 时的降级安全：
  • 问题：当 _DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER 为 0 时，Wayland 环境下将不再注册 FDO selection manager。如果 dde-daemon 没有如预期般接管这部分工作，系统托盘的 XEmbed 嵌入功能可能会静默失败，用户无法看到托盘图标，且没有任何错误提示。
  • 建议：在 #else 分支中增加调试日志，明确提示开发者/用户当前未注册 FDO Selection Manager。
  • 改进代码示例：

    #if DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER
        if ((qgetenv("XDG_SESSION_TYPE") == "wayland") && UTIL->isXAvailable()) {
            qWarning() << "Not running on X11, registering FDO selection manager";
            m_selectionManager = new FdoSelectionManager(this);
        }
    #else
        if (qgetenv("XDG_SESSION_TYPE") == "wayland") {
            qDebug() << "FDO selection manager registration is disabled, expecting dde-daemon to handle it.";
        }
    #endif

综合修改建议

结合以上分析，我为你提供一份优化后的代码片段：

xembedprotocolhandler.h:

// SPDX-FileCopyrightText: 2024 - 2026 UnionTech Software Technology Co., Ltd.
//
// SPDX-License-Identifier: GPL-3.0-or-later

#ifndef XEMBEDPROTOCOLHANDLER_H
#define XEMBEDPROTOCOLHANDLER_H

#include <xcb/composite.h>
#include <xcb/xcb_image.h>

// FDO selection manager registration is handled by dde-daemon.
// If enabled here, tray context menu positioning must also be handled correctly.
// Consider moving FdoSelectionManager to dde-shell in the future.
#ifndef DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER
#define DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER 0
#endif

#if DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER
class FdoSelectionManager;
#endif

namespace tray {
class XembedProtocol : public AbstractTrayProtocol, public QAbstractNativeEventFilter
{
    // ...
#if DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER
    FdoSelectionManager *m_selectionManager = nullptr;
#endif
};
}
#endif // XEMBEDPROTOCOLHANDLER_H

xembedprotocolhandler.cpp:

#include "abstracttrayprotocol.h"
#include "traymanager1interface.h"
#include "xembedprotocolhandler.h"

#if DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER
#include "fdoselectionmanager.h"
#endif

#include "util.h"
#include "plugin.h"

// ... 其他代码 ...

XembedProtocol::XembedProtocol(QObject *parent)
    : AbstractTrayProtocol(parent)
{
    // ... 其他初始化 ...

    QMetaObject::invokeMethod(this, &XembedProtocol::onTrayIconsChanged, Qt::QueuedConnection);

#if DDE_TRAY_LOADER_HANDLE_XEMBED_SELECTION_MANAGER
    // 注意：修复了 isXAvaliable 的拼写错误 -> isXAvailable
    if ((qgetenv("XDG_SESSION_TYPE") == "wayland") && UTIL->isXAvailable()) {
        qWarning() << "Not running on X11, registering FDO selection manager";
        m_selectionManager = new FdoSelectionManager(this);
    }
#else
    if (qgetenv("XDG_SESSION_TYPE") == "wayland") {
        qDebug() << "Skipping FDO selection manager registration (disabled by macro).";
    }
#endif

    QTimer::singleShot(0, this, [this](){
        m_trayManager->Manage();
    });
}

希望这些审查意见和代码改进能对你有所帮助！如果有任何疑问，欢迎随时提问。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, tsic404

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