Bug 226077 - Add MediaSession.callActionHandler
Summary: Add MediaSession.callActionHandler
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Media (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jean-Yves Avenard [:jya]
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-05-21 04:31 PDT by Jean-Yves Avenard [:jya]
Modified: 2021-05-28 13:08 PDT (History)
11 users (show)

See Also:


Attachments
Patch (9.97 KB, patch)
2021-05-21 04:40 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (9.96 KB, patch)
2021-05-27 07:35 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff
Patch (10.15 KB, patch)
2021-05-27 22:16 PDT, Jean-Yves Avenard [:jya]
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jean-Yves Avenard [:jya] 2021-05-21 04:31:11 PDT
Add MediaSession.callActionHandler
Comment 1 Jean-Yves Avenard [:jya] 2021-05-21 04:31:35 PDT
rdar://77463304
Comment 2 Jean-Yves Avenard [:jya] 2021-05-21 04:40:01 PDT
Created attachment 429283 [details]
Patch
Comment 3 Peng Liu 2021-05-25 10:47:20 PDT
Comment on attachment 429283 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=429283&action=review

> Source/WebCore/Modules/mediasession/MediaSession.cpp:288
> +bool MediaSession::callActionHandler(const MediaSessionActionDetails& actionDetails, TriggerGestureIndicator triggerGestureIndicator )

Nit. An extra space.

> Source/WebCore/Modules/mediasession/MediaSession.h:94
> +        Yes,

Normally we define `No` as the first value in an enum class.
Comment 4 Chris Dumez 2021-05-25 10:51:22 PDT
Comment on attachment 429283 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=429283&action=review

> Source/WebCore/Modules/mediasession/MediaSession.cpp:291
> +        Optional<UserGestureIndicator> maybeGestureIndicator;

The existing pattern in WebKit seems to be:
UserGestureIndicator gestureIndicator(triggerGestureIndicator == TriggerGestureIndicator::Yes ? makeOptional(ProcessingUserGesture) : WTF::nullopt);

>> Source/WebCore/Modules/mediasession/MediaSession.h:94
>> +        Yes,
> 
> Normally we define `No` as the first value in an enum class.

+1 (so that No = 0, Yes = 1)
Comment 5 Jean-Yves Avenard [:jya] 2021-05-27 07:35:48 PDT
Created attachment 429877 [details]
Patch

Apply comments
Comment 6 Eric Carlson 2021-05-27 08:45:33 PDT
Comment on attachment 429877 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=429877&action=review

> Source/WebCore/ChangeLog:6
> +

It would be good to have a description of why we are making this change.
Comment 7 Jean-Yves Avenard [:jya] 2021-05-27 22:16:20 PDT
Created attachment 429985 [details]
Patch
Comment 8 EWS 2021-05-28 13:08:17 PDT
Committed r278222 (238260@main): <https://commits.webkit.org/238260@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 429985 [details].