[WebAudio] Implement AudioContext.playbackStats behind a flag [chromium/src : main]
Hongchan Choi (Gerrit)
Hongchan Choi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hongchan Choi
| Commit-Queue | +1 |
1 comment
bool stats_are_from_current_task_ = false;Hongchan Choinit: Blink Style Guide: Precede boolean values with words like “is” and “did”. Consider renaming 'stats_are_from_current_task_' to 'are_stats_from_current_task_' to strictly follow the prefix rule.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Done
Related details
- Fredrik Hernqvist
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongchan Choi (Gerrit)
Hongchan Choi added 1 comment![]( "Open in Gerrit")
PTAL
The intent to ship for this feature is currently under review:
https://groups.google.com/a/chromium.org/g/blink-dev/c/p-IHBp8Nw2Q
This CL is to prepare the shipment in M146, by landing the changes behind the flag and enable it later after the approval.
Related details
- Fredrik Hernqvist
- Kent Tamura
- Michael Wilson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Wilson (Gerrit)
Michael Wilson voted and added 3 comments![]( "Open in Gerrit")
Votes added by Michael Wilson
| Code-Review | +1 |
3 comments
LGTM overall.
[RuntimeEnabled=AudioContextPlayoutStats, MeasureAs=AudioContextPlayoutStats, SameObject] readonly attribute AudioPlayoutStats playoutStats;Consider using DeprecateAs instead? (https://chromium.googlesource.com/chromium/src/third_party/+/HEAD/blink/renderer/bindings/IDLExtendedAttributes.md)
```suggestion
[RuntimeEnabled=AudioContextPlayoutStats, DeprecateAs=AudioContextPlayoutStats, SameObject] readonly attribute AudioPlayoutStats playoutStats;
```
bool stats_are_from_current_task_ = false;Hongchan Choinit: Blink Style Guide: Precede boolean values with words like “is” and “did”. Consider renaming 'stats_are_from_current_task_' to 'are_stats_from_current_task_' to strictly follow the prefix rule.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Done
Nit: this should probably be `is_stats_from_current_task_` because it's referring to the `stats_` variable, not a general collection of statistics abbreviated as stats.
I don't feel strongly about this, so also fine to leave as-is.
Related details
- Fredrik Hernqvist
- Hongchan Choi
- Kent Tamura
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. |
Hongchan Choi (Gerrit)
Hongchan Choi added 3 comments![]( "Open in Gerrit")
tkent@ PTAL at:
- r/b/r/p/runtime_enabled_features.json5
- t/b/web_tests/VirtualTestSuites
Mainly this CL is the name change of the API that's currently under Origin Trial.
The I2S is currently under review, so I am trying to land the CL behind the "experimetal" gate.
[RuntimeEnabled=AudioContextPlayoutStats, MeasureAs=AudioContextPlayoutStats, SameObject] readonly attribute AudioPlayoutStats playoutStats;Consider using DeprecateAs instead? (https://chromium.googlesource.com/chromium/src/third_party/+/HEAD/blink/renderer/bindings/IDLExtendedAttributes.md)
```suggestion
[RuntimeEnabled=AudioContextPlayoutStats, DeprecateAs=AudioContextPlayoutStats, SameObject] readonly attribute AudioPlayoutStats playoutStats;
```
Not yet, but in the follow-up CL that enables the Playback API by default.
Thanks for the suggestion!
bool stats_are_from_current_task_ = false;Hongchan Choinit: Blink Style Guide: Precede boolean values with words like “is” and “did”. Consider renaming 'stats_are_from_current_task_' to 'are_stats_from_current_task_' to strictly follow the prefix rule.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Michael WilsonDone
Nit: this should probably be `is_stats_from_current_task_` because it's referring to the `stats_` variable, not a general collection of statistics abbreviated as stats.
I don't feel strongly about this, so also fine to leave as-is.
- Fredrik Hernqvist
- Kent Tamura
- Michael Wilson
Kent Tamura (Gerrit)
Kent Tamura voted and added 1 comment![]( "Open in Gerrit")
Votes added by Kent Tamura
| Code-Review | +1 |
1 comment
tkent@ PTAL at:
- r/b/r/p/runtime_enabled_features.json5
- t/b/web_tests/VirtualTestSuites
Mainly this CL is the name change of the API that's currently under Origin Trial.
The I2S is currently under review, so I am trying to land the CL behind the "experimetal" gate.
- Fredrik Hernqvist
- Hongchan Choi
- Michael Wilson
Code-Coverage
Code-Owners
Hongchan Choi (Gerrit)
Hongchan Choi added 2 comments![]( "Open in Gerrit")
Apologies - renaming variables reset +1s.
Could you stamp the CL if possible?
bool stats_are_from_current_task_ = false;Hongchan Choinit: Blink Style Guide: Precede boolean values with words like “is” and “did”. Consider renaming 'stats_are_from_current_task_' to 'are_stats_from_current_task_' to strictly follow the prefix rule.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your 🙏 feedback 🙏 to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Michael WilsonDone
Hongchan ChoiNit: this should probably be `is_stats_from_current_task_` because it's referring to the `stats_` variable, not a general collection of statistics abbreviated as stats.
I don't feel strongly about this, so also fine to leave as-is.
I'll include this fix when I rebase this CL later.
- Fredrik Hernqvist
- Kent Tamura
- Michael Wilson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kent Tamura (Gerrit)
Kent Tamura voted Code-Review+1![]( "Open in Gerrit")
- Fredrik Hernqvist
- Hongchan Choi
- Michael Wilson
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongchan Choi (Gerrit)
Hongchan Choi voted Commit-Queue+2![]( "Open in Gerrit")
- Fredrik Hernqvist
- Michael Wilson
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[WebAudio] Implement AudioContext.playbackStats behind a flag
This CL implements the specification-compliant
`AudioContext.playbackStats` attribute and the associated
`AudioPlaybackStats` interface. This new API provides detailed metrics
regarding audio rendering, including latency extremes and underrun
(glitch) durations, using standard SI units (seconds).
The legacy `playoutStats` attribute (which uses milliseconds) is
maintained for backward compatibility in M146 and is scheduled for
removal in M147. Both APIs are independently gated and tracked via use
counters.
- Implemented `AudioContext.playbackStats` and the
`AudioPlaybackStats` interface.
- Added the `AudioContextPlaybackStats` runtime feature flag and
associated use counter.
- Refactored `MockPermissionService` to use `mojo::ReceiverSet` to
fix unit test instability.
- Maintained legacy `playoutStats` with independent test coverage
and metrics.
- Updated web-exposed API baselines for Blink, WebView, and WPT
idlharness.
- M android_webview/test/data/web_tests/webexposed/global-interface-listing-expected.txt
- M third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
- M third_party/blink/renderer/bindings/generated_in_modules.gni
- M third_party/blink/renderer/bindings/idl_in_modules.gni
- M third_party/blink/renderer/modules/webaudio/BUILD.gn
- M third_party/blink/renderer/modules/webaudio/audio_context.cc
- M third_party/blink/renderer/modules/webaudio/audio_context.h
- M third_party/blink/renderer/modules/webaudio/audio_context.idl
- M third_party/blink/renderer/modules/webaudio/audio_context_test.cc
- A third_party/blink/renderer/modules/webaudio/audio_playback_stats.cc
- A third_party/blink/renderer/modules/webaudio/audio_playback_stats.h
- A third_party/blink/renderer/modules/webaudio/audio_playback_stats.idl
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
- M third_party/blink/web_tests/VirtualTestSuites
- M third_party/blink/web_tests/external/wpt/webaudio/idlharness.https.window-expected.txt
- C third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-playbackstats.html
- A third_party/blink/web_tests/virtual/webaudio-playback-stats/README.md
- R third_party/blink/web_tests/webaudio/AudioContext/audiocontext-playoutstats.html
- M third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt
- M tools/metrics/histograms/metadata/blink/enums.xml
Code-Review: +1 by Kent Tamura
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/57581
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Sergey Poromov (Gerrit)
Sergey Poromov has created a revert of this change![]( "Open in Gerrit")
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongchan Choi (Gerrit)
Hongchan Choi voted and added 1 comment![]( "Open in Gerrit")
Votes added by Hongchan Choi
| Commit-Queue | +1 |
1 comment
PTAL again - This is a re-land of https://crrev.com/c/7298786.
No changes from the previous CL, except for restoring the 'timing out' test to ToT. So the problematic test is not a part of the CL anymore.
Thank you for your review!
Related details
- Kent Tamura
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Kent Tamura (Gerrit)
Kent Tamura voted Code-Review+1![]( "Open in Gerrit")
- Hongchan Choi
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Hongchan Choi (Gerrit)
Hongchan Choi voted Commit-Queue+2![]( "Open in Gerrit")
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)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
[WebAudio] Implement AudioContext.playbackStats behind a flag
This CL implements the specification-compliant
`AudioContext.playbackStats` attribute and the associated
`AudioPlaybackStats` interface. This new API provides detailed metrics
regarding audio rendering, including latency extremes and underrun
(glitch) durations, using standard SI units (seconds).
The legacy `playoutStats` attribute (which uses milliseconds) is
maintained for backward compatibility in M146 and is scheduled for
removal in M147. Both APIs are independently gated and tracked via use
counters.
- Implemented `AudioContext.playbackStats` and the
`AudioPlaybackStats` interface.
- Added the `AudioContextPlaybackStats` runtime feature flag and
associated use counter.
- Refactored `MockPermissionService` to use `mojo::ReceiverSet` to
fix unit test instability.
- Maintained legacy `playoutStats` with independent test coverage
and metrics.
- Updated web-exposed API baselines for Blink, WebView, and WPT
idlharness.
- M android_webview/test/data/web_tests/webexposed/global-interface-listing-expected.txt
- M third_party/blink/public/mojom/use_counter/metrics/web_feature.mojom
- M third_party/blink/renderer/bindings/generated_in_modules.gni
- M third_party/blink/renderer/bindings/idl_in_modules.gni
- M third_party/blink/renderer/modules/webaudio/BUILD.gn
- M third_party/blink/renderer/modules/webaudio/audio_context.cc
- M third_party/blink/renderer/modules/webaudio/audio_context.h
- M third_party/blink/renderer/modules/webaudio/audio_context.idl
- M third_party/blink/renderer/modules/webaudio/audio_context_test.cc
- A third_party/blink/renderer/modules/webaudio/audio_playback_stats.cc
- A third_party/blink/renderer/modules/webaudio/audio_playback_stats.h
- A third_party/blink/renderer/modules/webaudio/audio_playback_stats.idl
- M third_party/blink/renderer/platform/runtime_enabled_features.json5
- M third_party/blink/web_tests/VirtualTestSuites
- M third_party/blink/web_tests/external/wpt/webaudio/idlharness.https.window-expected.txt
- A third_party/blink/web_tests/external/wpt/webaudio/the-audio-api/the-audiocontext-interface/audiocontext-playbackstats.html
- A third_party/blink/web_tests/virtual/webaudio-playback-stats/README.md
- M third_party/blink/web_tests/webexposed/global-interface-listing-expected.txt
- M tools/metrics/histograms/metadata/blink/enums.xml
Code-Review: +1 by Kent Tamura
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Blink W3C Test Autoroller (Gerrit)
Message from Blink W3C Test Autoroller![]( "Open in Gerrit")
The WPT PR for this CL has been merged upstream! https://github.com/web-platform-tests/wpt/pull/57597
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |