Skip to content

Conversation

@tsic404
Copy link
Contributor

@tsic404 tsic404 commented Dec 23, 2024

The size of the popup window is not initialized and limited to the maximum value, resulting in excessive memory usage due to excessive size in some cases

log: as title

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tsic404, yixinshark

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

The size of the popup window is not initialized and limited to the
maximum value, resulting in excessive memory usage due to excessive
size in some cases

log: as title
@deepin-ci-robot
Copy link

deepin pr auto review

代码审查意见:

  1. PopupWindow类中,setMaximumSize函数被定义为一个lambda表达式,并在screenChanged信号连接后立即调用。这种做法是合理的,但可以考虑将setMaximumSize函数提取为一个独立的成员函数,以提高代码的可读性和可维护性。

  2. PopupWindow类的构造函数中,setMinimumSize被设置为QSize(1, 1),这可能是一个临时解决方案,以防止某些协议错误。如果这个解决方案不是必要的,应该移除它,以避免不必要的限制。

  3. PluginSurfacePluginPopup类的构造函数中,新增了m_heightm_width成员变量,并初始化为1。这种做法可能会影响窗口的大小和布局,应该确认这是否是预期的行为。如果这是必要的,建议添加相应的注释说明原因。

  4. PluginSurfacePluginPopup类的构造函数中,init函数被调用,但没有看到init函数的定义。如果init函数是必要的,应该确保它被正确实现,并且不会引入任何潜在的问题。

  5. PluginSurface::plugin_source_size函数中,widthheight参数被直接用于设置窗口大小。如果这些参数的值可能不正确,应该添加相应的检查和错误处理逻辑。

  6. PluginSurface::PluginSurfacePluginPopup::PluginPopup构造函数中,m_managerm_surfacem_pluginIdm_itemKeym_popupType等成员变量被初始化,但没有看到这些成员变量的使用。如果这些成员变量是必要的,应该确保它们在类的其他部分被正确使用。

  7. 代码中没有看到对QWaylandSurfaceQWaylandResource对象的任何错误处理。如果这些对象可能为空,应该添加相应的检查和错误处理逻辑。

  8. 代码中没有看到对QWindow::screen()返回的QScreen对象的任何错误处理。如果QScreen对象可能为空,应该添加相应的检查和错误处理逻辑。

  9. 代码中没有看到对QWindow::screenChanged信号的处理逻辑。如果这个信号的处理逻辑是必要的,应该确保它被正确实现,并且不会引入任何潜在的问题。

  10. 代码中没有看到对QWindow::screenChanged信号的处理逻辑。如果这个信号的处理逻辑是必要的,应该确保它被正确实现,并且不会引入任何潜在的问题。

@yixinshark
Copy link
Contributor

/forcemerge

@deepin-bot
Copy link

deepin-bot bot commented Dec 23, 2024

This pr force merged! (status: blocked)

@deepin-bot deepin-bot bot merged commit 677bf5e into linuxdeepin:master Dec 23, 2024
7 of 10 checks passed
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