gwsq (Gerrit)

unread,

Mar 31, 2026, 5:33:46 PM (2 days ago) Mar 31

to Phil Yan, Chromium IPC Reviews, Alex Gough, Dale Curtis, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Alex Gough, Dale Curtis and Frank Liberato

Message from gwsq

From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: aj...@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): aj...@chromium.org

Reviewer source(s):
aj...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)

Open in Gerrit

Related details

Attention is currently required from:

Alex Gough
Dale Curtis
Frank Liberato

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Frank Liberato (Gerrit)

unread,

Mar 31, 2026, 5:54:32 PM (2 days ago) Mar 31

to Phil Yan, Chromium IPC Reviews, Alex Gough, Dale Curtis, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Alex Gough, Dale Curtis and Phil Yan

Frank Liberato added 1 comment

Patchset-level comments

File-level comment, Patchset 4 (Latest):

Frank Liberato . resolved

me=>cc since i dont' think i own anything that Dale doesn't.

thanks
-fl

Open in Gerrit

Related details

Attention is currently required from:

Alex Gough
Dale Curtis
Phil Yan

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review
Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Dale Curtis (Gerrit)

unread,

Mar 31, 2026, 5:56:04 PM (2 days ago) Mar 31

to Phil Yan, Chromium IPC Reviews, Alex Gough, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Alex Gough and Phil Yan

Dale Curtis voted and added 2 comments

Votes added by Dale Curtis

Code-Review

+1

2 comments

File third_party/blink/public/platform/web_media_player.h

Line 207, Patchset 4 (Latest): // Requests the media player to resume playback when authorized by the system

Dale Curtis . unresolved

I think you just want to call this something like `UnlockBackgroundPlayback()`

File third_party/blink/renderer/platform/media/web_media_player_impl.cc

Line 1016, Patchset 4 (Latest): video_locked_when_paused_when_hidden_ = false;

Dale Curtis . resolved

This naming has always confused me. We should probably invert and rename this in a follow-up to something like `allow_background_video_playback_`.

Open in Gerrit

Related details

Attention is currently required from:

Alex Gough
Phil Yan

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Phil Yan (Gerrit)

unread,

Mar 31, 2026, 6:40:24 PM (2 days ago) Mar 31

to Dale Curtis, Chromium IPC Reviews, Alex Gough, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Alex Gough

Phil Yan added 2 comments

File third_party/blink/public/platform/web_media_player.h

Line 207, Patchset 4: // Requests the media player to resume playback when authorized by the system

Dale Curtis . resolved

I think you just want to call this something like `UnlockBackgroundPlayback()`

Phil Yan

Indeed, that's a better name.

File third_party/blink/renderer/platform/media/web_media_player_impl.cc

Line 1016, Patchset 4: video_locked_when_paused_when_hidden_ = false;

Dale Curtis . resolved

This naming has always confused me. We should probably invert and rename this in a follow-up to something like `allow_background_video_playback_`.

Phil Yan

ACK

Open in Gerrit

Related details

Attention is currently required from:

Alex Gough

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Alex Gough (Gerrit)

unread,

Mar 31, 2026, 8:07:59 PM (2 days ago) Mar 31

to Phil Yan, Alex Gough, Dale Curtis, Chromium IPC Reviews, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Phil Yan

Alex Gough voted and added 1 comment

Votes added by Alex Gough

Code-Review

+1

1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Alex Gough . resolved

lgtm mojom

Open in Gerrit

Related details

Attention is currently required from:

Phil Yan

satisfied_requirement

unsatisfied_requirement

open

diffy

Phil Yan (Gerrit)

unread,

Mar 31, 2026, 8:17:30 PM (2 days ago) Mar 31

to Daniel Cheng, Alex Gough, Dale Curtis, Chromium IPC Reviews, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Daniel Cheng

Phil Yan added 1 comment

Patchset-level comments

File-level comment, Patchset 5 (Latest):

Phil Yan . resolved

Hi Daniel, could you please review the changes in the following files:
```
third_party/blink/public/platform/web_media_player.h
third_party/blink/renderer/modules/webaudio/audio_context.h
```
thank you

Open in Gerrit

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Daniel Cheng (Gerrit)

unread,

Apr 1, 2026, 11:39:34 AM (19 hours ago) Apr 1

to Phil Yan, Daniel Cheng, Alex Gough, Dale Curtis, Chromium IPC Reviews, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Phil Yan

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

Code-Review

+1

1 comment

File third_party/blink/renderer/core/html/media/html_media_element_test.cc

Line 1825, Patchset 5 (Latest): static_cast<media::mojom::blink::MediaPlayer*>(Media())->RequestPlay(

Daniel Cheng . unresolved

I have mixed feelings about these `static_cast`s. I'm not sure how much sense it makes to make the overrides private since anyone can downcast.

Open in Gerrit

Related details

Attention is currently required from:

Phil Yan

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Phil Yan (Gerrit)

unread,

Apr 1, 2026, 2:38:21 PM (16 hours ago) Apr 1

to Daniel Cheng, Alex Gough, Dale Curtis, Chromium IPC Reviews, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Daniel Cheng

Phil Yan added 1 comment

File third_party/blink/renderer/core/html/media/html_media_element_test.cc

Line 1825, Patchset 5: static_cast<media::mojom::blink::MediaPlayer*>(Media())->RequestPlay(

Daniel Cheng . unresolved

I have mixed feelings about these `static_cast`s. I'm not sure how much sense it makes to make the overrides private since anyone can downcast.

Phil Yan

Agree that making them private doesn't actually prevent them from being called publicly.

My concern with moving them to public members is that they could be accidentally misused by developers (e.g., showing up in auto-completion where someone might not pay close attention to the differences between Play() and RequestPlay()).

For tests specifically, to avoid the static_cast clutters, I moved it to a helper method.

Open in Gerrit

Related details

Attention is currently required from:

Daniel Cheng

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.

Gerrit

satisfied_requirement

unsatisfied_requirement

open

diffy

Daniel Cheng (Gerrit)

unread,

Apr 1, 2026, 4:59:58 PM (13 hours ago) Apr 1

to Phil Yan, Daniel Cheng, Alex Gough, Dale Curtis, Chromium IPC Reviews, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Attention needed from Phil Yan

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

Code-Review

+1

1 comment

File third_party/blink/renderer/core/html/media/html_media_element_test.cc

Line 1825, Patchset 5: static_cast<media::mojom::blink::MediaPlayer*>(Media())->RequestPlay(

Daniel Cheng . resolved

I have mixed feelings about these `static_cast`s. I'm not sure how much sense it makes to make the overrides private since anyone can downcast.

Phil Yan

Agree that making them private doesn't actually prevent them from being called publicly.
My concern with moving them to public members is that they could be accidentally misused by developers (e.g., showing up in auto-completion where someone might not pay close attention to the differences between Play() and RequestPlay()).
For tests specifically, to avoid the static_cast clutters, I moved it to a helper method.

Daniel Cheng

Fair enough. It might be worth considering composition rather than inheritance (in the style of LocalFrameMojoHandler) but it is a significant amount of extra plumbing, and is out of scope here anyway.

Open in Gerrit

Related details

Attention is currently required from:

Phil Yan

Submit Requirements:

Code-Coverage
Code-Owners
Code-Review

Review-Enforcement

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

satisfied_requirement

open

diffy

Phil Yan (Gerrit)

unread,

Apr 1, 2026, 6:14:52 PM (12 hours ago) Apr 1

to Daniel Cheng, Alex Gough, Dale Curtis, Chromium IPC Reviews, Chromium LUCI CQ, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Phil Yan voted Commit-Queue+2

Commit-Queue

+2

Open in Gerrit

Related details

Attention set is empty

satisfied_requirement

open

diffy

Chromium LUCI CQ (Gerrit)

unread,

Apr 1, 2026, 8:12:40 PM (10 hours ago) Apr 1

to Phil Yan, Daniel Cheng, Alex Gough, Dale Curtis, Chromium IPC Reviews, chromium...@chromium.org, Hongchan Choi, srirama chandra sekhar, blink-re...@chromium.org, blink-rev...@chromium.org, blink-...@chromium.org, eric.c...@apple.com, erickun...@chromium.org, feature-me...@chromium.org, ipc-securi...@chromium.org, jophba...@chromium.org, kinuko...@chromium.org, mfoltz+wa...@chromium.org

Chromium LUCI CQ submitted the change

Change information

Commit message:

[Media] Prevent user activation bypass on system audio focus resume

When a system event (like audio focus regain after a notification)
triggered a media playback resume, the browser sent a RequestPlay
IPC to the renderer. HTMLMediaElement::RequestPlay() unconditionally
granted a Transient User Activation token to bypass background
playback restrictions in WebMediaPlayerImpl.

Because the system audio resume was indistinguishable from a user
interaction (like pressing a Bluetooth headset button), a malicious
background page could listen for audio focus events to harvest a
free User Activation token without user interaction.

This CL fixes the vulnerability by:
1. Adding a `triggered_by_user` flag to the `RequestPlay` Mojo IPC.
2. Only granting the Transient User Activation token in
   `HTMLMediaElement::RequestPlay` if the request was truly
   user-triggered.
3. Adding a new `UnlockBackgroundPlayback` method to the
   `WebMediaPlayer` interface so that background media playback
   can still resume smoothly without relying on a fake user
   activation token.

TODO: although `HTMLMediaElement::RequestPause` already has a
`triggered_by_user`, it's hardcoded to true in
`MediaSessionController::OnSuspend`. We need to set it dynamically by
plumbing the value from `MediaSessionImpl::OnSuspendInternal`.

Bug: 497548558

Change-Id: I3a86a6aa327b90fa2ff051c4fd07d893f1e4a30a

Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7714520

Reviewed-by: Daniel Cheng <dch...@chromium.org>

Reviewed-by: Alex Gough <aj...@chromium.org>

Reviewed-by: Dale Curtis <dalec...@chromium.org>

Commit-Queue: Phil Yan <phi...@chromium.org>

Cr-Commit-Position: refs/heads/main@{#1608880}

Files:

M content/browser/media/media_web_contents_observer_unittest.cc
M content/browser/media/session/media_session_controller.cc
M content/browser/media/session/media_session_controller.h
M content/browser/media/session/media_session_controller_unittest.cc
M content/browser/media/session/media_session_impl.cc
M content/browser/media/session/media_session_impl_service_routing_unittest.cc
M content/browser/media/session/media_session_player_observer.h
M content/browser/media/session/media_session_service_impl_browsertest.cc
M content/browser/media/session/mock_media_session_player_observer.cc
M content/browser/media/session/mock_media_session_player_observer.h
M content/browser/picture_in_picture/picture_in_picture_service_impl_unittest.cc
M content/browser/picture_in_picture/video_picture_in_picture_window_controller_impl.cc
M media/mojo/mojom/media_player.mojom
M third_party/blink/public/platform/web_media_player.h
M third_party/blink/renderer/core/exported/web_media_player_impl_unittest.cc
M third_party/blink/renderer/core/html/media/html_media_element.cc
M third_party/blink/renderer/core/html/media/html_media_element.h
M third_party/blink/renderer/core/html/media/html_media_element_test.cc
M third_party/blink/renderer/modules/webaudio/audio_context.h
M third_party/blink/renderer/platform/media/web_media_player_impl.cc
M third_party/blink/renderer/platform/media/web_media_player_impl.h

Change size: M

Delta: 21 files changed, 130 insertions(+), 28 deletions(-)

Branch: refs/heads/main

Submit Requirements:

Code-Review: +1 by Dale Curtis, +1 by Alex Gough, +1 by Daniel Cheng

Open in Gerrit

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

open

diffy

satisfied_requirement

Reply all

Reply to author

Forward

[Media] Prevent user activation bypass on system audio focus resume [chromium/src : main]

gwsq (Gerrit)

Message from gwsq

Related details

Frank Liberato (Gerrit)

Frank Liberato added 1 comment

Related details

Dale Curtis (Gerrit)

Dale Curtis voted and added 2 comments

Votes added by Dale Curtis

2 comments

Related details

Phil Yan (Gerrit)

Phil Yan added 2 comments

Related details

Alex Gough (Gerrit)

Alex Gough voted and added 1 comment

Votes added by Alex Gough

1 comment

Related details

Phil Yan (Gerrit)

Phil Yan added 1 comment

Related details

Daniel Cheng (Gerrit)

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

1 comment

Related details

Phil Yan (Gerrit)

Phil Yan added 1 comment

Related details

Daniel Cheng (Gerrit)

Daniel Cheng voted and added 1 comment

Votes added by Daniel Cheng

1 comment

Related details

Phil Yan (Gerrit)

Phil Yan voted Commit-Queue+2

Related details

Chromium LUCI CQ (Gerrit)

Chromium LUCI CQ submitted the change

Change information