fix: sometimes dock might be empty after boot and login #1260
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR adds a readiness signal from the application manager model through the AppsApplet to the DockGlobalElementModel, ensuring that dock elements are loaded only after the app model is fully initialized, preventing empty dock states. It implements a new Sequence diagram for dock initialization with app model readiness signalsequenceDiagram
participant DockGlobalElementModel
participant DAppletBridge
participant AppsApplet
participant AMAppItemModel
DockGlobalElementModel->>DAppletBridge: Get AppsApplet instance
DAppletBridge->>AppsApplet: Provide applet()
AppsApplet->>AMAppItemModel: Initialize
AMAppItemModel-->>AppsApplet: readyChanged(true)
AppsApplet-->>DockGlobalElementModel: appModelReadyChanged(true)
DockGlobalElementModel->>DockGlobalElementModel: initDockedElements(true)
DockGlobalElementModel->>DockGlobalElementModel: loadDockedElements()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- In AMAppItemModel, setting the "ready" property via setProperty won’t update m_ready nor emit readyChanged(), so make sure you assign m_ready = true and emit readyChanged(true) when the model is ready.
- The initial invokeMethod(loadDockedElements) call may still fire before the app model is actually ready, causing duplicate or early loads—consider removing that queued call or guarding it so elements only load once the ready signal arrives.
- Add error handling or logging for the case where DAppletBridge.applet() returns null to surface issues instead of silently skipping the dock initialization.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In AMAppItemModel, setting the "ready" property via setProperty won’t update m_ready nor emit readyChanged(), so make sure you assign m_ready = true and emit readyChanged(true) when the model is ready.
- The initial invokeMethod(loadDockedElements) call may still fire before the app model is actually ready, causing duplicate or early loads—consider removing that queued call or guarding it so elements only load once the ready signal arrives.
- Add error handling or logging for the case where DAppletBridge.applet() returns null to surface issues instead of silently skipping the dock initialization.
## Individual Comments
### Comment 1
<location> `applets/dde-apps/amappitemmodel.cpp:65-66` </location>
<code_context>
}
}
+
+ setProperty("ready", true);
+ qCDebug(appsLog) << "AMAppItemModel is now ready with apps counts:" << rowCount();
});
}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Directly setting the 'ready' property may bypass signal emission.
Use a setter for 'm_ready' that emits 'readyChanged' to ensure listeners are notified of state changes.
Suggested implementation:
```cpp
setReady(true);
qCDebug(appsLog) << "AMAppItemModel is now ready with apps counts:" << rowCount();
```
```cpp
void AMAppItemModel::setReady(bool ready)
{
if (m_ready == ready)
return;
m_ready = ready;
emit readyChanged(m_ready);
}
bool AMAppItemModel::ready() const
{
return m_ready;
```
You must ensure that:
1. The `setReady(bool ready)` method is declared in the header file (`amappitemmodel.h`).
2. The `readyChanged(bool)` signal is declared in the header file if not already present.
3. Any code relying on the property or signal is updated to use the setter and signal as appropriate.
</issue_to_address>
### Comment 2
<location> `applets/dde-apps/amappitemmodel.h:16` </location>
<code_context>
{
Q_OBJECT
-
+ Q_PROPERTY(bool ready MEMBER m_ready READ ready NOTIFY readyChanged)
public:
explicit AMAppItemModel(QObject *parent = nullptr);
</code_context>
<issue_to_address>
**suggestion:** Using MEMBER in Q_PROPERTY with custom signal may not emit signal automatically.
To ensure 'readyChanged' is emitted when 'm_ready' changes, update 'm_ready' only via a setter that emits the signal.
Suggested implementation:
```c
Q_PROPERTY(bool ready READ ready WRITE setReady NOTIFY readyChanged)
```
```c
signals:
void readyChanged(bool);
public:
void setReady(bool ready);
private:
bool m_ready;
```
You must implement the `setReady(bool ready)` method in the corresponding `.cpp` file. The implementation should look like:
void AMAppItemModel::setReady(bool ready)
{
if (m_ready == ready)
return;
m_ready = ready;
emit readyChanged(m_ready);
}
Also, ensure all updates to `m_ready` use `setReady()` instead of direct assignment.
</issue_to_address>
### Comment 3
<location> `panels/dock/taskmanager/dockglobalelementmodel.cpp:153-155` </location>
<code_context>
}
});
+ DS_NAMESPACE::DAppletBridge ddeAppsBridge("org.deepin.ds.dde-apps");
+ auto ddeAppsApplet = ddeAppsBridge.applet();
+ if (ddeAppsApplet) {
+ connect(ddeAppsApplet, SIGNAL(appModelReadyChanged(bool)), this, SLOT(initDockedElements(bool)));
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Using old SIGNAL/SLOT syntax may miss compile-time checks.
Switch to the Qt5 signal-slot syntax for improved type safety: connect(ddeAppsApplet, &AppsApplet::appModelReadyChanged, this, &DockGlobalElementModel::initDockedElements);
```suggestion
if (ddeAppsApplet) {
connect(ddeAppsApplet, &AppsApplet::appModelReadyChanged, this, &DockGlobalElementModel::initDockedElements);
}
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+65
to
+66
| setProperty("ready", true); | ||
| qCDebug(appsLog) << "AMAppItemModel is now ready with apps counts:" << rowCount(); |
There was a problem hiding this comment.
suggestion (bug_risk): Directly setting the 'ready' property may bypass signal emission.
Use a setter for 'm_ready' that emits 'readyChanged' to ensure listeners are notified of state changes.
Suggested implementation:
setReady(true);
qCDebug(appsLog) << "AMAppItemModel is now ready with apps counts:" << rowCount();
void AMAppItemModel::setReady(bool ready)
{
if (m_ready == ready)
return;
m_ready = ready;
emit readyChanged(m_ready);
}
bool AMAppItemModel::ready() const
{
return m_ready;
You must ensure that:
- The
setReady(bool ready)method is declared in the header file (amappitemmodel.h). - The
readyChanged(bool)signal is declared in the header file if not already present. - Any code relying on the property or signal is updated to use the setter and signal as appropriate.
| { | ||
| Q_OBJECT | ||
|
|
||
| Q_PROPERTY(bool ready MEMBER m_ready READ ready NOTIFY readyChanged) |
There was a problem hiding this comment.
suggestion: Using MEMBER in Q_PROPERTY with custom signal may not emit signal automatically.
To ensure 'readyChanged' is emitted when 'm_ready' changes, update 'm_ready' only via a setter that emits the signal.
Suggested implementation:
Q_PROPERTY(bool ready READ ready WRITE setReady NOTIFY readyChanged)signals:
void readyChanged(bool);
public:
void setReady(bool ready);
private:
bool m_ready;You must implement the setReady(bool ready) method in the corresponding .cpp file. The implementation should look like:
void AMAppItemModel::setReady(bool ready)
{
if (m_ready == ready)
return;
m_ready = ready;
emit readyChanged(m_ready);
}
Also, ensure all updates to m_ready use setReady() instead of direct assignment.
Member
Author
|
测试包： |
robertkill
previously approved these changes
Sep 22, 2025
18202781743
reviewed
Sep 22, 2025
18202781743
reviewed
Sep 22, 2025
18202781743
reviewed
Sep 22, 2025
| } | ||
| } | ||
|
|
||
| setProperty("ready", true); |
Contributor
There was a problem hiding this comment.
这种行为，有没有model提供的机制呀，初始化的时候，是初始化里面的数据，能使用beginResetModel这种么，
Member
Author
There was a problem hiding this comment.
可以用reset，相当于要改model初始化那边填入数据时的形式，暂时没排查有没有别的影响。
Member
Author
There was a problem hiding this comment.
QStandardItemModel 没提供比较方便的 reset 的方式，加上暂时未排查是否有别的影响，暂时不改为完全依赖 reset 状态来载入任务栏驻留状态的写法。
部分情况下,dock驻留区域的加载早于 AM 对应的模型中的数据就绪,导致驻留 加载完毕后因为有效性检查而被过滤,致使托盘区域原本驻留的应用全部未展 示. 此提交新增了一个就绪信号,以供在更准确的时机通知驻留信息的加载. PMS: BUG-334171 Log:
deepin pr auto review根据提供的git diff，我来对代码进行审查，并提出改进意见：
优点：
改进建议：
优点：
改进建议：
优点：
改进建议：
优点：
改进建议：
总结： |
18202781743
approved these changes
Sep 23, 2025
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
部分情况下,dock驻留区域的加载早于 AM 对应的模型中的数据就绪,导致驻留加载完毕后因为有效性检查而被过滤,致使托盘区域原本驻留的应用全部未展示. 此提交新增了一个就绪信号,以供在更准确的时机通知驻留信息的加载.
Summary by Sourcery
Delay loading of docked elements until the application model signals readiness to prevent the dock from appearing empty after boot or login.
New Features:
Bug Fixes:
Enhancements: