Skip to content

feat: pass launch_type when launching apps from launcher#757

Merged
Ivy233 merged 1 commit intolinuxdeepin:masterfrom
Ivy233:feat/launch-type-eventlog
May 8, 2026
Merged

feat: pass launch_type when launching apps from launcher#757
Ivy233 merged 1 commit intolinuxdeepin:masterfrom
Ivy233:feat/launch-type-eventlog

Conversation

@Ivy233
Copy link
Copy Markdown
Contributor

@Ivy233 Ivy233 commented May 8, 2026

启动器启动应用时传入 launch_type 参数,用于应用启动埋点统计。
launch_type=1 表示从启动器点击启动。

Pass launch_type parameter when launching applications from launcher for application launch event reporting. launch_type=1 indicates the app was launched from the launcher.

PMS: TASK-388657

Summary by Sourcery

New Features:

  • Include a launch_type=1 parameter when invoking dde-am for apps started from the launcher to enable launch analytics.

@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented May 8, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Adds a launch_type parameter when the launcher starts apps so that downstream components can distinguish launcher-initiated launches for analytics/telemetry.

Sequence diagram for launcher-initiated app start with launch_type

sequenceDiagram
    actor User
    participant LauncherAppMgr as Launcher_AppMgr
    participant QProcess
    participant DdeAm as dde_am

    User ->> LauncherAppMgr: launchApp(desktopId)
    activate LauncherAppMgr
    LauncherAppMgr ->> LauncherAppMgr: resolve amAppIface and path
    LauncherAppMgr ->> QProcess: setProcessChannelMode(MergedChannels)
    LauncherAppMgr ->> QProcess: start(dde-am, [--by-user, --launch-type, 1, path])
    activate QProcess
    QProcess ->> DdeAm: execute dde-am with args
    DdeAm -->> QProcess: process launch result
    QProcess -->> LauncherAppMgr: waitForFinished result
    deactivate QProcess
    LauncherAppMgr ->> LauncherAppMgr: check success and log if failed
    deactivate LauncherAppMgr
Loading

Class diagram for AppMgr.launchApp change to include launch_type

classDiagram
    class AppMgr {
        +bool launchApp(desktopId: QString)
    }

    class QProcess {
        +setProcessChannelMode(mode: ProcessChannelMode) void
        +start(program: QString, arguments: QStringList) void
        +waitForFinished() bool
        +errorString() QString
    }

    class AmAppIface {
        +path() QString
    }

    AppMgr --> QProcess : uses
    AppMgr --> AmAppIface : uses

    note for AppMgr "launchApp now calls QProcess.start with arguments [--by-user, --launch-type, 1, path] to mark launcher-initiated launches"
Loading

File-Level Changes

Change Details Files
Include a fixed launch_type=1 argument when launching applications via dde-am from the launcher for telemetry.
  • Extend the dde-am process start arguments to include --launch-type and value 1 before the app path.
  • Preserve existing flags such as --by-user and existing process configuration, including merged channels and error handling.
src/ddeintegration/appmgr.cpp

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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.

Hey - I've left some high level feedback:

  • Consider replacing the hardcoded "1" launch_type value with a named constant or enum to make the meaning clearer and reduce the chance of mistakes if additional launch types are added later.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- Consider replacing the hardcoded "1" launch_type value with a named constant or enum to make the meaning clearer and reduce the chance of mistakes if additional launch types are added later.

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Ivy233 Ivy233 requested a review from BLumia May 8, 2026 05:03
@Ivy233 Ivy233 force-pushed the feat/launch-type-eventlog branch 2 times, most recently from dd96b55 to 12eab7c Compare May 8, 2026 10:37
启动器启动应用时传入 launch_type 参数,用于应用启动埋点统计。
launch_type=1 表示从启动器点击启动。

Pass launch_type parameter when launching applications from
launcher for application launch event reporting. launch_type=1
indicates the app was launched from the launcher.

