fix: app hang a few seconds when launch app for the first time

[BLumia]

修复启动器首次启动应用时,可能导致界面卡死几秒的问题.

[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。这次代码变更的主要意图是将DConfig的初始化和QDBusServiceWatcher的创建延迟到AppManager1服务就绪后进行，以避免在服务未就绪时进行无效操作。

整体思路上，这种延迟加载和按需初始化是很好的优化，但在代码逻辑、内存管理和性能方面存在一些需要改进的隐患。以下是我的详细审查意见：

1. 逻辑问题 - 内存泄漏与悬空指针隐患

问题代码：

auto serviceWatcher = new QDBusServiceWatcher(...);
connect(serviceWatcher, &QDBusServiceWatcher::serviceRegistered, this, [this, serviceWatcher]() {
    // ...
    serviceWatcher->disconnect();
});

分析：
在Lambda表达式中调用serviceWatcher->disconnect()仅仅是断开了信号与槽的连接，并没有释放serviceWatcher所占用的内存。由于serviceWatcher的父对象是this，它只有在AppMgr析构时才会被删除。如果在整个AppMgr生命周期内该服务一直未注册，或者服务注册后再次注销又重新注册，这不仅是内存泄漏，更严重的是如果未来重新连接信号，可能会导致逻辑混乱。

改进建议：
使用serviceWatcher->deleteLater()来安全地释放资源。由于deleteLater会在事件循环的下一轮销毁对象，此时Lambda已经执行完毕，因此是安全的。

connect(serviceWatcher, &QDBusServiceWatcher::serviceRegistered, this, [this, serviceWatcher]() {
    qCInfo(logDdeIntegration) << "AppManager1 service registered on bus, fetching app items";
    fetchAppItems();
    loadAppsLaunchedTimes();
    serviceWatcher->deleteLater(); // 安全释放内存
});

2. 性能与逻辑问题 - loadAppsLaunchedTimes() 可能被重复调用

问题代码：

if (isValid()) {
    qCInfo(logDdeIntegration) << "AppManager1 service already ready, fetching app items right away";
    fetchAppItems();
    loadAppsLaunchedTimes(); // 调用点 1
} else {
    auto serviceWatcher = new QDBusServiceWatcher(...);
    connect(serviceWatcher, ..., [this, ...]() {
        fetchAppItems();
        loadAppsLaunchedTimes(); // 调用点 2
        // ...
    });
}

分析：
在else分支中，当AppManager1服务注册上线时，会触发信号并调用loadAppsLaunchedTimes()。但是，QDBusServiceWatcher在创建时如果指定的服务已经存在，可能会立即触发serviceRegistered信号（Qt的行为是在事件循环中排队发送）。如果在某些极端时序下，isValid()判断与信号触发存在竞态，或者在AppMgr生命周期内服务重启，loadAppsLaunchedTimes()可能会被多次调用，导致DConfig被重复创建，信号被重复绑定。

改进建议：
增加一个状态标志位（如m_isLoaded），确保初始化逻辑只执行一次；或者确保DConfig的创建是幂等的。鉴于你将m_serviceWatcher成员变量去掉了，建议加一个简单的布尔标志位防止重复加载。

// 在 appmgr.h 中添加:
bool m_launchedTimesLoaded = false;

// 在 appmgr.cpp 中修改:
if (isValid()) {
    fetchAppItems();
    if (!m_launchedTimesLoaded) {
        loadAppsLaunchedTimes();
        m_launchedTimesLoaded = true;
    }
} else {
    // ...
    connect(serviceWatcher, &QDBusServiceWatcher::serviceRegistered, this, [this, serviceWatcher]() {
        fetchAppItems();
        if (!m_launchedTimesLoaded) {
            loadAppsLaunchedTimes();
            m_launchedTimesLoaded = true;
        }
        serviceWatcher->deleteLater();
    });
}

3. 代码质量 - DConfig 对象的生命周期管理

问题代码：

void AppMgr::loadAppsLaunchedTimes()
{
    DConfig *config = DConfig::create("org.deepin.dde.application-manager", "org.deepin.dde.am", "", this);
    if (!config->isValid()) {
        qCWarning(logDdeIntegration) << "DConfig is invalid when getting launched times.";
        // 注意：这里如果 config 无效，没有 return，也没有 delete config
    } else {
        // ...
    }
}

分析：

1. 如果config->isValid()为false，代码仅打印了警告，但没有提前返回，这会导致后续逻辑错误。同时，虽然config指定了this为父对象会随AppMgr析构而释放，但如果loadAppsLaunchedTimes被多次调用（如上一点所述），会产生多个DConfig实例挂在this下，浪费资源。
2. DConfig::create在无效时，是否一定返回非空指针并且可以安全地交给Qt对象树管理？最好在无效时显式清理或提前返回。

改进建议：
增加return语句，并考虑将config作为类成员变量，避免多次创建。

void AppMgr::loadAppsLaunchedTimes()
{
    DConfig *config = DConfig::create("org.deepin.dde.application-manager", "org.deepin.dde.am", "", this);
    if (!config->isValid()) {
        qCWarning(logDdeIntegration) << "DConfig is invalid when getting launched times.";
        config->deleteLater(); // 视 DConfig 实现而定，如果 create 返回的是 new 出的对象，建议清理
        return; // 必须返回
    }

    static const QString AppsLaunchedTimes(u8"appsLaunchedTimes");
    const auto &value = config->value(AppsLaunchedTimes).toMap();
    updateAppsLaunchedTimes(value);
    QObject::connect(config, &DConfig::valueChanged, this, [this, config](const QString &key) {
        if (key != AppsLaunchedTimes) {
            qCDebug(logDdeIntegration) << "Ignoring non-appsLaunchedTimes key:" << key;
            return;
        }

        qCInfo(logDdeIntegration) << "appsLaunchedTimes of DConfig changed, updating";
        const auto &value = config->value(AppsLaunchedTimes).toMap();
        updateAppsLaunchedTimes(value);
    });
}

更优的方案是将 DConfig *m_config 提升为类成员变量，在 loadAppsLaunchedTimes 中判断如果已创建则直接复用。

4. 代码安全 - Lambda 表达式捕获 this 的潜在风险

问题代码：

QObject::connect(config, &DConfig::valueChanged, this, [this, config](const QString &key) { ... });

分析：
Lambda捕获了this并连接到了config的信号。虽然connect的第3个参数指定了this作为context对象，意味着this被销毁时连接会自动断开，这是安全的。但是，如果loadAppsLaunchedTimes因为前面的逻辑问题被调用了两次，就会建立两个信号槽连接，导致updateAppsLaunchedTimes被触发两次。

改进建议：
结合第2点的防重复加载机制，可以避免此问题。如果未来有更复杂的逻辑，建议在连接前先disconnect一下相关的信号，或者使用QScopedPointer/成员变量来管理DConfig。

总结与重构建议

综合以上问题，我为你提供一份重构后的代码参考：

appmgr.h 修改部分：

private:
    void fetchAppItems();
    void loadAppsLaunchedTimes();
    void watchingAppItemAdded(const QString &key, AppMgr::AppItem *appItem);
    void watchingAppItemRemoved(const QString &key);
    void watchingAppItemPropertyChanged(const QString &key, AppMgr::AppItem *appItem);

private:
    __AppManager1ApplicationObjectManager *m_objectManager;
    QMap<QString, AppMgr::AppItem *> m_appItems;
    QMap<QString, AppMgr::AppItem *> m_pendingAppItems;
    QTimer *m_checkTimer;
    bool m_launchedTimesLoaded = false; // 新增：防重复加载标志

appmgr.cpp 修改部分：

// 构造函数中
if (isValid()) {
    qCInfo(logDdeIntegration) << "AppManager1 service already ready, fetching app items right away";
    fetchAppItems();
    loadAppsLaunchedTimes();
} else {
    auto serviceWatcher = new QDBusServiceWatcher(QStringLiteral("org.desktopspec.ApplicationManager1"),
                                           QDBusConnection::sessionBus(),
                                           QDBusServiceWatcher::WatchForRegistration,
                                           this);
    connect(serviceWatcher, &QDBusServiceWatcher::serviceRegistered, this, [this, serviceWatcher]() {
        qCInfo(logDdeIntegration) << "AppManager1 service registered on bus, fetching app items";
        fetchAppItems();
        loadAppsLaunchedTimes();
        serviceWatcher->deleteLater(); // 修复：安全释放 watcher
    });
}

// loadAppsLaunchedTimes 函数
void AppMgr::loadAppsLaunchedTimes()
{
    if (m_launchedTimesLoaded) { // 防止重复加载
        return;
    }
    m_launchedTimesLoaded = true;

    DConfig *config = DConfig::create("org.deepin.dde.application-manager", "org.deepin.dde.am", "", this);
    if (!config->isValid()) {
        qCWarning(logDdeIntegration) << "DConfig is invalid when getting launched times.";
        config->deleteLater(); // 清理无效的 config 对象
        return; // 修复：提前返回
    }

    static const QString AppsLaunchedTimes(u8"appsLaunchedTimes");
    const auto &value = config->value(AppsLaunchedTimes).toMap();
    updateAppsLaunchedTimes(value);
    
    QObject::connect(config, &DConfig::valueChanged, this, [this, config](const QString &key) {
        if (key != AppsLaunchedTimes) {
            qCDebug(logDdeIntegration) << "Ignoring non-appsLaunchedTimes key:" << key;
            return;
        }

        qCInfo(logDdeIntegration) << "appsLaunchedTimes of DConfig changed, updating";
        const auto &value = config->value(AppsLaunchedTimes).toMap();
        updateAppsLaunchedTimes(value);
    });
}

这些修改将提升代码的健壮性，防止内存泄漏，并解决潜在的重复初始化问题。希望这些意见对你有所帮助！

[18202781743]

是不是可以直接deleteLater它呀，

[BLumia]

有道理...合早了 😂

[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