Media Session: Add MediaSessionEnterPictureInPictureReason [chromium/src : main]
AI Code Reviewer (Gerrit)
AI Code Reviewer added 1 comment![]( "Open in Gerrit")
const blink::mojom::blink::MediaSessionActionDetailsPtr& details);Blink Style Guide: Naming - May leave obvious parameter names out of function declarations. The parameter name 'details' is evident from its type `MediaSessionActionDetailsPtr` and can be omitted in the header file declaration.
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **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)_
Related details
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tommy Steimel (Gerrit)
Tommy Steimel voted and added 1 comment![]( "Open in Gerrit")
Votes added by Tommy Steimel
| Commit-Queue | +1 |
1 comment
const blink::mojom::blink::MediaSessionActionDetailsPtr& details);Blink Style Guide: Naming - May leave obvious parameter names out of function declarations. The parameter name 'details' is evident from its type `MediaSessionActionDetailsPtr` and can be omitted in the header file declaration.
_To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **Won't fix**: reason | **b/<bug_id>** | **Invalid:** reasonThis 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
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tommy Steimel (Gerrit)
Tommy Steimel voted Commit-Queue+1![]( "Open in Gerrit")
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Frank Liberato (Gerrit)
Frank Liberato voted Code-Review+1![]( "Open in Gerrit")
- Tommy Steimel
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tommy Steimel (Gerrit)
Tommy Steimel added 1 comment![]( "Open in Gerrit")
+dcheng@ owners review of:
third_party/blink/public/mojom/mediasession/media_session.mojom
third_party/blink/renderer/modules/mediasession/media_session_type_converters.*
Related details
- Daniel Cheng
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Daniel Cheng added 1 comment![]( "Open in Gerrit")
kOther,These are kind of straightforward, but I'm not entirely sure what would fall into the `kOther` bucket. Are there specific examples we can provide in comments here?
Related details
- Tommy Steimel
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nattapon Jenphri (Gerrit)
Nattapon Jenphri added 1 comment![]( "Open in Gerrit")
font
gwsq (Gerrit)
Message from gwsq![]( "Open in Gerrit")
From googleclient/chrome/chromium_gwsq/ipc/config.gwsq:
IPC: arthurs...@chromium.org, dch...@chromium.org, mk...@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): arthurs...@chromium.org, dch...@chromium.org, mk...@chromium.org
Note: IPC gwsq added no new reviewers; existing reviewers satisfied requirements!
Reviewer source(s):
dch...@chromium.org is from context(googleclient/chrome/chromium_gwsq/ipc/config.gwsq)
Related details
- Anders Hartvoll Ruud
- Arthur Sonzogni
- Daniel Libby
- Dave Tapuska
- David Baron
- Ian Kilpatrick
- Justin Novosad
- Kenneth Russell
- Kent Tamura
- Koji Ishii
- Kouhei Ueno
- Mike West
- Robert Flack
- Rune Lillesveen
- Scott Haseley
- Stefan Zager
- Stephen White
- Steve Kobes
- Ted (Chromium) Meyer
- Tommy Steimel
- Vladimir Levin
- Xiaohan Wang
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tommy Steimel (Gerrit)
Tommy Steimel voted and added 1 comment![]( "Open in Gerrit")
Votes added by Tommy Steimel
| Commit-Queue | +1 |
1 comment
These are kind of straightforward, but I'm not entirely sure what would fall into the `kOther` bucket. Are there specific examples we can provide in comments here?
- Daniel Cheng
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Daniel Cheng added 3 comments![]( "Open in Gerrit")
return V8MediaSessionEnterPictureInPictureReason::Enum::kContentoccluded;Can we make this a TypeConvert specialization as well?
(Normally I'd ask for a typemap but that's a bit annoying with the V8 IDLs)
blink_details = TypeConverter<
blink::MediaSessionEnterPictureInPictureActionDetails*,
blink::mojom::blink::MediaSessionActionDetailsPtr>::Convert(details);Rather than invoking the specialization directly:
```suggestion
blink_details = mojo::ConvertTo<
blink::MediaSessionSeekToActionDetails*>::Convert(details);
```
Should work.
blink_details->setEnterPictureInPictureReason(MojomReasonToBlinkReason(And this would "just" be `mojo::ConvertTo<V8MediaSessionEnterPictureInPictureReason>(details->get_enter_picture_in_picture()->reason())`
Related details
- Tommy Steimel
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tommy Steimel (Gerrit)
Tommy Steimel voted and added 3 comments![]( "Open in Gerrit")
Votes added by Tommy Steimel
| Commit-Queue | +1 |
3 comments
return V8MediaSessionEnterPictureInPictureReason::Enum::kContentoccluded;Can we make this a TypeConvert specialization as well?
(Normally I'd ask for a typemap but that's a bit annoying with the V8 IDLs)
Done
blink_details = TypeConverter<
blink::MediaSessionEnterPictureInPictureActionDetails*,
blink::mojom::blink::MediaSessionActionDetailsPtr>::Convert(details);Rather than invoking the specialization directly:
```suggestion
blink_details = mojo::ConvertTo<
blink::MediaSessionSeekToActionDetails*>::Convert(details);
```Should work.
Done
blink_details->setEnterPictureInPictureReason(MojomReasonToBlinkReason(And this would "just" be `mojo::ConvertTo<V8MediaSessionEnterPictureInPictureReason>(details->get_enter_picture_in_picture()->reason())`
- Daniel Cheng
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Daniel Cheng (Gerrit)
Daniel Cheng voted Code-Review+1![]( "Open in Gerrit")
- Tommy Steimel
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Tommy Steimel (Gerrit)
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Media Session: Add MediaSessionEnterPictureInPictureReason
This CL adds a reason to be sent alongside `enterpictureinpicture`
Media Session actions to indicate the source of the picture-in-picture
request.
Spec change: https://github.com/w3c/mediasession/pull/362
Chromestatus: https://chromestatus.com/feature/6415506970116096
- M content/browser/media/session/media_session_impl.cc
- M content/browser/media/session/media_session_impl_unittest.cc
- M third_party/blink/public/mojom/mediasession/media_session.mojom
- M third_party/blink/renderer/bindings/generated_in_modules.gni
- M third_party/blink/renderer/bindings/idl_in_modules.gni
- A third_party/blink/renderer/modules/mediasession/media_session_enter_picture_in_picture_action_details.idl
- M third_party/blink/renderer/modules/mediasession/media_session_type_converters.cc
- M third_party/blink/renderer/modules/mediasession/media_session_type_converters.h
Code-Review: +1 by Frank Liberato, +1 by Daniel Cheng
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |