Skip to content

fix: delay archive loading for non-first instance on Wayland#407

Merged
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/eagle-20260415from
LiHua000:develop/eagle-20260415
May 8, 2026
Merged

fix: delay archive loading for non-first instance on Wayland#407
deepin-bot[bot] merged 1 commit into
linuxdeepin:develop/eagle-20260415from
LiHua000:develop/eagle-20260415

Conversation

@LiHua000
Copy link
Copy Markdown
Contributor

@LiHua000 LiHua000 commented May 8, 2026

On Wayland, the window position is set by the compositor for non-first instances. Previously the archive loading was delayed unconditionally on Wayland, causing the home page to flash. Now only non-first instances delay loading, and show the loading animation before the delay to avoid flashing.

Bug: https://pms.uniontech.com/bug-view-359077.html

On Wayland, the window position is set by the compositor for
non-first instances. Previously the archive loading was delayed
unconditionally on Wayland, causing the home page to flash.
Now only non-first instances delay loading, and show the loading
animation before the delay to avoid flashing.

Bug: https://pms.uniontech.com/bug-view-359077.html
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Please try again later or upgrade to continue using Sourcery

@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码主要是为了修复在 Wayland 显示协议下,非首个实例窗口位置显示不正确的问题。通过引入延迟加载机制,等待窗口管理器设置好窗口位置后再显示对话框。

以下是对这段代码的详细审查意见,包括语法逻辑、代码质量、代码性能和代码安全四个方面:

1. 语法逻辑

  • 逻辑判断分支的完整性
    • main.cpp 中,if (dbus.registerService(...)) 分支用于处理成功注册服务(即首个实例)的情况,else if 处理 Wayland 下的情况。如果既不是首个实例,也不是 Wayland 环境,代码将什么都不做。
    • 建议:虽然逻辑上可能没问题,但建议在 else 分支中添加注释或日志,说明在非 Wayland 且非首个实例下的预期行为(例如:直接退出或静默失败),以增加代码可读性。
  • 静态变量与成员变量的替换
    • 原代码使用 static bool firstLoad 来判断是否首次加载。新代码引入了成员变量 m_bDelayQueryDialog
    • 优点:使用成员变量是更好的做法,因为 static 局部变量在程序整个生命周期内存在,如果对象被销毁重建,static 变量状态不会重置,可能导致逻辑错误。成员变量随对象生命周期管理,逻辑更严谨。
  • 标志位复位时机
    • handleArguments_Open 中,m_bDelayQueryDialog 被检查后立即置为 false (m_bDelayQueryDialog = false;)。
    • 潜在风险:如果在 QTimer::singleShot 的 lambda 执行之前,对象被销毁,或者 handleArguments_Open 被意外多次调用,可能会导致状态混乱。但在当前主窗口逻辑中通常是单次调用,风险较低。

2. 代码质量

  • 命名规范
    • 变量名 m_bDelayQueryDialog 符合匈牙利命名法(m_ 成员变量,b 布尔型),在 C++ 项目(尤其是 Qt 项目)中很常见,可读性尚可。
  • 硬编码常量
    • QTimer::singleShot(200, ...) 中的 200 毫秒是硬编码的。
    • 建议:将其定义为类常量或宏,例如 static constexpr int kWaylandDelayMs = 200;,并添加注释解释为什么需要这个特定的延迟时间(例如:给窗口管理器留出足够的时间进行 XDG Foreign 等握手操作)。
  • 魔法字符串
    • tr("Loading, please wait...") 是硬编码字符串。
    • 建议:如果项目中已有统一的字符串管理(如 QStringLiteral 宏或专门的字符串头文件),建议统一管理,以减少翻译时的冗余和内存分配。
  • Lambda 捕获
    • [this, path] 捕获了 this 指针和 path 变量。
    • 注意path 是按值捕获的,这是安全的。this 指针捕获需要确保 MainWindow 对象在定时器触发时仍然存活。在 main 函数中 w 是栈对象,通常没问题,但需注意对象生命周期管理。

3. 代码性能

  • 延迟加载
    • 使用 QTimer::singleShot 进行延迟加载是一种标准的异步处理方式,不会阻塞主线程,性能影响微乎其微。
  • 字符串拼接
    • "com.deepin.compressor"+QString::number(...) 这种字符串拼接在启动时执行一次,性能损耗可以忽略不计。
  • UI 刷新
    • 代码中显式调用了 m_pLoadingPage->startLoading(),这能确保用户在等待期间看到加载动画,提升用户体验,这是良好的实践。

4. 代码安全

  • 数组/列表越界风险
    • auto path = listParam[0];
    • 严重问题:在 handleArguments_Open 函数中,直接访问 listParam[0] 而没有检查 listParam 是否为空。虽然函数名暗示是打开文件,参数可能非空,但如果外部调用传入空列表,会导致程序崩溃。
    • 改进建议:必须添加边界检查。
      if (listParam.isEmpty()) {
          qWarning() << "Arguments list is empty!";
          return false;
      }
      auto path = listParam[0];
  • 空指针检查
    • m_pLoadingPage->setDes(...)m_pLoadingPage->startLoading()
    • 建议:确认 m_pLoadingPage 在此时是否已经初始化。如果 setDelayQueryDialog(true) 被过早调用,可能会导致空指针解引用。建议添加断言或空指针检查。

5. 综合改进建议代码示例

针对 src/source/mainwindow.cpp 中的修改,建议优化如下:

// 在 mainwindow.cpp 中
// 定义常量
static constexpr int kWaylandDelayMs = 200;

bool MainWindow::handleArguments_Open(const QStringList &listParam)
{
    // 1. 安全性改进:添加参数检查
    if (listParam.isEmpty()) {
        qWarning() << "Error: No file path provided to open.";
        return false;
    }

    qInfo() << "打开文件";
    m_eStartupType = StartupType::ST_Normal;

    // 2. 逻辑改进:使用成员变量控制延迟
    if (m_bDelayQueryDialog) {
        m_bDelayQueryDialog = false; // 重置标志位
        
        const QString &path = listParam[0]; // 使用 const引用避免拷贝

        // 3. 安全性/健壮性改进:确保 LoadingPage 存在
        if (m_pLoadingPage) {
            m_ePageID = PI_Loading;
            m_pLoadingPage->setDes(tr("Loading, please wait..."));
            m_pLoadingPage->startLoading();
        } else {
            qWarning() << "LoadingPage is not initialized!";
            // 即使没有 LoadingPage,也要尝试加载,或者做错误处理
        }

        // 4. 性能/逻辑改进:使用常量代替魔法数字
        // 捕获 path 时,由于 path 是 const 引用,lambda 存储时最好按值捕获或确保生命周期
        QTimer::singleShot(kWaylandDelayMs, [this, path]() {
            // 再次检查对象有效性(可选,视具体架构而定)
            loadArchive(path);
        });
        
        return true; // 或者根据实际逻辑返回
    }
    
    // ... 原有逻辑 ...
}

总结

这段代码的修改方向是正确的,通过引入延迟机制解决了 Wayland 环境下的窗口定位问题。
主要需要关注的是 安全性问题(listParam 越界检查)代码健壮性(空指针检查、常量定义)。将静态局部变量改为成员变量是正确的重构方向。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, max-lvs

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

Details Needs approval from an approver in each of these files:

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

@LiHua000
Copy link
Copy Markdown
Contributor Author

LiHua000 commented May 8, 2026

/merge

@deepin-bot deepin-bot Bot merged commit 12702bc into linuxdeepin:develop/eagle-20260415 May 8, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants