Fix a possible deadlock in OpenXRRenderLoop [chromium/src : main]
0 views
Skip to first unread message
Yong Li (xWF) (Gerrit)
May 16, 2026, 1:59:44 PMMay 16
to Alexander Cooper, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Alexander Cooper and Brandon Jones
Yong Li (xWF) added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 1 (Latest):
Yong Li (xWF) . resolved
I also got a browsertest that can reproduce this problem. https://chromium-review.git.corp.google.com/c/chromium/src/+/7849217
Related details
Attention is currently required from:
- Alexander Cooper
- Brandon Jones
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Cooper (Gerrit)
May 18, 2026, 1:38:34 PMMay 18
to Yong Li (xWF), Chromium LUCI CQ, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Brandon Jones and Yong Li (xWF)
Alexander Cooper added 2 comments![]( "Open in Gerrit")
File device/vr/openxr/openxr_render_loop.cc
Line 161, Patchset 2 (Latest):
// If the pending frame is already used for last WebXR Frame, we must be
// waiting for the overlay texture to be submitted for the last frame. DoAlexander Cooper . unresolved
Why not `waiting_for_overlay_` from the frame here?
Line 166, Patchset 2 (Latest):
if ((!webxr_visible_ && !on_webxr_submitted_) || webxr_has_pose_ ||
(pending_frame_ && !pending_frame_->frame_data_)) {Alexander Cooper . unresolved
Semi-existing, but this is quite the conditional. Can we break this into helper bools with their own comments/better explanatory names?
Related details
Attention is currently required from:
- Brandon Jones
- Yong Li (xWF)
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
May 19, 2026, 8:32:31 AMMay 19
to Chromium LUCI CQ, Alexander Cooper, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Alexander Cooper and Brandon Jones
Yong Li (xWF) added 2 comments![]( "Open in Gerrit")
File device/vr/openxr/openxr_render_loop.cc
Line 161, Patchset 2 (Latest):
// If the pending frame is already used for last WebXR Frame, we must be
// waiting for the overlay texture to be submitted for the last frame. DoAlexander Cooper . unresolved
Why not `waiting_for_overlay_` from the frame here?
Yong Li (xWF)
I can assert waiting_for_overlay_, but I try to prevent it from sending a null frame data to renderer. Another solution is reset webxr_has_pose_ in RequestNextOverlayPose when pending frame is null. But I feel it is still better to guarantee frame data is always good to send.
Line 166, Patchset 2 (Latest):
if ((!webxr_visible_ && !on_webxr_submitted_) || webxr_has_pose_ ||
(pending_frame_ && !pending_frame_->frame_data_)) {Alexander Cooper . unresolved
Semi-existing, but this is quite the conditional. Can we break this into helper bools with their own comments/better explanatory names?
Attention is currently required from:
- Alexander Cooper
- Brandon Jones
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
May 19, 2026, 10:15:39 AMMay 19
to Chromium LUCI CQ, Alexander Cooper, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Alexander Cooper and Brandon Jones
Yong Li (xWF) added 1 comment![]( "Open in Gerrit")
File device/vr/openxr/openxr_render_loop.cc
Line 161, Patchset 2 (Latest):
// If the pending frame is already used for last WebXR Frame, we must be
// waiting for the overlay texture to be submitted for the last frame. DoAlexander Cooper . unresolved
Yong Li (xWF)Why not `waiting_for_overlay_` from the frame here?
I can assert waiting_for_overlay_, but I try to prevent it from sending a null frame data to renderer. Another solution is reset webxr_has_pose_ in RequestNextOverlayPose when pending frame is null. But I feel it is still better to guarantee frame data is always good to send.
Yong Li (xWF)
Actually I cannot do waiting_for_overlay_ here, because that doesn't mean we should delay GetFrameData. waiting_for_overlay_ can be true when pending_frame_ has valid frame data and we should proceed.
Yong Li (xWF) (Gerrit)
May 19, 2026, 10:31:00 AMMay 19
to Chromium LUCI CQ, Alexander Cooper, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Alexander Cooper and Brandon Jones
Yong Li (xWF) added 2 comments![]( "Open in Gerrit")
File device/vr/openxr/openxr_render_loop.cc
Line 161, Patchset 2:
// If the pending frame is already used for last WebXR Frame, we must be
// waiting for the overlay texture to be submitted for the last frame. DoAlexander Cooper . resolved
Yong Li (xWF)Why not `waiting_for_overlay_` from the frame here?
Yong Li (xWF)I can assert waiting_for_overlay_, but I try to prevent it from sending a null frame data to renderer. Another solution is reset webxr_has_pose_ in RequestNextOverlayPose when pending frame is null. But I feel it is still better to guarantee frame data is always good to send.
Actually I cannot do waiting_for_overlay_ here, because that doesn't mean we should delay GetFrameData. waiting_for_overlay_ can be true when pending_frame_ has valid frame data and we should proceed.
Yong Li (xWF)
Done
Line 166, Patchset 2:
if ((!webxr_visible_ && !on_webxr_submitted_) || webxr_has_pose_ ||
(pending_frame_ && !pending_frame_->frame_data_)) {Alexander Cooper . resolved
Yong Li (xWF)Semi-existing, but this is quite the conditional. Can we break this into helper bools with their own comments/better explanatory names?
will give a try
Yong Li (xWF)
Done
Related details
Attention is currently required from:
- Alexander Cooper
- Brandon Jones
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Cooper (Gerrit)
May 21, 2026, 1:10:40 PM (12 days ago) May 21
to Yong Li (xWF), Code Review Nudger, Chromium LUCI CQ, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Brandon Jones and Yong Li (xWF)
Alexander Cooper added 1 comment![]( "Open in Gerrit")
File device/vr/openxr/openxr_render_loop.cc
Line 1232, Patchset 3 (Latest):
if ((!webxr_visible_ && !on_webxr_submitted_) || webxr_has_pose_) {
return true;
}Alexander Cooper . unresolved
Maybe split this up once more e.g.
```
// If we've already given out a pose for the current frame delay
if (webxr_has_pose_) {
return true;
}
If we aren't visible and the browser is not waiting to learn that we've submitted a frame we should delay. If the browser *is* waiting to learn that we've submitted a frame though, we can give out a pose as though we are visible.
if (!webxr_visible_ && !on_webxr_submitted)) {
return true;
}
```
Comments I left can probably be improved.
Related details
Attention is currently required from:
- Brandon Jones
- Yong Li (xWF)
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
May 21, 2026, 1:19:13 PM (12 days ago) May 21
to Code Review Nudger, Chromium LUCI CQ, Alexander Cooper, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Alexander Cooper and Brandon Jones
Yong Li (xWF) added 1 comment![]( "Open in Gerrit")
File device/vr/openxr/openxr_render_loop.cc
Line 1232, Patchset 3:
if ((!webxr_visible_ && !on_webxr_submitted_) || webxr_has_pose_) {
return true;
}Alexander Cooper . resolved
Maybe split this up once more e.g.
```
// If we've already given out a pose for the current frame delay
if (webxr_has_pose_) {
return true;
}If we aren't visible and the browser is not waiting to learn that we've submitted a frame we should delay. If the browser *is* waiting to learn that we've submitted a frame though, we can give out a pose as though we are visible.
if (!webxr_visible_ && !on_webxr_submitted)) {
return true;
}
```Comments I left can probably be improved.
Attention is currently required from:
- Alexander Cooper
- Brandon Jones
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Cooper (Gerrit)
May 21, 2026, 2:10:14 PM (12 days ago) May 21
to Yong Li (xWF), Code Review Nudger, Chromium LUCI CQ, Brandon Jones, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Brandon Jones and Yong Li (xWF)
Alexander Cooper voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Brandon Jones
- Yong Li (xWF)
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brandon Jones (Gerrit)
May 22, 2026, 9:11:34 PM (11 days ago) May 22
to Yong Li (xWF), Alexander Cooper, Code Review Nudger, Chromium LUCI CQ, chromium...@chromium.org, feature-v...@chromium.org
Attention needed from Yong Li (xWF)
Brandon Jones voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Yong Li (xWF)
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yong Li (xWF) (Gerrit)
May 25, 2026, 8:20:34 AM (9 days ago) May 25
to Brandon Jones, Alexander Cooper, Code Review Nudger, Chromium LUCI CQ, chromium...@chromium.org, feature-v...@chromium.org
Yong Li (xWF) voted Commit-Queue+2![]( "Open in Gerrit")
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
May 25, 2026, 9:07:56 AM (8 days ago) May 25
to Yong Li (xWF), Brandon Jones, Alexander Cooper, Code Review Nudger, chromium...@chromium.org, feature-v...@chromium.org
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Commit message:
Fix a possible deadlock in OpenXRRenderLoop
Prevent from using a pending_frame_ with invalid frame_data_
Bug: 513163401
Change-Id: I7b89fe58ef7bfc04587cd8feb71ba9dcfd20adec
Reviewed-by: Alexander Cooper <alco...@chromium.org>
Commit-Queue: Yong Li (xWF) <yyon...@google.com>
Reviewed-by: Brandon Jones <baj...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1635691}
Files:
- M device/vr/openxr/openxr_render_loop.cc
- M device/vr/openxr/openxr_render_loop.h
Change size: S
Delta: 2 files changed, 31 insertions(+), 10 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Brandon Jones, +1 by Alexander Cooper
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages