-
Notifications
You must be signed in to change notification settings - Fork 50
fix: 修复删除VPN后任务栏图标没有变化的问题 #475
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
删除VPN的时候,会先删除节点,删除节点会先断开信号槽函数的连接,然后才发送VPN状态变化的信号,由于此时信号已经断开,发送的信号无法收到,所以状态没有变化 Log: 修复删除VPN后任务栏图标没有变化的问题 Influence: 从控制中心删除已经连接的VPN,观察任务栏图标中是否还存在VPN图标 Bug: https://pms.uniontech.com/bug-view-341927.html
Reviewer's guide (collapsed on small PRs)Reviewer's GuideReconnects and propagates VPN status change signals from NetManagerPrivate through NetManager to NetStatus so that the taskbar icon updates correctly when a connected VPN is deleted. Sequence diagram for VPN deletion status propagationsequenceDiagram
actor ControlCenterUser
participant ControlCenter
participant NetManager as NetManager
participant NetManagerPrivate as NetManagerPrivate
participant NetStatus as NetStatus
participant TaskbarIcon as TaskbarIcon
ControlCenterUser ->> ControlCenter: deleteVPN(vpnId)
ControlCenter ->> NetManagerPrivate: removeVpnConnection(vpnId)
NetManagerPrivate ->> NetManagerPrivate: disconnectSignalsForVpn(vpnId)
NetManagerPrivate ->> NetManagerPrivate: performVpnRemoval(vpnId)
NetManagerPrivate -->> NetManager: vpnStatusChanged()
NetManager -->> NetStatus: vpnStatusChanged()
NetStatus ->> NetStatus: updateVpnAndProxyStatus()
NetStatus -->> TaskbarIcon: setVpnStatus(disconnected)
Class diagram for updated VPN status change signal chainclassDiagram
class NetManagerPrivate {
+request()
+vpnStatusChanged()
+init(flags)
+removeVpnConnection(vpnId)
}
class NetManager {
-QScopedPointer_NetManagerPrivate d_ptrNetManager
+NetManager(flags, parent)
+request()
+vpnStatusChanged()
+toControlCenter()
+netCheckAvailableChanged(netCheckAvailable)
+primaryConnectionTypeChanged(type)
}
class NetStatus {
+onChildAdded(child)
+updateVpnAndProxyStatus()
}
class NetDeviceItem {
+nameChanged()
+enabledableChanged()
+enabledChanged()
}
NetManagerPrivate <|-- NetManager : has_pointer_to
NetManagerPrivate --> NetManager : emits_vpnStatusChanged
NetManager --> NetStatus : emits_vpnStatusChanged
NetStatus --> NetDeviceItem : connects_signals
NetDeviceItem --> NetStatus : nameChanged
NetDeviceItem --> NetStatus : enabledableChanged
NetDeviceItem --> NetStatus : enabledChanged
NetStatus --> NetStatus : updateVpnAndProxyStatus_calls
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
deepin pr auto review这段代码主要涉及在 Qt 框架中启用 VPN 状态变更信号的连接。以下是对这段 1. 语法逻辑
2. 代码质量
3. 代码性能
4. 代码安全
总结这段代码的主要目的是打通 VPN 状态变更的通知链路。整体实现符合 Qt 规范,逻辑正确。主要的改进点在于关注 |
There was a problem hiding this 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:
- In
NetStatus::onChildAdded, the connection fromm_manager'svpnStatusChangedtoupdateVpnAndProxyStatuswill be created every time aVpnControlItemis added, which can lead to the slot being invoked multiple times for a single signal; consider usingQt::UniqueConnectionor moving this connection to a place where it is made only once.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `NetStatus::onChildAdded`, the connection from `m_manager`'s `vpnStatusChanged` to `updateVpnAndProxyStatus` will be created every time a `VpnControlItem` is added, which can lead to the slot being invoked multiple times for a single signal; consider using `Qt::UniqueConnection` or moving this connection to a place where it is made only once.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: caixr23, ut003640 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/forcemerge |
|
This pr force merged! (status: unstable) |
删除VPN的时候,会先删除节点,删除节点会先断开信号槽函数的连接,然后才发送VPN状态变化的信号,由于此时信号已经断开,发送的信号无法收到,所以状态没有变化
Log: 修复删除VPN后任务栏图标没有变化的问题
Influence: 从控制中心删除已经连接的VPN,观察任务栏图标中是否还存在VPN图标
Bug: https://pms.uniontech.com/bug-view-341927.html
Summary by Sourcery
Bug Fixes: