[wayland] Recover BeginFrameSource from deadlocks and GPU crashes [chromium/src : main]
0 views
Skip to first unread message
Thomas Anderson (Gerrit)
Jul 6, 2026, 11:22:54 AM (2 days ago) Jul 6
to Mitchell Cohen, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Mitchell Cohen
Thomas Anderson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Thomas Anderson
| Code-Review | +1 |
1 comment
Patchset-level comments
File-level comment, Patchset 9 (Latest):
Thomas Anderson . resolved
owners stamping ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc
Related details
Attention is currently required from:
- Mitchell Cohen
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. |
Kramer Ge (Gerrit)
Jul 6, 2026, 11:49:54 AM (2 days ago) Jul 6
to Mitchell Cohen, Thomas Anderson, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Mitchell Cohen
Kramer Ge voted and added 2 comments![]( "Open in Gerrit")
Votes added by Kramer Ge
| Code-Review | +1 |
| Commit-Queue | +1 |
2 comments
File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
Line 144, Patchset 3:
last_begin_frame_args_ = BeginFrameArgs();Kramer Ge . resolved
Kramer GeOriginal `ExternalBeginFrameSourceMojo` logic is changed if `last_begin_frame_args_` reset gets moved here. I don't think this should be changed.
Done
File services/viz/privileged/mojom/compositing/frame_sink_manager.mojom
Line 57, Patchset 6:
bool skip_wait_for_frame_sinks = false;Kramer Ge . resolved
Kramer Genit: should avoid double negation here for ease of read. `bool wait_for_frame_sinks = true`
Done
Related details
Attention is currently required from:
- Mitchell Cohen
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. |
Thomas Anderson (Gerrit)
Jul 6, 2026, 12:03:41 PM (2 days ago) Jul 6
to Mitchell Cohen, Chromium IPC Reviews, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Chromium IPC Reviews and Mitchell Cohen
Thomas Anderson added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Thomas Anderson . resolved
+chrome-ipc-reviews@ for services/viz/privileged/mojom/compositing/frame_sink_manager.mojom
Related details
Attention is currently required from:
- Chromium IPC Reviews
- Mitchell Cohen
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. |
gwsq (Gerrit)
Jul 6, 2026, 12:06:25 PM (2 days ago) Jul 6
to Mitchell Cohen, Chromium IPC Reviews, Daniel Cheng, Thomas Anderson, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng and Mitchell Cohen
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: dch...@chromium.org
📎 It looks like you’re making a possibly security-sensitive change! 📎 IPC security review isn’t a rubberstamp, so your friendly security reviewer will need a fair amount of context to review your CL effectively. Please review your CL description and code comments to make sure they provide context for someone unfamiliar with your project/area. Pay special attention to where data comes from and which processes it flows between (and their privilege levels). Feel free to point your security reviewer at design docs, bugs, or other links if you can’t reasonably make a self-contained CL description. (Also see https://cbea.ms/git-commit/).
IPC reviewer(s): dch...@chromium.org
Reviewer source(s):
dch...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
Attention is currently required from:
- Daniel Cheng
- Mitchell Cohen
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. |
Mitchell Cohen (Gerrit)
Jul 6, 2026, 12:59:25 PM (2 days ago) Jul 6
to Chromium IPC Reviews, Daniel Cheng, Thomas Anderson, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng
Mitchell Cohen added 2 comments![]( "Open in Gerrit")
Commit Message
Line 8, Patchset 3:
Kramer Ge . resolved
Mitchell CohenCan you add commit message explaining that ExternalBeginFrameSourceMojo is for headless and AR, it waits for all frame_sinks to act on a BeginFrame. So a non-blocking *Wayland subclass is needed?
Done
File components/viz/service/frame_sinks/external_begin_frame_source_mojo_wayland.cc
Line 27, Patchset 3:
void ExternalBeginFrameSourceMojoWayland::IssueExternalBeginFrame(Kramer Ge . resolved
Mitchell Cohenseveral functions are the same as `ExternalBeginFrameSourceMojo::` that you don't need to copy impl here.
Done
Related details
Attention is currently required from:
- Daniel Cheng
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Jul 7, 2026, 1:32:45 AM (yesterday) Jul 7
to Mitchell Cohen, Daniel Cheng, Chromium IPC Reviews, Thomas Anderson, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Mitchell Cohen and Stacy Gaikovaia
Daniel Cheng voted and added 1 comment![]( "Open in Gerrit")
Votes added by Daniel Cheng
| Code-Review | +1 |
1 comment
File services/viz/privileged/mojom/compositing/frame_sink_manager.mojom
Line 56, Patchset 9 (Latest):
// wait for all frame sinks.Daniel Cheng . unresolved
(It might be nice if there's some comment somewhere to help a reader understand when it's useful for this to be `true` vs `false`. In the context of this CL, it's fairly obvious–but I'm not sure if that will always be the case for a reader that comes along later)
Related details
Attention is currently required from:
- Mitchell Cohen
- Stacy Gaikovaia
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. |
Mitchell Cohen (Gerrit)
Jul 7, 2026, 11:08:34 AM (yesterday) Jul 7
to Daniel Cheng, Chromium IPC Reviews, Kramer Ge, Thomas Anderson, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Kramer Ge, Stacy Gaikovaia and Thomas Anderson
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
File services/viz/privileged/mojom/compositing/frame_sink_manager.mojom
(It might be nice if there's some comment somewhere to help a reader understand when it's useful for this to be `true` vs `false`. In the context of this CL, it's fairly obvious–but I'm not sure if that will always be the case for a reader that comes along later)
Attention is currently required from:
- Daniel Cheng
- Kramer Ge
- Stacy Gaikovaia
- Thomas Anderson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kramer Ge (Gerrit)
Jul 7, 2026, 12:21:43 PM (yesterday) Jul 7
to Mitchell Cohen, Daniel Cheng, Chromium IPC Reviews, Thomas Anderson, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Mitchell Cohen, Stacy Gaikovaia and Thomas Anderson
Kramer Ge voted and added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Daniel Cheng
- Mitchell Cohen
- Stacy Gaikovaia
- Thomas Anderson
Daniel Cheng (Gerrit)
Jul 7, 2026, 12:55:26 PM (yesterday) Jul 7
to Mitchell Cohen, Daniel Cheng, Chromium IPC Reviews, Thomas Anderson, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Mitchell Cohen, Stacy Gaikovaia and Thomas Anderson
Daniel Cheng voted and added 1 comment![]( "Open in Gerrit")
Votes added by Daniel Cheng
| Code-Review | +1 |
1 comment
Patchset-level comments
Attention is currently required from:
- Mitchell Cohen
- Stacy Gaikovaia
- Thomas Anderson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Thomas Anderson (Gerrit)
Jul 7, 2026, 6:21:25 PM (yesterday) Jul 7
to Mitchell Cohen, Daniel Cheng, Chromium IPC Reviews, Stacy Gaikovaia, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Mitchell Cohen and Stacy Gaikovaia
Thomas Anderson voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Mitchell Cohen
- Stacy Gaikovaia
Stacy Gaikovaia (Gerrit)
Jul 7, 2026, 6:30:44 PM (yesterday) Jul 7
to Mitchell Cohen, Thomas Anderson, Daniel Cheng, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Mitchell Cohen
Stacy Gaikovaia voted and added 1 comment![]( "Open in Gerrit")
Votes added by Stacy Gaikovaia
| Code-Review | +1 |
1 comment
File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
Line 103, Patchset 10 (Latest):
if (!pending_frame_sinks_.empty())Stacy Gaikovaia . unresolved
nit: should we check (wait_for_frame_sinks_ && !pending_frame_sinks_.empty()) here instead?
Related details
Attention is currently required from:
- Mitchell Cohen
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. |
Mitchell Cohen (Gerrit)
2:17 PM (7 hours ago) 2:17 PM
to Stacy Gaikovaia, Thomas Anderson, Daniel Cheng, Kramer Ge, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Kramer Ge, Stacy Gaikovaia and Thomas Anderson
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
nit: should we check (wait_for_frame_sinks_ && !pending_frame_sinks_.empty()) here instead?
Attention is currently required from:
- Daniel Cheng
- Kramer Ge
- Stacy Gaikovaia
- Thomas Anderson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Stacy Gaikovaia (Gerrit)
2:42 PM (7 hours ago) 2:42 PM
to Mitchell Cohen, Thomas Anderson, Daniel Cheng, Kramer Ge, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Kramer Ge, Mitchell Cohen and Thomas Anderson
Stacy Gaikovaia voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Daniel Cheng
- Kramer Ge
- Mitchell Cohen
- Thomas Anderson
Kramer Ge (Gerrit)
2:51 PM (7 hours ago) 2:51 PM
to Mitchell Cohen, Stacy Gaikovaia, Thomas Anderson, Daniel Cheng, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Mitchell Cohen and Thomas Anderson
Kramer Ge voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Daniel Cheng
- Mitchell Cohen
- Thomas Anderson
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Mitchell Cohen (Gerrit)
5:34 PM (4 hours ago) 5:34 PM
to Kramer Ge, Stacy Gaikovaia, Thomas Anderson, Daniel Cheng, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Kramer Ge, Stacy Gaikovaia and Thomas Anderson
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
Line 103, Patchset 10:
if (!pending_frame_sinks_.empty())Stacy Gaikovaia . unresolved
Mitchell Cohennit: should we check (wait_for_frame_sinks_ && !pending_frame_sinks_.empty()) here instead?
Good catch!
Mitchell Cohen
Needed to revert this change after all since it allowed frame callbacks to be dispatched before the frame was displayed at all. The goal of the wait param is to control whether to keep waiting for sinks after the display draws.
Related details
Attention is currently required from:
- Daniel Cheng
- Kramer Ge
- Stacy Gaikovaia
- Thomas Anderson
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. |
Kramer Ge (Gerrit)
5:45 PM (4 hours ago) 5:45 PM
to Mitchell Cohen, Stacy Gaikovaia, Thomas Anderson, Daniel Cheng, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Mitchell Cohen, Stacy Gaikovaia and Thomas Anderson
Kramer Ge added 1 comment![]( "Open in Gerrit")
File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
Line 103, Patchset 10:
if (!pending_frame_sinks_.empty())Stacy Gaikovaia . unresolved
Mitchell Cohennit: should we check (wait_for_frame_sinks_ && !pending_frame_sinks_.empty()) here instead?
Mitchell CohenGood catch!
Needed to revert this change after all since it allowed frame callbacks to be dispatched before the frame was displayed at all. The goal of the wait param is to control whether to keep waiting for sinks after the display draws.
Kramer Ge
I'm confused why this caused headless test to fail.
That aside, can we eliminate the effect of `pending_frame_sinks_` by skipping `insert` and `erase` instead?
Related details
Attention is currently required from:
- Daniel Cheng
- Mitchell Cohen
- Stacy Gaikovaia
- Thomas Anderson
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. |
Kramer Ge (Gerrit)
5:46 PM (4 hours ago) 5:46 PM
to Mitchell Cohen, Stacy Gaikovaia, Thomas Anderson, Daniel Cheng, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Mitchell Cohen, Stacy Gaikovaia and Thomas Anderson
Kramer Ge added 1 comment![]( "Open in Gerrit")
File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
Line 103, Patchset 10:
if (!pending_frame_sinks_.empty())Stacy Gaikovaia . unresolved
Mitchell Cohennit: should we check (wait_for_frame_sinks_ && !pending_frame_sinks_.empty()) here instead?
Mitchell CohenGood catch!
Kramer GeNeeded to revert this change after all since it allowed frame callbacks to be dispatched before the frame was displayed at all. The goal of the wait param is to control whether to keep waiting for sinks after the display draws.
Kramer GeI'm confused why this caused headless test to fail.
That aside, can we eliminate the effect of `pending_frame_sinks_` by skipping `insert` and `erase` instead?
I'm confused why this caused headless test to fail.
Ah I see it was due to `skip_wait_for_frame_sinks_` removed `skip_` but default value is unchanged.
Mitchell Cohen (Gerrit)
6:21 PM (3 hours ago) 6:21 PM
to Kramer Ge, Stacy Gaikovaia, Thomas Anderson, Daniel Cheng, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Kramer Ge, Stacy Gaikovaia and Thomas Anderson
Mitchell Cohen added 1 comment![]( "Open in Gerrit")
File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
Line 103, Patchset 10:
if (!pending_frame_sinks_.empty())Stacy Gaikovaia . unresolved
Mitchell Cohennit: should we check (wait_for_frame_sinks_ && !pending_frame_sinks_.empty()) here instead?
Mitchell CohenGood catch!
Kramer GeNeeded to revert this change after all since it allowed frame callbacks to be dispatched before the frame was displayed at all. The goal of the wait param is to control whether to keep waiting for sinks after the display draws.
Kramer GeI'm confused why this caused headless test to fail.
That aside, can we eliminate the effect of `pending_frame_sinks_` by skipping `insert` and `erase` instead?
I'm confused why this caused headless test to fail.
Ah I see it was due to `skip_wait_for_frame_sinks_` removed `skip_` but default value is unchanged.
Mitchell Cohen
Yes; there were two issues: 1) renaming the boolean accidentally turned this on for headless 2) even when we set the param deliberately , we do not want to send callbacks before the display finishes.
Related details
Attention is currently required from:
- Daniel Cheng
- Kramer Ge
- Stacy Gaikovaia
- Thomas Anderson
Thomas Anderson (Gerrit)
6:33 PM (3 hours ago) 6:33 PM
to Mitchell Cohen, Kramer Ge, Stacy Gaikovaia, Daniel Cheng, Chromium IPC Reviews, Mitsuru Oshima, android-bu...@system.gserviceaccount.com, Chromium LUCI CQ, chromium...@chromium.org, Ian Vollick, cc-...@chromium.org, chromium-a...@chromium.org, extension...@chromium.org, ipc-securi...@chromium.org, jbauma...@chromium.org, jophba...@chromium.org, max+watc...@igalia.com, mfoltz+wa...@chromium.org, nickdiego+wa...@igalia.com, ozone-...@chromium.org, roblia...@chromium.org, sky+...@chromium.org
Attention needed from Daniel Cheng, Kramer Ge, Mitchell Cohen and Stacy Gaikovaia
Thomas Anderson voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Daniel Cheng
- Kramer Ge
- Mitchell Cohen
- Stacy Gaikovaia
Reply all
Reply to author
Forward
0 new messages