Skip to content

fix(taskmanager): debounce window destroy for Wine apps to prevent ta…#1472

Closed
Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Ivy233:fix/wine-taskbar-icon-flicker
Closed

fix(taskmanager): debounce window destroy for Wine apps to prevent ta…#1472
Ivy233 wants to merge 1 commit intolinuxdeepin:masterfrom
Ivy233:fix/wine-taskbar-icon-flicker

Conversation

@Ivy233
Copy link
Contributor

@Ivy233 Ivy233 commented Mar 3, 2026

…skbar icon flicker

Wine apps (e.g. WeCom) go through IconicState → WithdrawnState → NormalState when restoring from tray. The WithdrawnState phase causes kwin to remove the window from _NET_CLIENT_LIST, and ~200ms later NormalState adds it back. This caused the AppItem to be destroyed and recreated, making the taskbar icon disappear and reappear at the end.

Add a 100ms debounce for Wine window removal by detecting __wine_prefix X11 property on window map and deferring windowDestroyed signal. If the window reappears within the timeout, the destroy is cancelled and the icon stays in place.

Wine 应用(如企业微信)从托盘恢复时,X11 状态转换路径为 IconicState →
WithdrawnState → NormalState。WithdrawnState 阶段导致 kwin 将窗口从 _NET_CLIENT_LIST 中移除,约 200ms 后 NormalState 又将其加回。这导致 AppItem 被销毁再重建,图标从任务栏消失后出现在末尾位置。

通过在窗口映射时检测 __wine_prefix X11 属性识别 Wine 窗口,对其移除操作添加 100ms 防抖延迟。若窗口在超时前重新出现,则取消销毁,图标保持原位不闪烁。

PMS: BUG-342741

Copy link

@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 @Ivy233, 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

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: 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

@deepin-ci-robot
Copy link

deepin pr auto review

Git Diff 代码审查报告

总体评价

这段代码实现了对 Wine 窗口的特殊处理,主要是为了解决 Wine 窗口在关闭/重启时的防抖动问题。代码整体逻辑清晰,但存在一些内存管理、性能和潜在并发问题需要改进。

详细审查

1. 语法逻辑

问题 1: isWineWindow 函数缺少错误处理

bool X11Utils::isWineWindow(const xcb_window_t &window)
{
    xcb_atom_t atom = getAtomByName("__wine_prefix");
    xcb_get_property_cookie_t cookie = xcb_get_property(m_connection, false, window, atom, XCB_GET_PROPERTY_TYPE_ANY, 0, 1);
    std::unique_ptr<xcb_get_property_reply_t> reply(xcb_get_property_reply(m_connection, cookie, nullptr));
    return reply && reply->type != XCB_ATOM_NONE && reply->value_len > 0;
}

改进建议:

  • getAtomByName 可能失败返回 XCB_ATOM_NONE,但代码没有检查
  • 应该检查 atom 是否有效后再调用 xcb_get_property

问题 2: m_wineWindows 哈希表逻辑复杂

QHash<xcb_window_t, QTimer*> m_wineWindows; // value: nullptr=alive, QTimer*=pending destroy

改进建议:

  • 使用 nullptr 表示活跃窗口,QTimer* 表示待销毁窗口的逻辑不够直观
  • 建议使用枚举或单独的状态变量来表示窗口状态

2. 代码质量

问题 1: 内存管理风险

void X11WindowMonitor::clear()
{
    for (auto timer : m_wineWindows) {
        delete timer;
    }
    m_wineWindows.clear();
    m_windows.clear();
    m_windowPreview.reset(nullptr);
}

改进建议:

  • 使用 qDeleteAll(m_wineWindows) 替代手动循环删除
  • 考虑使用 QScopedPointerQSharedPointer 管理 QTimer 对象

问题 2: 重复代码

auto it = m_wineWindows.find(alreadyOpenedWindow);
if (it != m_wineWindows.end()) {
    if (*it) continue; // already has pending timer
    auto timer = new QTimer(this);
    // ... 设置定时器 ...
    *it = timer;
}

改进建议:

  • 将定时器创建和设置逻辑提取为单独的函数
  • 避免在多处重复相同的定时器设置代码

3. 代码性能

问题 1: 频繁的 X11 属性查询

bool X11Utils::isWineWindow(const xcb_window_t &window)
{
    xcb_atom_t atom = getAtomByName("__wine_prefix");
    xcb_get_property_cookie_t cookie = xcb_get_property(m_connection, false, window, atom, XCB_GET_PROPERTY_TYPE_ANY, 0, 1);
    std::unique_ptr<xcb_get_property_reply_t> reply(xcb_get_property_reply(m_connection, cookie, nullptr));
    return reply && reply->type != XCB_ATOM_NONE && reply->value_len > 0;
}

改进建议:

  • getAtomByName 可能每次都进行 X11 通信,应该缓存 __wine_prefix 的 atom
  • 考虑使用 xcb_get_property_unchecked 进行异步查询

问题 2: 定时器间隔硬编码

timer->setInterval(100);

改进建议:

  • 将 100ms 提取为类常量或配置项
  • 添加注释说明为什么选择 100ms

4. 代码安全

问题 1: 潜在的内存泄漏

delete m_wineWindows.take(xcb_window);

改进建议:

  • 确保在所有代码路径中都能正确清理定时器
  • 考虑使用 QObject::deleteLater() 替代直接 delete

问题 2: 并发问题