PMS: TASK-388657
@Ivy233 Ivy233 force-pushed the feat/launch-type-eventlog branch from 12eab7c to 0c88e49 Compare May 8, 2026 10:47
@deepin-ci-robot
Copy link
Copy Markdown

deepin pr auto review

这段代码修改主要涉及对 dde-application-manager 依赖版本的升级,以及引入了新的 EventLogger 功能,用于记录应用启动事件。以下是对该代码的详细审查和改进建议:

1. 语法逻辑审查

代码在语法上是正确的,使用了标准的 CMake 和 C++ 语法。逻辑上,通过 HAVE_DDE_API_EVENTLOGGER 宏来控制是否启用事件日志记录功能,这是合理的条件编译方式。

潜在问题:

  • appmgr.cpp 中,process.start() 调用后直接使用 waitForFinished() 可能会导致主线程阻塞,尤其是在 dde-am 响应较慢的情况下。

2. 代码质量审查

优点:

  • 使用了条件编译来处理可选依赖,保证了向后兼容性。
  • 日志输出使用了 qCWarning,符合 Qt 的日志规范。

改进建议:

  • 错误处理不完整waitForFinished() 返回 false 时,仅输出了警告,但未检查进程的退出状态。建议增加对 process.exitCode()process.exitStatus() 的检查。
  • 硬编码命令dde-am 命令是硬编码的,建议将其定义为常量或配置项,便于维护。

3. 代码性能审查

  • 同步调用问题waitForFinished() 是同步阻塞调用,可能会影响 UI 响应。如果 dde-am 执行时间较长,建议改为异步方式(如使用 QProcess::finished 信号)。

4. 代码安全审查

  • 命令注入风险:虽然 path 来自 amAppIface->path(),但未验证其合法性。如果 path 包含特殊字符(如 ;, |),可能存在命令注入风险。建议对 path 进行校验或转义。
  • 依赖版本升级:将 dde-application-manager 版本从 1.2.2 升级到 1.2.51,需确保新版本没有破坏性变更。

5. 改进后的代码示例

以下是优化后的 appmgr.cpp 片段:

bool AppMgr::launchApp(const QString &desktopId)
{
    const auto path = amAppIface->path();
    
    // 校验 path 合法性
    if (path.isEmpty() || path.contains(QRegularExpression("[;|&`$]"))) {
        qCWarning(logDdeIntegration) << "Invalid path for desktopId:" << desktopId;
        return false;
    }

    QProcess process;
    process.setProcessChannelMode(QProcess::MergedChannels);

#ifdef HAVE_DDE_API_EVENTLOGGER
    process.start("dde-am", {"--by-user", "--launch-type", "dde-launchpad", path});
#else
    process.start("dde-am", {"--by-user", path});
#endif

    if (!process.waitForFinished(5000)) { // 设置超时时间
        qCWarning(logDdeIntegration) << "Failed to launch the desktopId:" << desktopId 
                                     << "Error:" << process.errorString();
        process.kill(); // 确保进程终止
        return false;
    }

    if (process.exitCode() != 0) {
        qCWarning(logDdeIntegration) << "dde-am exited with error:" << process.exitCode()
                                     << "Output:" << process.readAll();
        return false;
    }

    return true;
}

6. 其他建议

  • CMakeLists.txt:建议将 DDEAPI::EventLogger 的依赖检查放在更早的阶段,并明确其是否为必需依赖。
  • 日志记录:如果启用了 EventLogger,建议在日志中明确记录事件类型(如 dde-launchpad),便于后续分析。

总结

整体代码逻辑清晰,但需加强错误处理和安全性检查,同时考虑性能优化(避免阻塞主线程)。建议在升级依赖版本后进行充分的测试,确保兼容性。

@deepin-ci-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: BLumia, Ivy233

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

@Ivy233 Ivy233 merged commit c0161ac into linuxdeepin:master May 8, 2026
11 checks passed
@Ivy233 Ivy233 deleted the feat/launch-type-eventlog branch May 8, 2026 12:23
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