fix: fix async access causing invalid properties in SNI tray

[18202781743]

1. Changed StatusNotifierItem from async to sync mode to ensure
   properties are available when accessed
2. Implemented lazy initialization for DBusMenuImporter to avoid
   accessing uninitialized menu
3. Added lazy generation of tray ID to prevent empty ID when accessed
   before initialization
4. Stored service path separately for later use in menu importer
   creation
5. Fixed menu access to use lazy-loaded menu importer instead of
   potentially null pointer

Log: Fixed system tray icons sometimes showing no menu or incorrect
behavior

Influence:

1. Test system tray icons from various applications (especially those
   using SNI protocol)
2. Verify right-click menus appear correctly for all tray icons
3. Check that tray icon IDs remain consistent and unique
4. Test theme switching while tray menus are open
5. Verify left-click activation works properly
6. Test with applications that send attention signals

fix: 修复异步访问导致SNI托盘属性无效的问题

1. 将StatusNotifierItem从异步模式改为同步模式，确保访问时属性已就绪
2. 实现DBusMenuImporter的延迟初始化，避免访问未初始化的菜单
3. 添加托盘ID的延迟生成，防止在初始化前访问时返回空ID
4. 单独存储服务路径供后续创建菜单导入器使用
5. 修复菜单访问，使用延迟加载的菜单导入器代替可能为空的指针

Log: 修复系统托盘图标有时不显示菜单或行为异常的问题

Influence:

1. 测试来自不同应用程序的系统托盘图标（特别是使用SNI协议的）
2. 验证所有托盘图标的右键菜单是否正确显示
3. 检查托盘图标ID是否保持唯一且一致
4. 测试在托盘菜单打开时切换主题
5. 验证左键激活功能是否正常工作
6. 测试发送attention信号的应用程序

PMS: BUG-351643

Summary by Sourcery

Ensure SNI tray properties and menus are available when accessed by making notifier interaction synchronous and deferring menu/importer initialization until needed.

Bug Fixes:

• Prevent system tray SNI items from exposing invalid or empty properties due to asynchronous access timing.
• Avoid null or uninitialized DBus menu importer usage by lazily creating it when the menu is needed.
• Guarantee non-empty, stable tray IDs by generating them on first access instead of at construction time.
• Ensure theme change handling updates the SNI tray menu via a valid, lazily loaded menu importer.

[sourcery-ai]

Hey - I've found 3 issues, and left some high level feedback:

• The new menuImporter() implementation is declared non-const in the header but defined as const in the cpp and then uses const_cast; consider making it non-const everywhere (and calling it only from non-const contexts) to avoid the mismatch and the need for const_cast.
• In the constructor you assign m_path = pair.first but then pass m_path as the first argument to DBusMenu(m_path, ...) while the description says you are storing the service path; double-check whether pair.first/pair.second semantics are correct here and whether m_path should actually hold the service name or the object path, and rename the member accordingly for clarity.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- The new `menuImporter()` implementation is declared non-const in the header but defined as `const` in the cpp and then uses `const_cast`; consider making it non-const everywhere (and calling it only from non-const contexts) to avoid the mismatch and the need for `const_cast`.
- In the constructor you assign `m_path = pair.first` but then pass `m_path` as the first argument to `DBusMenu(m_path, ...)` while the description says you are storing the service path; double-check whether `pair.first`/`pair.second` semantics are correct here and whether `m_path` should actually hold the service name or the object path, and rename the member accordingly for clarity.

## Individual Comments

### Comment 1
<location path="plugins/application-tray/sniprotocolhandler.h" line_range="77" />
<code_context>

 private:
     void generateId();
+    DBusMenuImporter *menuImporter();

 private:
</code_context>
<issue_to_address>
**issue (bug_risk):** Method signature for menuImporter differs between header and implementation (missing const in header).

In the .cpp file this method is defined as `DBusMenuImporter *SniTrayProtocolHandler::menuImporter() const`, but here it’s declared without `const`, which will cause a compile error. Please make the signatures match—ideally `DBusMenuImporter *menuImporter() const;`, given it’s used from `const` contexts (and adjust call sites if required).
</issue_to_address>

### Comment 2
<location path="plugins/application-tray/sniprotocolhandler.cpp" line_range="148-152" />
<code_context>
     m_id = UTIL->generateUniqueId(id);
 }

