147416 2015-07-29 12:31:58 -0700 Media Session: test Next/Previous Track media control events delivered to Content media sessions 2015-07-30 10:22:58 -0700 1 1 1 Unclassified WebKit Media 528+ (Nightly build) All All RESOLVED FIXED InRadar P2 Normal --- 145411 1 mrajca webkit-unassigned conrad_shultz eric.carlson mrajca webkit-bug-importer oldest_to_newest 1113141 0 mrajca 2015-07-29 12:31:58 -0700 Add tests to ensure that active Content media sessions respond to Next/Previous Track media control events. 1113142 1 webkit-bug-importer 2015-07-29 12:32:34 -0700 <rdar://problem/22056719> 1113195 2 257777 mrajca 2015-07-29 15:27:53 -0700 Created attachment 257777 Patch 1113349 3 257777 eric.carlson 2015-07-30 07:12:26 -0700 Comment on attachment 257777 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=257777&action=review > LayoutTests/media/session/track-media-events-in-content-sessions.html:16 > + controls.addEventListener("nexttrack", skipToNextTrack, false); > + controls.addEventListener("previoustrack", skipToPreviousTrack, false); Nit: in case you don't know you can use waitForEvent on any object to get the automatic logging it adds. For example: waitForEvent('nexttrack', skipToNextTrack, false, false, controls) > LayoutTests/media/session/track-media-events-in-content-sessions.html:30 > + function beginPlaying(event) > + { > + if (window.internals) > + testExpected('internals.mediaSessionCurrentState(session)', "idle"); > + > + testExpected('video.paused', true); Nit: If you want to add the event listener with "video.onplaying", I think it will make the test results easier to understand if you log the event name as you would if you used waitForEvent, eg: consoleWrite("EVENT(" + event.type + ")"); > LayoutTests/media/session/track-media-events-in-content-sessions.html:33 > + consoleWrite("Playing media."); > + video.play(); Nit: "Begin playing media" would be more correct. Alternatively you could use "run('video.play()')" instead. > LayoutTests/media/session/track-media-events-in-content-sessions.html:38 > + video.onplaying = null; Nit: "waitForEvent('playing', beganPlaying, false, true)" would make this unnecessary. > LayoutTests/media/session/track-media-events-in-content-sessions.html:43 > + consoleWrite("Active Media Sessions should respond to Previous/Next Track events."); Nit: This is essentially the same as the paragraph in the body so I don't think it is necessary. > LayoutTests/media/session/track-media-events-in-content-sessions.html:57 > + function skipToNextTrack(event) > + { > + consoleWrite("Sending Previous Track media event."); > + run('internals.sendMediaControlEvent("previous-track")'); > + } Nit: If you want to add the event listener with addEventListener(), I think it will make the test results easier to understand if you log the event name as you would if you used waitForEvent, eg: consoleWrite("EVENT(" + event.type + ")"); > LayoutTests/media/session/track-media-events-in-content-sessions.html:62 > + function skipToPreviousTrack(event) > + { > + endTest(); > + } Ditto. 1113396 4 mrajca 2015-07-30 10:21:50 -0700 (In reply to comment #3) > Comment on attachment 257777 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=257777&action=review > > > LayoutTests/media/session/track-media-events-in-content-sessions.html:16 > > + controls.addEventListener("nexttrack", skipToNextTrack, false); > > + controls.addEventListener("previoustrack", skipToPreviousTrack, false); > > Nit: in case you don't know you can use waitForEvent on any object to get > the automatic logging it adds. For example: > > waitForEvent('nexttrack', skipToNextTrack, false, false, controls) I switched to `waitForEvent`. > > > LayoutTests/media/session/track-media-events-in-content-sessions.html:30 > > + function beginPlaying(event) > > + { > > + if (window.internals) > > + testExpected('internals.mediaSessionCurrentState(session)', "idle"); > > + > > + testExpected('video.paused', true); > > Nit: If you want to add the event listener with "video.onplaying", I think > it will make the test results easier to understand if you log the event name > as you would if you used waitForEvent, eg: > > consoleWrite("EVENT(" + event.type + ")"); Switched to `waitForEvent`. > > > LayoutTests/media/session/track-media-events-in-content-sessions.html:33 > > + consoleWrite("Playing media."); > > + video.play(); > > Nit: "Begin playing media" would be more correct. Fixed. > Alternatively you could use "run('video.play()')" instead. > > > LayoutTests/media/session/track-media-events-in-content-sessions.html:38 > > + video.onplaying = null; > > Nit: "waitForEvent('playing', beganPlaying, false, true)" would make this > unnecessary. Switched for `waitForEvent`. > > > LayoutTests/media/session/track-media-events-in-content-sessions.html:43 > > + consoleWrite("Active Media Sessions should respond to Previous/Next Track events."); > > Nit: This is essentially the same as the paragraph in the body so I don't > think it is necessary. Removed. > > > LayoutTests/media/session/track-media-events-in-content-sessions.html:57 > > + function skipToNextTrack(event) > > + { > > + consoleWrite("Sending Previous Track media event."); > > + run('internals.sendMediaControlEvent("previous-track")'); > > + } > > Nit: If you want to add the event listener with addEventListener(), I think > it will make the test results easier to understand if you log the event name > as you would if you used waitForEvent, eg: > > consoleWrite("EVENT(" + event.type + ")"); Switched to `waitForEvent`. > > > LayoutTests/media/session/track-media-events-in-content-sessions.html:62 > > + function skipToPreviousTrack(event) > > + { > > + endTest(); > > + } > > Ditto. Switched to `waitForEvent`. 1113397 5 mrajca 2015-07-30 10:22:58 -0700 Committed r187589: <http://trac.webkit.org/changeset/187589> 257777 2015-07-29 15:27:53 -0700 2015-07-30 07:12:26 -0700 Patch bug-147416-20150729152630.patch text/plain 5319 mrajca U3VidmVyc2lvbiBSZXZpc2lvbjogMTg3NTUyCmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9DaGFu Z2VMb2cgYi9MYXlvdXRUZXN0cy9DaGFuZ2VMb2cKaW5kZXggNTA0YTcxM2U1YzEzYzAxMjQ3NWE5 YTRlMmQ5NWJlOTc5YTc3NTg4Ni4uOTE0OWVjNWRiZGQ5ZTEwMTA0NzIyZjU0YWI2NDVjNjI1N2Jk ZmQwOSAxMDA2NDQKLS0tIGEvTGF5b3V0VGVzdHMvQ2hhbmdlTG9nCisrKyBiL0xheW91dFRlc3Rz L0NoYW5nZUxvZwpAQCAtMSw1ICsxLDE2IEBACisyMDE1LTA3LTI5ICBNYXR0IFJhamNhICA8bXJh amNhQGFwcGxlLmNvbT4KKworICAgICAgICBNZWRpYSBTZXNzaW9uOiB0ZXN0IE5leHQvUHJldmlv dXMgVHJhY2sgbWVkaWEgY29udHJvbCBldmVudHMgZGVsaXZlcmVkIHRvIENvbnRlbnQgbWVkaWEg c2Vzc2lvbnMKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lk PTE0NzQxNgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAg ICogbWVkaWEvc2Vzc2lvbi90cmFjay1tZWRpYS1ldmVudHMtaW4tY29udGVudC1zZXNzaW9ucy1l eHBlY3RlZC50eHQ6IEFkZGVkLgorICAgICAgICAqIG1lZGlhL3Nlc3Npb24vdHJhY2stbWVkaWEt ZXZlbnRzLWluLWNvbnRlbnQtc2Vzc2lvbnMuaHRtbDogQWRkZWQuCisgICAgICAgICogcGxhdGZv cm0vbWFjL1Rlc3RFeHBlY3RhdGlvbnM6IE1lZGlhIFNlc3Npb24gc3VwcG9ydCBpcyBkaXNhYmxl ZCBieSBkZWZhdWx0LgorCiAyMDE1LTA3LTI5ICBNYXR0IFJhamNhICA8bXJhamNhQGFwcGxlLmNv bT4KIAogICAgICAgICBNZWRpYSBTZXNzaW9uOiB0ZXN0IFBsYXkvUGF1c2UgbWVkaWEgY29udHJv bCBldmVudHMgZGVsaXZlcmVkIHRvIG5vbi1Db250ZW50IG1lZGlhIHNlc3Npb25zCiAgICAgICAg IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNDc0MTQKIApkaWZmIC0t Z2l0IGEvTGF5b3V0VGVzdHMvbWVkaWEvc2Vzc2lvbi90cmFjay1tZWRpYS1ldmVudHMtaW4tY29u dGVudC1zZXNzaW9ucy1leHBlY3RlZC50eHQgYi9MYXlvdXRUZXN0cy9tZWRpYS9zZXNzaW9uL3Ry YWNrLW1lZGlhLWV2ZW50cy1pbi1jb250ZW50LXNlc3Npb25zLWV4cGVjdGVkLnR4dApuZXcgZmls ZSBtb2RlIDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAw MDAwLi4wMmNlODdhODViMDVhZDZhMmExZGRjZjEyZWVhNWUzYzkzOTZhZDYwCi0tLSAvZGV2L251 bGwKKysrIGIvTGF5b3V0VGVzdHMvbWVkaWEvc2Vzc2lvbi90cmFjay1tZWRpYS1ldmVudHMtaW4t Y29udGVudC1zZXNzaW9ucy1leHBlY3RlZC50eHQKQEAgLTAsMCArMSwxNyBAQAorJ0NvbnRlbnQn IHNlc3Npb25zIHRoYXQgYXJlIGFjdGl2ZSBzaG91bGQgcmVzcG9uZCB0byBQcmV2aW91cy9OZXh0 IFRyYWNrIG1lZGlhIGNvbnRyb2wgZXZlbnRzLgorCisKK0VWRU5UKGNhbnBsYXl0aHJvdWdoKQor RVhQRUNURUQgKGludGVybmFscy5tZWRpYVNlc3Npb25DdXJyZW50U3RhdGUoc2Vzc2lvbikgPT0g J2lkbGUnKSBPSworRVhQRUNURUQgKHZpZGVvLnBhdXNlZCA9PSAndHJ1ZScpIE9LCitQbGF5aW5n IG1lZGlhLgorTWVkaWEgYmVnYW4gcGxheWluZy4KK0VYUEVDVEVEICh2aWRlby5wYXVzZWQgPT0g J2ZhbHNlJykgT0sKK0FjdGl2ZSBNZWRpYSBTZXNzaW9ucyBzaG91bGQgcmVzcG9uZCB0byBQcmV2 aW91cy9OZXh0IFRyYWNrIGV2ZW50cy4KK0VYUEVDVEVEIChpbnRlcm5hbHMubWVkaWFTZXNzaW9u Q3VycmVudFN0YXRlKHNlc3Npb24pID09ICdhY3RpdmUnKSBPSworU2VuZGluZyBOZXh0IFRyYWNr IG1lZGlhIGV2ZW50LgorUlVOKGludGVybmFscy5zZW5kTWVkaWFDb250cm9sRXZlbnQoIm5leHQt dHJhY2siKSkKK1NlbmRpbmcgUHJldmlvdXMgVHJhY2sgbWVkaWEgZXZlbnQuCitSVU4oaW50ZXJu YWxzLnNlbmRNZWRpYUNvbnRyb2xFdmVudCgicHJldmlvdXMtdHJhY2siKSkKK0VORCBPRiBURVNU CisKZGlmZiAtLWdpdCBhL0xheW91dFRlc3RzL21lZGlhL3Nlc3Npb24vdHJhY2stbWVkaWEtZXZl bnRzLWluLWNvbnRlbnQtc2Vzc2lvbnMuaHRtbCBiL0xheW91dFRlc3RzL21lZGlhL3Nlc3Npb24v dHJhY2stbWVkaWEtZXZlbnRzLWluLWNvbnRlbnQtc2Vzc2lvbnMuaHRtbApuZXcgZmlsZSBtb2Rl IDEwMDY0NAppbmRleCAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwMDAwLi42 YjA5NTFkYTNkOWU4MzBmZDA3MzIyZmQzOWVlZGRlNTA3YTM3YjZmCi0tLSAvZGV2L251bGwKKysr IGIvTGF5b3V0VGVzdHMvbWVkaWEvc2Vzc2lvbi90cmFjay1tZWRpYS1ldmVudHMtaW4tY29udGVu dC1zZXNzaW9ucy5odG1sCkBAIC0wLDAgKzEsNjkgQEAKKzwhRE9DVFlQRSBodG1sPgorPGh0bWw+ Cis8aGVhZD4KKyAgICA8c2NyaXB0IHNyYz0iLi4vdmlkZW8tdGVzdC5qcyI+PC9zY3JpcHQ+Cisg ICAgPHNjcmlwdCBzcmM9Ii4uL21lZGlhLWZpbGUuanMiPjwvc2NyaXB0PgorICAgIDxzY3JpcHQg dHlwZT0idGV4dC9qYXZhc2NyaXB0Ij4KKyAgICAgICAgdmFyIHNlc3Npb24sIGNvbnRyb2xzOwor CisgICAgICAgIGZ1bmN0aW9uIHJ1blRlc3QoKSAKKyAgICAgICAgeworICAgICAgICAgICAgc2Vz c2lvbiA9IG5ldyBNZWRpYVNlc3Npb24oImNvbnRlbnQiKTsKKyAgICAgICAgICAgIGNvbnRyb2xz ID0gc2Vzc2lvbi5jb250cm9sczsKKyAgICAgICAgICAgIGNvbnRyb2xzLm5leHRUcmFja0VuYWJs ZWQgPSB0cnVlOworICAgICAgICAgICAgY29udHJvbHMucHJldmlvdXNUcmFja0VuYWJsZWQgPSB0 cnVlOworICAgICAgICAgICAgY29udHJvbHMuYWRkRXZlbnRMaXN0ZW5lcigibmV4dHRyYWNrIiwg c2tpcFRvTmV4dFRyYWNrLCBmYWxzZSk7CisgICAgICAgICAgICBjb250cm9scy5hZGRFdmVudExp c3RlbmVyKCJwcmV2aW91c3RyYWNrIiwgc2tpcFRvUHJldmlvdXNUcmFjaywgZmFsc2UpOworCisg ICAgICAgICAgICBmaW5kTWVkaWFFbGVtZW50KCk7CisgICAgICAgICAgICB3YWl0Rm9yRXZlbnQo J2NhbnBsYXl0aHJvdWdoJywgYmVnaW5QbGF5aW5nKTsKKyAgICAgICAgICAgIHZpZGVvLnNyYyA9 IGZpbmRNZWRpYUZpbGUoInZpZGVvIiwgIi4uL2NvbnRlbnQvY291bnRpbmciKTsKKyAgICAgICAg ICAgIHZpZGVvLnNlc3Npb24gPSBzZXNzaW9uOworICAgICAgICAgICAgdmlkZW8ub25wbGF5aW5n ID0gYmVnYW5QbGF5aW5nOworICAgICAgICB9CisKKyAgICAgICAgZnVuY3Rpb24gYmVnaW5QbGF5 aW5nKGV2ZW50KQorICAgICAgICB7CisgICAgICAgICAgICBpZiAod2luZG93LmludGVybmFscykK KyAgICAgICAgICAgICAgICB0ZXN0RXhwZWN0ZWQoJ2ludGVybmFscy5tZWRpYVNlc3Npb25DdXJy ZW50U3RhdGUoc2Vzc2lvbiknLCAiaWRsZSIpOworCisgICAgICAgICAgICB0ZXN0RXhwZWN0ZWQo J3ZpZGVvLnBhdXNlZCcsIHRydWUpOworCisgICAgICAgICAgICBjb25zb2xlV3JpdGUoIlBsYXlp bmcgbWVkaWEuIik7CisgICAgICAgICAgICB2aWRlby5wbGF5KCk7CisgICAgICAgIH0KKworICAg ICAgICBmdW5jdGlvbiBiZWdhblBsYXlpbmcoZXZlbnQpCisgICAgICAgIHsKKyAgICAgICAgICAg IHZpZGVvLm9ucGxheWluZyA9IG51bGw7CisKKyAgICAgICAgICAgIGNvbnNvbGVXcml0ZSgiTWVk aWEgYmVnYW4gcGxheWluZy4iKTsKKyAgICAgICAgICAgIHRlc3RFeHBlY3RlZCgndmlkZW8ucGF1 c2VkJywgZmFsc2UpOworCisgICAgICAgICAgICBjb25zb2xlV3JpdGUoIkFjdGl2ZSBNZWRpYSBT ZXNzaW9ucyBzaG91bGQgcmVzcG9uZCB0byBQcmV2aW91cy9OZXh0IFRyYWNrIGV2ZW50cy4iKTsK KyAgICAgICAgICAgICAgICAgICAgICAgIAorICAgICAgICAgICAgaWYgKHdpbmRvdy5pbnRlcm5h bHMpIHsKKyAgICAgICAgICAgICAgICB0ZXN0RXhwZWN0ZWQoJ2ludGVybmFscy5tZWRpYVNlc3Np b25DdXJyZW50U3RhdGUoc2Vzc2lvbiknLCAiYWN0aXZlIik7CisKKyAgICAgICAgICAgICAgICBj b25zb2xlV3JpdGUoIlNlbmRpbmcgTmV4dCBUcmFjayBtZWRpYSBldmVudC4iKTsKKyAgICAgICAg ICAgICAgICBydW4oJ2ludGVybmFscy5zZW5kTWVkaWFDb250cm9sRXZlbnQoIm5leHQtdHJhY2si KScpOworICAgICAgICAgICAgfQorICAgICAgICB9CisgICAgICAgIAorICAgICAgICBmdW5jdGlv biBza2lwVG9OZXh0VHJhY2soZXZlbnQpCisgICAgICAgIHsKKyAgICAgICAgICAgIGNvbnNvbGVX cml0ZSgiU2VuZGluZyBQcmV2aW91cyBUcmFjayBtZWRpYSBldmVudC4iKTsKKyAgICAgICAgICAg IHJ1bignaW50ZXJuYWxzLnNlbmRNZWRpYUNvbnRyb2xFdmVudCgicHJldmlvdXMtdHJhY2siKScp OworICAgICAgICB9CisgICAgICAgIAorICAgICAgICBmdW5jdGlvbiBza2lwVG9QcmV2aW91c1Ry YWNrKGV2ZW50KQorICAgICAgICB7CisgICAgICAgICAgICBlbmRUZXN0KCk7CisgICAgICAgIH0K KyAgICA8L3NjcmlwdD4KKzwvaGVhZD4KKzxib2R5IG9ubG9hZD0icnVuVGVzdCgpIj4KKyAgICA8 cD4nQ29udGVudCcgc2Vzc2lvbnMgdGhhdCBhcmUgYWN0aXZlIHNob3VsZCByZXNwb25kIHRvIFBy ZXZpb3VzL05leHQgVHJhY2sgbWVkaWEgY29udHJvbCBldmVudHMuPC9wPgorICAgIDx2aWRlbyBp ZD0idmlkZW8iIC8+Cis8L2JvZHk+Cis8L2h0bWw+CmRpZmYgLS1naXQgYS9MYXlvdXRUZXN0cy9w bGF0Zm9ybS9tYWMvVGVzdEV4cGVjdGF0aW9ucyBiL0xheW91dFRlc3RzL3BsYXRmb3JtL21hYy9U ZXN0RXhwZWN0YXRpb25zCmluZGV4IDhhMzlmMjY5NTBiZjU3MDRiNWJmNjkyZGZlMDQ4NmZjZTQx MjBkMDIuLjAzNmFmYTI3ODYwMzU0YzY1ZjMyZGQ3OTM1ZGMxMGZhY2ExZmEwOTQgMTAwNjQ0Ci0t LSBhL0xheW91dFRlc3RzL3BsYXRmb3JtL21hYy9UZXN0RXhwZWN0YXRpb25zCisrKyBiL0xheW91 dFRlc3RzL3BsYXRmb3JtL21hYy9UZXN0RXhwZWN0YXRpb25zCkBAIC05OTksNiArOTk5LDcgQEAg bWVkaWEvc2Vzc2lvbi9jb250cm9scy1leGlzdGVuY2UuaHRtbAogbWVkaWEvc2Vzc2lvbi9kZWZh dWx0LXNlc3Npb24uaHRtbAogbWVkaWEvc2Vzc2lvbi9wbGF5LXBhdXNlLW1lZGlhLWV2ZW50cy1p bi1jb250ZW50LXNlc3Npb25zLmh0bWwKIG1lZGlhL3Nlc3Npb24vcGxheS1wYXVzZS1tZWRpYS1l dmVudHMtaW4tbm9uLWNvbnRlbnQtc2Vzc2lvbnMuaHRtbAorbWVkaWEvc2Vzc2lvbi90cmFjay1t ZWRpYS1ldmVudHMtaW4tY29udGVudC1zZXNzaW9ucy5odG1sCiBtZWRpYS9zZXNzaW9uL3RyYW5z aWVudC1pbnRlcnJ1cHRpb25zLmh0bWwKIG1lZGlhL3Nlc3Npb24vdHJhbnNpZW50LXNvbG8taW50 ZXJydXB0aW9ucy5odG1sCiAK