sync: from linuxdeepin/dde-session-shell

[deepin-ci-robot]

Synchronize source files from linuxdeepin/dde-session-shell.

Source-pull-request: linuxdeepin/dde-session-shell#64

[sourcery-ai]

Sorry @deepin-ci-robot, 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

这段代码主要涉及对壁纸服务 dde-wallpaper-cache 的存在性检测和文件操作的改进。以下是对该 git diff 的详细审查意见：

1. 语法逻辑

• 条件编译逻辑清晰：使用 #ifndef ENABLE_DSS_SNIPE 和 #ifdef ENABLE_DSS_SNIPE 分隔了不同环境下的服务检测逻辑，逻辑上是合理的。
• 文件操作改进：在调用 QDBusInterface 方法前显式关闭了文件句柄 file.close()，这是一个好的改进，避免了潜在的文件句柄泄漏。

2. 代码质量

• 代码可读性：代码结构清晰，但可以进一步优化注释，例如说明 ENABLE_DSS_SNIPE 宏的具体用途。
• 错误处理：对 wallpaperCacheInterface.isValid() 的检查仅在 ENABLE_DSS_SNIPE 定义时生效，建议在非 ENABLE_DSS_SNIPE 环境下也添加类似的检查，以增强健壮性。

3. 代码性能

• 文件操作：显式关闭文件句柄 file.close() 是必要的，但 Qt 的 QFile 析构函数会自动关闭文件，因此这一改进对性能影响不大，更多是代码规范性的提升。
• 服务检测：在 ENABLE_DSS_SNIPE 环境下，直接通过 QDBusInterface::isValid() 检测服务是否存在，比检查文件路径更高效，建议统一采用这种方式。

4. 代码安全

• 文件句柄泄漏：显式调用 file.close() 是好的实践，尤其是在调用可能阻塞的 QDBusInterface 方法前，避免文件句柄长时间占用。
• 服务检测安全性：在 ENABLE_DSS_SNIPE 环境下，通过 QDBusInterface::isValid() 检测服务是否存在比检查文件路径更安全，因为服务可能被动态卸载或替换。

改进建议

1. 统一服务检测方式：

   • 建议在所有环境下统一使用 QDBusInterface::isValid() 检测服务是否存在，而不是依赖文件路径检查。例如：

     QDBusInterface wallpaperCacheInterface("org.deepin.dde.WallpaperCache", "/org/deepin/dde/WallpaperCache",
                                            "org.deepin.dde.WallpaperCache", QDBusConnection::systemBus());
     if (!wallpaperCacheInterface.isValid()) {
         qWarning() << "dde-wallpaper-cache service not existed";
         return false;
     }

   • 如果必须保留文件路径检查，建议添加注释说明原因。

2. 增强错误处理：

   • 在调用 wallpaperCacheInterface.call 方法前，可以增加对参数的校验，例如检查 originPath 是否为空或无效路径。

3. 资源管理：

   • 虽然显式调用 file.close() 是好的实践，但可以考虑使用 RAII（如 QFile 的析构函数）来管理资源，减少手动管理的复杂性。

4. 宏定义说明：

   • 建议在代码或文档中说明 ENABLE_DSS_SNIPE 宏的具体用途和适用场景，便于后续维护。

优化后的代码示例

bool FullScreenBackground::getScaledBlurImage(const QString &originPath, QString &scaledPath)
{
    // 壁纸服务dde-wallpaper-cache
    QDBusInterface wallpaperCacheInterface("org.deepin.dde.WallpaperCache", "/org/deepin/dde/WallpaperCache",
                                           "org.deepin.dde.WallpaperCache", QDBusConnection::systemBus());
    if (!wallpaperCacheInterface.isValid()) {
        qWarning() << "dde-wallpaper-cache service not existed";
        return false;
    }

    QFile file(originPath);
    if (!file.open(QIODevice::ReadOnly)) {
        qWarning() << "open origin file failed:" << originPath;
        return false;
    }

    // 显式关闭文件句柄，避免资源泄漏
    file.close();

    QDBusReply<QStringList> pathList = wallpaperCacheInterface.call("RequestBlurImage", originPath);
    if (!pathList.isValid()) {
        qWarning() << "call RequestBlurImage failed:" << pathList.error().message();
        return false;
    }

    QString path = pathList.value().at(0);
    if (!path.isEmpty() && path != originPath) {
        scaledPath = path;
    }

    return true;
}

总结

这段代码的改进方向是正确的，尤其是在文件句柄管理和条件编译逻辑上。通过统一服务检测方式和增强错误处理，可以进一步提升代码的健壮性和可维护性。

[deepin-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: deepin-ci-robot, yixinshark

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