[wayland] Add Viz ExternalBeginFrameSourceMojoWayland [chromium/src : main]

0 views
Skip to first unread message

Mitchell Cohen (Gerrit)

unread,
Jun 28, 2026, 1:26:33 PM (9 days ago) Jun 28
to android-bu...@system.gserviceaccount.com, Kramer Ge, 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 Kramer Ge

Mitchell Cohen added 1 comment

File ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc
Line 317, Patchset 2 (Latest): compositor()->set_skip_wait_for_frame_sinks(true);
Mitchell Cohen . unresolved

I believe it's appropriate to always set this to true here headless mode does not use the desktop tree host, and any desktop frame sources created here would almost certainly want to render without waiting for all sinks. (In practice, the only impl is Ozone Wayland.)

Open in Gerrit

Related details

Attention is currently required from:
  • Kramer Ge
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I8dac39c2371ccfa9f65a4e0916974c7459b5b1ff
Gerrit-Change-Number: 8015303
Gerrit-PatchSet: 2
Gerrit-Owner: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
Gerrit-Reviewer: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-CC: Ian Vollick <vol...@chromium.org>
Gerrit-Attention: Kramer Ge <fang...@chromium.org>
Gerrit-Comment-Date: Sun, 28 Jun 2026 17:26:26 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy

Kramer Ge (Gerrit)

unread,
Jul 6, 2026, 9:34:06 AM (21 hours ago) Jul 6
to Mitchell Cohen, Stacy Gaikovaia, Mitsuru Oshima, Thomas Anderson, 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, Mitsuru Oshima, Stacy Gaikovaia and Thomas Anderson

Kramer Ge voted and added 5 comments

Votes added by Kramer Ge

Commit-Queue+1

5 comments

Patchset-level comments
File-level comment, Patchset 3:
Kramer Ge . resolved

adding gaiko@ for beginframes
```
components/viz/service/
content/browser/compositor/
```
oshima@ for
```
ui/compositor/
ui/platform_window/extensions/
```
thomasanderson@ for
```
ui/views/widget/desktop_aura/
```

Commit Message
Line 8, Patchset 3:
Kramer Ge . unresolved

Can 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?

File components/viz/service/frame_sinks/external_begin_frame_source_mojo.cc
Line 144, Patchset 3: last_begin_frame_args_ = BeginFrameArgs();
Kramer Ge . unresolved

Original `ExternalBeginFrameSourceMojo` logic is changed if `last_begin_frame_args_` reset gets moved here. I don't think this should be changed.

File components/viz/service/frame_sinks/external_begin_frame_source_mojo_wayland.cc
Line 27, Patchset 3:void ExternalBeginFrameSourceMojoWayland::IssueExternalBeginFrame(
Kramer Ge . unresolved

several functions are the same as `ExternalBeginFrameSourceMojo::` that you don't need to copy impl here.

File ui/views/widget/desktop_aura/desktop_window_tree_host_platform.cc
Line 317, Patchset 2: compositor()->set_skip_wait_for_frame_sinks(true);
Mitchell Cohen . resolved

I believe it's appropriate to always set this to true here headless mode does not use the desktop tree host, and any desktop frame sources created here would almost certainly want to render without waiting for all sinks. (In practice, the only impl is Ozone Wayland.)

Kramer Ge

Acknowledged

Open in Gerrit

Related details

Attention is currently required from:
  • Mitchell Cohen
  • Mitsuru Oshima
  • Stacy Gaikovaia
  • Thomas Anderson
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I8dac39c2371ccfa9f65a4e0916974c7459b5b1ff
Gerrit-Change-Number: 8015303
Gerrit-PatchSet: 3
Gerrit-Owner: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
Gerrit-Reviewer: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-Reviewer: Mitsuru Oshima <osh...@chromium.org>
Gerrit-Reviewer: Stacy Gaikovaia <ga...@google.com>
Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>
Gerrit-Attention: Stacy Gaikovaia <ga...@google.com>
Gerrit-Attention: Thomas Anderson <thomasa...@chromium.org>
Gerrit-Attention: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-Attention: Mitsuru Oshima <osh...@chromium.org>
Gerrit-Comment-Date: Mon, 06 Jul 2026 13:33:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Mitchell Cohen <mitc...@agilebits.com>
satisfied_requirement
unsatisfied_requirement
open
diffy

Kramer Ge (Gerrit)

unread,
Jul 6, 2026, 9:36:02 AM (21 hours ago) Jul 6
to Mitchell Cohen, Stacy Gaikovaia, Mitsuru Oshima, Thomas Anderson, 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, Mitsuru Oshima, Stacy Gaikovaia and Thomas Anderson

Kramer Ge added 1 comment

File services/viz/privileged/mojom/compositing/frame_sink_manager.mojom
Line 57, Patchset 6 (Latest): bool skip_wait_for_frame_sinks = false;
Kramer Ge . unresolved

nit: should avoid double negation here for ease of read. `bool wait_for_frame_sinks = true`

Open in Gerrit

Related details

Attention is currently required from:
  • Mitchell Cohen
  • Mitsuru Oshima
  • Stacy Gaikovaia
  • Thomas Anderson
Submit Requirements:
  • requirement satisfiedCode-Coverage
  • requirement is not satisfiedCode-Owners
  • requirement is not satisfiedCode-Review
  • requirement is not satisfiedNo-Unresolved-Comments
  • requirement is not satisfiedReview-Enforcement
Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. DiffyGerrit
Gerrit-MessageType: comment
Gerrit-Project: chromium/src
Gerrit-Branch: main
Gerrit-Change-Id: I8dac39c2371ccfa9f65a4e0916974c7459b5b1ff
Gerrit-Change-Number: 8015303
Gerrit-PatchSet: 6
Gerrit-Owner: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-Reviewer: Kramer Ge <fang...@chromium.org>
Gerrit-Reviewer: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-Reviewer: Mitsuru Oshima <osh...@chromium.org>
Gerrit-Reviewer: Stacy Gaikovaia <ga...@google.com>
Gerrit-Reviewer: Thomas Anderson <thomasa...@chromium.org>
Gerrit-CC: Ian Vollick <vol...@chromium.org>
Gerrit-Attention: Stacy Gaikovaia <ga...@google.com>
Gerrit-Attention: Thomas Anderson <thomasa...@chromium.org>
Gerrit-Attention: Mitchell Cohen <mitc...@agilebits.com>
Gerrit-Attention: Mitsuru Oshima <osh...@chromium.org>
Gerrit-Comment-Date: Mon, 06 Jul 2026 13:35:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
satisfied_requirement
unsatisfied_requirement
open
diffy
Reply all
Reply to author
Forward
0 new messages