229068 2021-08-12 20:54:17 -0700 nexttrack and previoustrack MediaSession handlers not working 2021-08-13 14:17:48 -0700 1 1 1 Unclassified WebKit Media WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED https://bugs.webkit.org/show_bug.cgi?id=229070 InRadar P2 Normal --- 229096 1 jean-yves.avenard jean-yves.avenard aakash_jain commit-queue eric.carlson ews-watchlist glenn jer.noble philipj sergio webkit-bug-importer youennf oldest_to_newest 1784037 0 jean-yves.avenard 2021-08-12 20:54:17 -0700 nexttrack and previoustrack MediaSession handlers not working 1784038 1 jean-yves.avenard 2021-08-12 20:55:38 -0700 rdar://80100092 1784044 2 435471 jean-yves.avenard 2021-08-12 21:18:13 -0700 Created attachment 435471 Patch 1784050 3 435471 youennf 2021-08-13 00:07:39 -0700 Comment on attachment 435471 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. 1784056 4 435471 jean-yves.avenard 2021-08-13 01:36:35 -0700 Comment on attachment 435471 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. 1784060 5 youennf 2021-08-13 01:45:40 -0700 > 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). 1784076 6 jean-yves.avenard 2021-08-13 04:33:42 -0700 I will take this in another bug. The scope to implement such test will be way more difficult than the fix itself 1784079 7 ews-feeder 2021-08-13 04:57:28 -0700 Committed r281013 (240503@main): <https://commits.webkit.org/240503@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435471. 1784112 8 435471 eric.carlson 2021-08-13 08:50:26 -0700 Comment on attachment 435471 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. 1784213 9 aakash_jain 2021-08-13 14:09:56 -0700 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) 1784215 10 commit-queue 2021-08-13 14:10:36 -0700 Re-opened since this is blocked by bug 229096 1784223 11 aakash_jain 2021-08-13 14:17:48 -0700 Commented on wrong bug, please ignore. 435471 2021-08-12 21:18:13 -0700 2021-08-13 04:57:29 -0700 Patch bug-229068-20210813141812.patch text/plain 2340 jean-yves.avenard U3VidmVyc2lvbiBSZXZpc2lvbjogMjgwOTkwCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggMjJkZTIzZGUzZmZmNzQy Y2M1ODBhMzIzM2ZkNjIwYWRkZmM4ODkzOS4uNjVjMGE2MmNiZjhiNTU3OWI4M2JmNzQ0N2YzYTRh ZWRjZTU2YTQ2YiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDIxLTA4LTEyICBKZWFu LVl2ZXMgQXZlbmFyZCAgPGp5YUBhcHBsZS5jb20+CisKKyAgICAgICAgbmV4dHRyYWNrIGFuZCBw cmV2aW91c3RyYWNrIE1lZGlhU2Vzc2lvbiBoYW5kbGVycyBub3Qgd29ya2luZworICAgICAgICBo dHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjI5MDY4CisgICAgICAgIHJk YXI6Ly84MDEwMDA5MgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIE1hcCBiZXR3ZWVuIE1lZGlhU2Vzc2lvbiBhY3Rpb24gbmV4dC9wcmV2aW91c1RyYWNr IGFuZCBSZW1vdGVDb250cm9sQ29tbWFuZFR5cGUgb25lcworICAgICAgICB3ZXJlIGludmVydGVk LgorICAgICAgICBUaGVyZSBpcyBubyBpbmZyYXN0cnVyZSBpbiBwbGFjZSB0byBlbnN1cmUgdGhh dCB0aGUgcmlnaHQgTVJNZWRpYVJlbW90ZUNvbW1hbmQgaXMKKyAgICAgICAgdXNlZCB3aXRoIHRo ZSBNZWRpYVJlbW90ZSBiYWNrZW5kLCB3aGljaCBwcmV2ZW50cyBhdXRvbWF0aW5nIHRoZSB0ZXN0 LgorCisgICAgICAgICogTW9kdWxlcy9tZWRpYXNlc3Npb24vTWVkaWFTZXNzaW9uLmNwcDoKKyAg ICAgICAgKFdlYkNvcmU6OnBsYXRmb3JtQ29tbWFuZEZvck1lZGlhU2Vzc2lvbkFjdGlvbik6CisK IDIwMjEtMDgtMTIgIEFsZXggQ2hyaXN0ZW5zZW4gIDxhY2hyaXN0ZW5zZW5Ad2Via2l0Lm9yZz4K IAogICAgICAgICBVbnJldmlld2VkLCByZXZlcnRpbmcgcjI4MDk3Ny4KZGlmZiAtLWdpdCBhL1Nv dXJjZS9XZWJDb3JlL01vZHVsZXMvbWVkaWFzZXNzaW9uL01lZGlhU2Vzc2lvbi5jcHAgYi9Tb3Vy Y2UvV2ViQ29yZS9Nb2R1bGVzL21lZGlhc2Vzc2lvbi9NZWRpYVNlc3Npb24uY3BwCmluZGV4IDUw MDkxMzA0OTA4ODQwMmE2ODRiOTNjMmYxNzZlM2JmZjU0MDdlMzkuLmFjOTA3OGUzOGJjM2E3NWNh YzFkNTFmOGZlNDBlYjI0MjMyZDY2MDIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL01vZHVs ZXMvbWVkaWFzZXNzaW9uL01lZGlhU2Vzc2lvbi5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvTW9k dWxlcy9tZWRpYXNlc3Npb24vTWVkaWFTZXNzaW9uLmNwcApAQCAtNjQsOCArNjQsOCBAQCBzdGF0 aWMgUGxhdGZvcm1NZWRpYVNlc3Npb246OlJlbW90ZUNvbnRyb2xDb21tYW5kVHlwZSBwbGF0Zm9y bUNvbW1hbmRGb3JNZWRpYVNlcwogICAgICAgICAgICAgeyBNZWRpYVNlc3Npb25BY3Rpb246OlBh dXNlLCBQbGF0Zm9ybU1lZGlhU2Vzc2lvbjo6UGF1c2VDb21tYW5kIH0sCiAgICAgICAgICAgICB7 IE1lZGlhU2Vzc2lvbkFjdGlvbjo6U2Vla2ZvcndhcmQsIFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpT a2lwRm9yd2FyZENvbW1hbmQgfSwKICAgICAgICAgICAgIHsgTWVkaWFTZXNzaW9uQWN0aW9uOjpT ZWVrYmFja3dhcmQsIFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpTa2lwQmFja3dhcmRDb21tYW5kIH0s Ci0gICAgICAgICAgICB7IE1lZGlhU2Vzc2lvbkFjdGlvbjo6UHJldmlvdXN0cmFjaywgUGxhdGZv cm1NZWRpYVNlc3Npb246Ok5leHRUcmFja0NvbW1hbmQgfSwKLSAgICAgICAgICAgIHsgTWVkaWFT ZXNzaW9uQWN0aW9uOjpOZXh0dHJhY2ssIFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpQcmV2aW91c1Ry YWNrQ29tbWFuZCB9LAorICAgICAgICAgICAgeyBNZWRpYVNlc3Npb25BY3Rpb246OlByZXZpb3Vz dHJhY2ssIFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpQcmV2aW91c1RyYWNrQ29tbWFuZCB9LAorICAg ICAgICAgICAgeyBNZWRpYVNlc3Npb25BY3Rpb246Ok5leHR0cmFjaywgUGxhdGZvcm1NZWRpYVNl c3Npb246Ok5leHRUcmFja0NvbW1hbmQgfSwKICAgICAgICAgICAgIHsgTWVkaWFTZXNzaW9uQWN0 aW9uOjpTdG9wLCBQbGF0Zm9ybU1lZGlhU2Vzc2lvbjo6U3RvcENvbW1hbmQgfSwKICAgICAgICAg ICAgIHsgTWVkaWFTZXNzaW9uQWN0aW9uOjpTZWVrdG8sIFBsYXRmb3JtTWVkaWFTZXNzaW9uOjpT ZWVrVG9QbGF5YmFja1Bvc2l0aW9uQ29tbWFuZCB9LAogICAgICAgICAgICAgeyBNZWRpYVNlc3Np b25BY3Rpb246OlNraXBhZCwgUGxhdGZvcm1NZWRpYVNlc3Npb246Ok5leHRUcmFja0NvbW1hbmQg fSwK