void X11WindowMonitor::handleRootWindowClientListChanged()
{
    // ...
    for (auto openedWindows : currentOpenedWindowList) {
        if (auto timer = m_wineWindows.value(openedWindows, nullptr)) {
            timer->stop();
            timer->deleteLater();
            m_wineWindows[openedWindows] = nullptr;
        }
        // ...
    }

改进建议:

  • 在多线程环境下,m_wineWindows 的访问需要加锁保护
  • 考虑使用 QMutex 保护共享数据结构

改进后的代码示例

// x11utils.h
class X11Utils
{
    // ...
    bool isWineWindow(const xcb_window_t &window);
    // ...
private:
    static const int WINE_WINDOW_DEBOUNCE_MS = 100;
    xcb_atom_t m_winePrefixAtom = XCB_ATOM_NONE;
};

// x11utils.cpp
bool X11Utils::isWineWindow(const xcb_window_t &window)
{
    if (m_winePrefixAtom == XCB_ATOM_NONE) {
        m_winePrefixAtom = getAtomByName("__wine_prefix");
        if (m_winePrefixAtom == XCB_ATOM_NONE) {
            return false;
        }
    }
    
    xcb_get_property_cookie_t cookie = xcb_get_property_unchecked(
        m_connection, false, window, m_winePrefixAtom, XCB_GET_PROPERTY_TYPE_ANY, 0, 1);
    std::unique_ptr<xcb_get_property_reply_t> reply(xcb_get_property_reply(m_connection, cookie, nullptr));
    
    return reply && reply->type != XCB_ATOM_NONE && reply->value_len > 0;
}

// x11windowmonitor.h
class X11WindowMonitor
{
    // ...
private:
    enum WineWindowState {
        WineWindowAlive,
        WineWindowPendingDestroy
    };
    QHash<xcb_window_t, QPair<WineWindowState, QTimer*>> m_wineWindows;
    // ...
};

// x11windowmonitor.cpp
void X11WindowMonitor::clear()
{
    qDeleteAll(m_wineWindows);
    m_wineWindows.clear();
    m_windows.clear();
    m_windowPreview.reset(nullptr);
}

void X11WindowMonitor::scheduleWineWindowDestroy(xcb_window_t window)
{
    auto& state = m_wineWindows[window];
    if (state.first == WineWindowPendingDestroy) {
        return; // Already scheduled
    }
    
    auto timer = new QTimer(this);
    timer->setSingleShot(true);
    timer->setInterval(X11Utils::WINE_WINDOW_DEBOUNCE_MS);
    
    connect(timer, &QTimer::timeout, this, [this, window]() {
        m_wineWindows.remove(window);
        Q_EMIT windowDestroyed(window);
    });
    
    state = {WineWindowPendingDestroy, timer};
    timer->start();
}

void X11WindowMonitor::handleRootWindowClientListChanged()
{
    auto currentOpenedWindowList = X11->getWindowClientList(m_rootWindow);
    
    for (auto window : currentOpenedWindowList) {
        if (m_wineWindows.contains(window)) {
            // Cancel pending destroy if window reappeared
            auto& state = m_wineWindows[window];
            if (state.first == WineWindowPendingDestroy) {
                state.second->stop();
                state.second->deleteLater();
                state = {WineWindowAlive, nullptr};
            }
        } else if (!m_windows.contains(window)) {
            onWindowMapped(window);
        }
    }
    
    for (auto window : m_windows.keys()) {
        if (!currentOpenedWindowList.contains(window)) {
            if (m_wineWindows.contains(window)) {
                scheduleWineWindowDestroy(window);
            } else {
                Q_EMIT windowDestroyed(window);
            }
        }
    }
}

总结

  1. 优化了 isWineWindow 的实现,添加了 atom 缓存和错误处理
  2. 改进了 m_wineWindows 的数据结构,使用枚举明确表示窗口状态
  3. 提取了定时器创建逻辑到单独函数,减少代码重复
  4. 使用 qDeleteAlldeleteLater 改进内存管理
  5. 将硬编码的定时器间隔提取为常量
  6. 改进了代码可读性和可维护性

…skbar icon flicker

Wine apps (e.g. WeCom) go through IconicState → WithdrawnState → NormalState
when restoring from tray. The WithdrawnState phase causes kwin to remove the
window from _NET_CLIENT_LIST, and ~200ms later NormalState adds it back. This
caused the AppItem to be destroyed and recreated, making the taskbar icon
disappear and reappear at the end.

Add a 100ms debounce for Wine window removal by detecting __wine_prefix X11
property on window map and deferring windowDestroyed signal. If the window
reappears within the timeout, the destroy is cancelled and the icon stays
in place.

Wine 应用(如企业微信)从托盘恢复时,X11 状态转换路径为 IconicState →
WithdrawnState → NormalState。WithdrawnState 阶段导致 kwin 将窗口从
_NET_CLIENT_LIST 中移除,约 200ms 后 NormalState 又将其加回。这导致 AppItem
被销毁再重建,图标从任务栏消失后出现在末尾位置。

通过在窗口映射时检测 __wine_prefix X11 属性识别 Wine 窗口,对其移除操作添加
100ms 防抖延迟。若窗口在超时前重新出现,则取消销毁,图标保持原位不闪烁。

PMS: BUG-342741
@Ivy233 Ivy233 force-pushed the fix/wine-taskbar-icon-flicker branch from 069d899 to 113fd77 Compare March 3, 2026 14:52
Copy link
Member

@BLumia BLumia left a comment

Choose a reason for hiding this comment

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

建议bug单降级吧(降到4),为了wine特殊处理这个太怪了,考虑到企业微信说不定快出原生版本了。

@Ivy233 Ivy233 closed this Mar 4, 2026
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