Bug 229068 – nexttrack and previoustrack MediaSession handlers not working

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Jean-Yves Avenard [:jya]]

nexttrack and previoustrack MediaSession handlers not working

[Jean-Yves Avenard [:jya]]

rdar://80100092

[Jean-Yves Avenard [:jya]]

Created attachment 435471 [details]
Patch

[youenn fablet]

Comment on attachment 435471 [details]
Patch

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

> Source/WebCore/ChangeLog:12
> +        used with the MediaRemote backend, which prevents automating the test.

Could we do the following:
- Use MediaSession API to trigger adding handlers for nexttrack.
- Add an Internals.idl API to get the supported commands, which would generate a string. Validate 'nexttrack' is in the commands?

Another approach would be to add a mock backend, which sounds feasible as well.

[Jean-Yves Avenard [:jya]]

Comment on attachment 435471 [details]
Patch

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

>> Source/WebCore/ChangeLog:12
>> +        used with the MediaRemote backend, which prevents automating the test.
> 
> Could we do the following:
> - Use MediaSession API to trigger adding handlers for nexttrack.
> - Add an Internals.idl API to get the supported commands, which would generate a string. Validate 'nexttrack' is in the commands?
> 
> Another approach would be to add a mock backend, which sounds feasible as well.

What this bug caused was that if you registered a nexttrack action handler it would have registered the MR previous track message that makes Now Playing display the wrong button. 

This was the only thing wrong. If you had pressed the previous button, it would have correctly called the previoustrack handler. The mapping from MR to MediaSession is correct. 

If we had a test, all we would test is the test itself, there would be no guarantee that if that test pass the right information would reach MR.

[youenn fablet]

> If we had a test, all we would test is the test itself, there would be no
> guarantee that if that test pass the right information would reach MR.

It depends how you implement the Internals API (it would need to get the information from the MR) or how you implement the mock (it would need to replace the session manager nowPlaying manager and then query what the mock is receiving, should not be too difficult with mock in web process, but harder in GPU process).

[Jean-Yves Avenard [:jya]]

I will take this in another bug. The scope to implement such test will be way more difficult than the fix itself

[EWS]

Committed r281013 (240503@main): <https://commits.webkit.org/240503@main>

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

[Eric Carlson]

Comment on attachment 435471 [details]
Patch

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

>>> Source/WebCore/ChangeLog:12
>>> +        used with the MediaRemote backend, which prevents automating the test.
>> 
>> Could we do the following:
>> - Use MediaSession API to trigger adding handlers for nexttrack.
>> - Add an Internals.idl API to get the supported commands, which would generate a string. Validate 'nexttrack' is in the commands?
>> 
>> Another approach would be to add a mock backend, which sounds feasible as well.
> 
> What this bug caused was that if you registered a nexttrack action handler it would have registered the MR previous track message that makes Now Playing display the wrong button. 
> 
> This was the only thing wrong. If you had pressed the previous button, it would have correctly called the previoustrack handler. The mapping from MR to MediaSession is correct. 
> 
> If we had a test, all we would test is the test itself, there would be no guarantee that if that test pass the right information would reach MR.

Making this testable could be as simple as adding `PlatformMediaSessionManager::getSupportedCommands()`, plumbing it down to `RemoteCommandListener`, and exposing that through Internals.idl.

[Aakash Jain]

Sees like this broke windows build
https://build.webkit.org/#/builders/67/builds/4559 failed with 240502@main (r281012)
https://build.webkit.org/#/builders/67/builds/4558 passed with 240501@main (r281011)

[WebKit Commit Bot]

Re-opened since this is blocked by bug 229096

[Aakash Jain]

Commented on wrong bug, please ignore.