fix(main_window): update sendSavingNotify to return notification ID

[dengzhongyuan365-dev]

• Change sendSavingNotify from void to unsigned int to return the notification ID.
• Implement logic to close the notification in exitRecord using the returned ID.
• Adjust related calls in stopRecord to handle the notification ID correctly.

This improves the notification handling during the recording process, ensuring that users receive accurate feedback on the saving status.

bug: https://pms.uniontech.com/bug-view-359845.html

[deepin-ci-robot]

deepin pr auto review

这份，我将从语法逻辑、代码质量、代码性能和代码安全四个方面对提供的 Git diff 进行详细审查。

1. 语法逻辑审查

整体逻辑：
代码的主要目的是改进录屏软件中的通知机制。具体来说，是将"正在保存"的通知与"录制完成"的通知进行关联。当停止录屏时，先显示"正在保存"，待保存完成后，关闭"正在保存"的通知，再显示"录制完成"。

具体改动：

1. MainWindow::sendSavingNotify:

   • 改动: 返回值从 void 改为 quint32。
   • 逻辑: 函数现在调用 D-Bus 的 Notify 方法后，会检查返回的消息类型 (QDBusMessage::ReplyMessage) 和参数列表。如果是有效回复，则提取第一个参数（通知 ID）并返回。如果调用失败（例如 root 用户或 D-Bus 错误），则返回 RecordProcess::RECORD_SAVING_NOTIFY_ID_INVALID (即 0)。
   • 评价: 逻辑清晰。通过返回通知 ID，使得调用者（MainWindow）能够追踪这个特定的通知，以便后续关闭它。if (Utils::isRootUser) 的提前返回是合理的，因为 root 用户通常不需要桌面通知。

2. MainWindow::stopRecord:

   • 改动: 调用 sendSavingNotify() 后，将其返回值通过 recordProcess.setRecordSavingNotifyId(...) 保存到 RecordProcess 对象中。
   • 逻辑: 将通知 ID 从 UI 层 (MainWindow) 传递到业务逻辑层 (RecordProcess) 存储，以便在 RecordProcess 的 exitRecord 方法中使用。
   • 评价: 逻辑通顺。注释“完成时用 CloseNotification 关闭本条再弹「录制完成」”准确描述了意图。

3. RecordProcess::setRecordSavingNotifyId:

   • 改动: 新增成员函数，用于设置成员变量 m_recordSavingNotifyId。
   • 评价: 标准的 setter 函数，逻辑简单正确。

4. RecordProcess::startRecord:

   • 改动: 初始化 m_recordSavingNotifyId = RECORD_SAVING_NOTIFY_ID_INVALID;。
   • 评价: 这是一个很好的防御性编程实践，确保每次开始新的录制时，旧的通知 ID 被重置，防止误关闭上一次的通知。

5. RecordProcess::exitRecord:

   • 改动: 在弹出"录制完成"通知之前，检查 m_recordSavingNotifyId。如果有效，则调用新增的静态函数 closeFdoNotification 关闭它，并将 ID 重置为无效值。
   • 评价: 逻辑正确。确保了通知的连贯性，避免了"正在保存"和"录制完成"两个通知同时堆叠在通知栏。

潜在逻辑问题：

• sendSavingNotify 中的 id 变量: 在 sendSavingNotify 函数中，定义了 quint32 id = RecordProcess::RECORD_SAVING_NOTIFY_ID_INVALID;，但这个局部变量 id 实际上并没有被使用。D-Bus 调用的返回值是通过 reply.arguments().constFirst() 获取的。这个未使用的变量虽然不会导致错误，但属于冗余代码，建议删除。

2. 代码质量审查

优点：

1. 类型安全: 使用 quint32 替代 unsigned int，与 Qt 的类型系统和 D-Bus 规范保持一致。
2. 命名规范: 新增的常量 RECORD_SAVING_NOTIFY_ID_INVALID 命名清晰，使用了 constexpr，符合现代 C++ 最佳实践。
3. 注释: 新增的注释（如中文注释）准确地解释了代码的意图，提高了可读性。
4. 封装: 将关闭通知的逻辑封装在 closeFdoNotification 静态函数中，避免了代码重复（虽然目前只在一处使用，但未来可能复用）。
5. 防御性编程: 在 startRecord 中重置通知 ID，在 exitRecord 中检查 ID 有效性，都体现了良好的防御性编程思想。

改进建议：

1. 删除未使用变量: 如前所述，sendSavingNotify 中的局部变量 id 是未使用的，应该删除。
2. 常量定义位置: RECORD_SAVING_NOTIFY_ID_INVALID 定义在 RecordProcess 类中。考虑到 MainWindow::sendSavingNotify 也需要使用它，这表明这个常量可能具有更广泛的适用性。如果未来其他类也需要使用，可以考虑将其移到一个更通用的头文件中（如 utils.h 或专门的 constants.h）。不过，目前的定义位置也是可以接受的，因为它主要与 RecordProcess 的逻辑相关。

3. 代码性能审查

1. D-Bus 调用: 代码涉及 D-Bus 通信（Notify 和 CloseNotification），这是 IPC（进程间通信），会有一定的性能开销。

   • sendSavingNotify 现在会等待 D-Bus 回复（callWithArgumentList 默认是阻塞调用）。在 UI 线程中执行阻塞的 IPC 调用可能会导致界面短暂卡顿。
   • 建议: 如果对响应速度要求极高，可以考虑使用异步调用（QDBusInterface::asyncCall），但这会增加代码复杂度。对于通知这种非关键路径操作，目前的同步阻塞调用在大多数情况下是可以接受的。

2. 内存分配: QList<QVariant> arg; 在 sendSavingNotify 中创建。这是必要的，因为 D-Bus 调用需要参数列表。性能影响极小。

3. 字符串拼接: tr("Ignore") 等字符串操作。Qt 的 tr 机制非常高效，通常不是性能瓶颈。

总结: 性能方面没有明显问题。D-Bus 调用是主要的性能考量点，但在当前上下文中是合理的。

4. 代码安全审查

1. D-Bus 安全:

   • 代码直接构造 QDBusInterface 连接到 org.freedesktop.Notifications。这是标准的桌面环境做法，通常没有安全风险，因为通知服务是受信任的系统服务。
   • 注意: 如果 org.freedesktop.Notifications 服务不存在或被恶意程序劫持（在标准 Linux 桌面环境中极不可能），call 可能会失败或返回意外结果。代码通过检查 reply.type() == QDBusMessage::ReplyMessage 处理了调用失败的情况，这是安全的。

2. 类型安全:

   • reply.arguments().constFirst().toUInt() 将返回值转换为 unsigned int。D-Bus 的 Notify 方法返回的 ID 确实是一个 uint32，所以这个转换是安全的。

3. Root 用户检查: if (Utils::isRootUser) 检查防止了在 root 用户下尝试显示桌面通知，这是一个很好的安全/健壮性措施。

4. 空指针/无效值检查:

   • !reply.arguments().isEmpty() 检查确保了在访问参数列表前列表不为空。
   • if (id == RecordProcess::RECORD_SAVING_NOTIFY_ID_INVALID) 检查确保了不会尝试关闭无效的通知 ID。

总结: 代码在安全性方面做得很好，包含了必要的检查和错误处理。

综合改进建议

1. 清理冗余代码:
   在 MainWindow::sendSavingNotify 中，删除未使用的局部变量 id。

   // 删除这一行
   // quint32 id = RecordProcess::RECORD_SAVING_NOTIFY_ID_INVALID;

2. 常量定义:
   虽然 RECORD_SAVING_NOTIFY_ID_INVALID 目前只在 RecordProcess 及其友元类中使用，但考虑到 MainWindow 依赖它，且它代表的是 D-Bus 通知的通用概念，未来可能会在其他地方使用。如果项目中有 constants.h 或 utils.h，建议将其移至此处。如果项目结构简单，保留在 RecordProcess 中也是可以的。

3. D-Bus 错误处理:
   当前代码检查了 reply.type() == QDBusMessage::ReplyMessage。如果 D-Bus 调用失败（返回 ErrorMessage），代码会返回 RECORD_SAVING_NOTIFY_ID_INVALID。这是正确的处理方式。
   可以考虑在 Debug 模式下打印错误信息，以便排查问题：

   QDBusMessage reply = notification.callWithArgumentList(QDBus::AutoDetect, "Notify", arg);
   if (reply.type() == QDBusMessage::ReplyMessage && !reply.arguments().isEmpty()) {
       return reply.arguments().constFirst().toUInt();
   } else {
       // 可选：打印错误日志
       if (reply.type() == QDBusMessage::ErrorMessage) {
           qCWarning(dsrApp) << "Failed to send saving notify:" << reply.errorMessage();
       }
       return RecordProcess::RECORD_SAVING_ID_INVALID;
   }

4. 异步调用考虑:
   如前所述，如果发现停止录制时 UI 有明显卡顿，可以将 sendSavingNotify 和 closeFdoNotification 改为异步调用。但这需要使用 QDBusPendingCallWatcher 和信号槽机制，会显著增加代码复杂度。目前的实现对于大多数应用场景是足够好的。

5. sendSavingNotify 函数签名:
   函数现在返回 quint32。考虑到 RECORD_SAVING_NOTIFY_ID_INVALID 被定义为 0，而 D-Bus 通知 ID 通常从 1 开始，使用 quint32 是合适的。确保调用者（如 MainWindow::stopRecord）正确处理了这个返回值（代码中已正确处理）。

最终代码示例（整合建议）

以下是整合了清理冗余变量和增强日志记录后的 sendSavingNotify 函数示例：

quint32 MainWindow::sendSavingNotify()
{
    if (Utils::isRootUser) {
        return RecordProcess::RECORD_SAVING_NOTIFY_ID_INVALID;
    }
    // Popup notify.
    QDBusInterface notification("org.freedesktop.Notifications",
                                "/org/freedesktop/Notifications",
                                "org.freedesktop.Notifications",
                                QDBusConnection::sessionBus());
    QStringList actions;
    actions << "_close" << tr("Ignore");
    int timeout = 3000;
    // 删除了未使用的 quint32 id 变量

    QList<QVariant> arg;
    arg << Utils::appName                                                   // appname
        << ""                                                               // app icon
        << tr("Recording is being saved")                                  // summary
        << tr("Recording is being saved, please wait...")                  // body
        << actions                                                          // actions
        << QVariantMap()                                                    // hints
        << timeout;                                                         // timeout
    
    QDBusMessage reply = notification.callWithArgumentList(QDBus::AutoDetect, "Notify", arg);
    if (reply.type() == QDBusMessage::ReplyMessage && !reply.arguments().isEmpty()) {
        return reply.arguments().constFirst().toUInt();
    } else {
        if (reply.type() == QDBusMessage::ErrorMessage) {
            qCWarning(dsrApp) << "Failed to send saving notify:" << reply.errorMessage();
        }
        return RecordProcess::RECORD_SAVING_NOTIFY_ID_INVALID;
    }
}

总结

这段代码修改整体质量很高，逻辑清晰，安全性良好。主要改进点在于实现了通知的替换机制，提升了用户体验。唯一的小瑕疵是存在一个未使用的局部变量，建议删除。除此之外，代码已经非常完善。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: dengzhongyuan365-dev, lzwind

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

[dengzhongyuan365-dev]

/forcemerge