+DBusMenuImporter *SniTrayProtocolHandler::menuImporter() const
+{
+    if (!m_dbusMenuImporter) {
+        auto that = const_cast<SniTrayProtocolHandler *>(this);
+        that->m_dbusMenuImporter = new DBusMenu(m_path, m_sniInter->menu().path(), that);
+    }
+    return m_dbusMenuImporter;
</code_context>
<issue_to_address>
**suggestion:** The use of const_cast for lazy initialization in menuImporter suggests the constness model might be off.

Initializing `m_dbusMenuImporter` via `const_cast` in a `const` method obscures object state and thread-safety guarantees. Prefer either (1) making `menuImporter` non-const and using it only from non-const contexts, or (2) marking `m_dbusMenuImporter` as `mutable` to allow lazy init from const call sites. This preserves const-correctness without `const_cast`.

Suggested implementation:

```cpp
DBusMenuImporter *SniTrayProtocolHandler::menuImporter()
{
    if (!m_dbusMenuImporter) {
        m_dbusMenuImporter = new DBusMenu(m_path, m_sniInter->menu().path(), this);
    }
    return m_dbusMenuImporter;
}

```

1. In the corresponding header file (likely `plugins/application-tray/sniprotocolhandler.h`), change the declaration from:
   `DBusMenuImporter *menuImporter() const;`
   to:
   `DBusMenuImporter *menuImporter();`
2. Ensure all call sites are from non-const contexts (the shown lambda using `this` is fine). If there are any calls on a `const SniTrayProtocolHandler`, those usages should be revisited or cast to non-const only where logically safe.
</issue_to_address>

### Comment 3
<location path="plugins/application-tray/sniprotocolhandler.cpp" line_range="162-165" />
<code_context>

 QString SniTrayProtocolHandler::id() const
 {
+    if (m_id.isEmpty()) {
+        const_cast<SniTrayProtocolHandler *>(this)->generateId();
+    }
     return m_id;
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Lazy initialization of m_id via const_cast in id() can have subtle concurrency and constness implications.

If `id()` is called from multiple threads, this lazy write to `m_id` is racy. Even in single-threaded use it weakens const semantics. Prefer restoring eager initialization (e.g. call `generateId()` in the constructor) or make `m_id` `mutable` and perform lazy init without `const_cast`, using proper synchronization if concurrent access is possible.

Suggested implementation:

```cpp
uint32_t SniTrayProtocolHandler::windowId() const
{
    return m_sniInter->windowId();
}

QString SniTrayProtocolHandler::id() const
{
    return m_id;
}

```

To fully satisfy your review comment, you should also:
1. Ensure `m_id` is eagerly initialized, typically by calling `generateId();` in the `SniTrayProtocolHandler` constructor (or wherever the instance is first fully set up).
2. If you still want lazy initialization for some reason, mark `m_id` as `mutable` in the header and implement a thread-safe lazy initialization strategy there (e.g. using `std::call_once` or equivalent), avoiding `const_cast` entirely.
3. Verify that no callers rely on `id()` having side effects; it should now be a pure accessor.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[deepin-ci-robot]

deepin pr auto review

这段代码主要对 SniTrayProtocolHandler 类进行了重构，将 DBusMenuImporter 的初始化从构造函数中移除，改为通过 menuImporter() 函数进行延迟初始化（Lazy Initialization）。以下是对这段代码的详细审查意见：

1. 语法逻辑

• const 正确性与 mutable 成员：

  • 问题：menuImporter() 被声明为 const 成员函数，但其内部逻辑修改了成员变量 m_dbusMenuImporter（通过 const_cast 去除 this 的 const 属性后赋值）。虽然在 C++ 中这样做在语法上是可以编译通过的，但从逻辑上讲，改变了对象状态的函数不应该是 const 的。
  • 建议：如果意图是保持接口的 const 特性（例如在其他 const 函数中调用），应将 m_dbusMenuImporter 声明为 mutable。这样就不需要 const_cast，逻辑上也更清晰，表明该成员变量的修改不影响对象的“逻辑常量性”。
  • 修改示例：

    // 头文件中
    mutable DBusMenuImporter *m_dbusMenuImporter;
    
    // cpp文件中
    DBusMenuImporter *SniTrayProtocolHandler::menuImporter() const
    {
        if (!m_dbusMenuImporter) {
            m_dbusMenuImporter = new DBusMenu(m_service, m_menuPath, const_cast<SniTrayProtocolHandler*>(this));
        }
        return m_dbusMenuImporter;
    }

    注：虽然去掉了 that 变量，但 new DBusMenu 的第三个参数通常需要非 const 的 QObject* 指针作为父对象，所以 const_cast 在这里可能仍然需要，或者确认 DBusMenu 构造函数是否接受 const 父对象（通常不接受）。

• 初始化顺序：

  • 问题：在构造函数中，init() 被调用，而 init() 内部调用了 generateId()。generateId() 依赖于 m_sniInter。虽然 m_sniInter 在 init() 之前已经初始化，逻辑上是通的，但这种依赖关系隐藏在 init() 内部，不如直接写在构造函数中直观。
  • 建议：目前的写法尚可，但需确保 m_sniInter 初始化后才能调用 init()，当前代码满足此条件。

2. 代码质量

• 延迟初始化：

  • 优点：将 DBusMenuImporter 的创建改为按需创建是一个很好的优化。并非所有的托盘应用都会被右键点击弹出菜单，延迟初始化可以节省不必要的 DBus 调用和内存开销。
  • 改进：menuImporter() 的实现是标准的单例/懒加载模式，代码质量良好。

• 变量命名与存储：

  • 观察：新增了 m_service 和 m_menuPath 成员变量来存储 DBus 服务名和菜单路径，以便在延迟初始化时使用。
  • 建议：这是必要的改动。确保 m_menuPath 在对象生命周期内不会变化。如果 StatusNotifierItem 的路径可能动态变化（例如重新注册），这里可能会有问题，但通常 SNI 规范中路径是静态的。

• 内存管理：

  • 观察：m_dbusMenuImporter 在 menuImporter() 中被 new，且父对象设为 this。
  • 确认：Qt 的对象树机制会确保在 SniTrayProtocolHandler 析构时自动删除 m_dbusMenuImporter，因此没有内存泄漏风险。

3. 代码性能

• 性能提升：
  • 通过延迟初始化，对于那些用户从未右键点击过的托盘图标，系统完全避免了创建 DBusMenu 对象以及与之相关的 DBus 交互（获取菜单布局等）。这能显著减少启动时的资源占用和 DBus 总线负载。

4. 代码安全

• 线程安全：

  • 问题：menuImporter() 包含检查指针并赋值的逻辑 if (!m_dbusMenuImporter) ...。如果 SniTrayProtocolHandler 的对象可能在多线程环境中被访问（例如 UI 线程和 DBus 事件线程同时触发右键菜单），这种写法不是线程安全的，可能导致创建多个实例或崩溃。
  • 分析：在 Qt 应用中，QObject 及其子类通常绑定在创建它的线程（通常是主线程）。DBus 信号默认使用 Qt::AutoConnection，如果 menuImporter() 仅在主线程（UI 线程）被调用，则是安全的。
  • 建议：确认该类是否仅在主线程使用。如果是，当前代码是安全的。如果未来可能涉及多线程调用，建议加锁或使用 QMutexLocker 保护 m_dbusMenuImporter 的初始化过程。

• 空指针检查：

  • 观察：在 eventFilter 中使用了 menuImporter()->menu()，并紧跟着 Q_CHECK_PTR(menu)。
  • 评价：这是一个好的防御性编程习惯。虽然 menuImporter() 保证返回非空指针（如果 new 失败会抛出异常），但检查 menu() 的返回值是否为空是必要的，因为 DBus 调用可能失败或返回空路径。

总结建议

这段代码的改动是积极的，主要实现了菜单导入器的延迟加载以优化性能。为了进一步提高代码质量，建议进行以下修改：

1. 将 m_dbusMenuImporter 声明为 mutable，以配合 const 成员函数 menuImporter()，避免使用 const_cast 修改 this 指针带来的潜在风险和阅读上的困惑。
2. 确保 m_service 和 m_menuPath 在构造函数初始化列表中被正确初始化（目前是在函数体中赋值，虽然对于 QString 来说性能差异可忽略，但初始化列表是更规范的写法）。

修改后的代码片段建议（头文件）：

private:
    mutable DBusMenuImporter *m_dbusMenuImporter; // 添加 mutable
    // ...

修改后的代码片段建议（源文件）：

DBusMenuImporter *SniTrayProtocolHandler::menuImporter() const
{
    if (!m_dbusMenuImporter) {
        // 使用 mutable 成员，不再需要 const_cast<SniTrayProtocolHandler*>(this)
        // 但如果 DBusMenu 构造函数严格要求非 const QObject*，则保留 const_cast 仅用于传参
        m_dbusMenuImporter = new DBusMenu(m_service, m_menuPath, const_cast<SniTrayProtocolHandler*>(this));
    }
    return m_dbusMenuImporter;
}

[deepin-ci-robot]

[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.

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

[18202781743]

/forcemerge

[deepin-bot]

This pr force merged! (status: blocked)