third_party/WebKit/LayoutTests/media/autoplay-when-visible-multiple-times.html - Issue 2654123002: Autoplay muted video when visible and pause when invisible

Side by Side Diff: third_party/WebKit/LayoutTests/media/autoplay-when-visible-multiple-times.html

Issue 2654123002: Autoplay muted video when visible and pause when invisible (Closed)

Patch Set: finalizing implementation and tests Created 3 years, 10 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <title>Test behaviour of autoplay muted videos with regards to visibility</title >

	3 <script src="../resources/testharness.js"></script>

	4 <script src="../resources/testharnessreport.js"></script>

	5 <script src="media-file.js"></script>

	6 <body>

	7 <script>

	8 window.internals.settings.setMediaPlaybackRequiresUserGesture(true);

	9 window.internals.runtimeFlags.autoplayMutedVideosEnabled = true;

	10

	11 var expectations = [ 'paused', 'playing', 'paused', 'playing', 'paused', 'play ing', 'paused' ];
	whywhat 2017/02/13 16:50:15 I don't think this is needed at all: - you know wh I don't think this is needed at all: - you know what to expect when visible/invisible - you can setup the onplay/onpause handlers in runWhenVisible and runWhenInvisible to guarantee that the right event happens - perhaps just two transitions is enough (hidden & paused -> visible & playing -> hidden & paused again)? Zhiqiang Zhang (Slow) 2017/02/13 17:48:05 Now add/remove event listeners to ensure the event Show quoted text On 2017/02/13 16:50:15, whywhat_slow_PST_till_Feb_6 wrote: > I don't think this is needed at all: > - you know what to expect when visible/invisible > - you can setup the onplay/onpause handlers in runWhenVisible and > runWhenInvisible to guarantee that the right event happens > - perhaps just two transitions is enough (hidden & paused -> visible & playing > -> hidden & paused again)? Now add/remove event listeners to ensure the events happen in the correct order. I still prefer test multiple times. Now I'm using a counter and the test passes if the counter reaches 3.
	12 var expectationId = 0;

	13

	14 function checkExpectations(t, observed) {

	15 assert_equals(expectations[expectationId++], observed);

	16 if (expectationId >= expectations.length)

	17 t.done();

	18 }

	19

	20 var video;

	21

	22 function runStepsWhenInvisible(t) {

	23 assert_true(video.paused);

	24 checkExpectations(t, 'paused');
	whywhat 2017/02/13 16:50:15 you could simplify these two lines (ditto below) b you could simplify these two lines (ditto below) by making the expectations[] a bool array as in expectedPausedValue = [ true, false, true, ..., true ] and do assert_equals(expectedPausedValue[id++], video.paused) in the check above. Zhiqiang Zhang (Slow) 2017/02/13 17:48:05 This no longer applies as removed. Show quoted text On 2017/02/13 16:50:15, whywhat_slow_PST_till_Feb_6 wrote: > you could simplify these two lines (ditto below) by making the expectations[] a > bool array as in expectedPausedValue = [ true, false, true, ..., true ] and do > assert_equals(expectedPausedValue[id++], video.paused) in the check above. This no longer applies as removed.
	25 video.style.top = '0px';

	26 }

	27

	28 function runStepsWhenVisible(t) {

	29 assert_false(video.paused);

	30 checkExpectations(t, 'playing');

	31 video.style.top = '-10000px';

	32 }

	33

	34 async_test(function(t) {

	35 // Create a video off screen.

	36 {

	37 video = document.createElement('video');

	38 video.src = findMediaFile('video', 'content/test');

	39 video.muted = true;

	40 video.autoplay = true;

	41 video.loop = true;

	42 video.style.position = 'absolute';

	43 video.style.top = '-10000px';

	44 document.body.appendChild(video);

	45 }

	46

	47 video.addEventListener('canplay', t.step_func(_ => runStepsWhenInvisible(t)) );
	whywhat 2017/02/13 16:50:15 will this event fire only once during this test? m will this event fire only once during this test? maybe you should remove the handler on the first fire? Zhiqiang Zhang (Slow) 2017/02/13 17:48:05 Done. Show quoted text On 2017/02/13 16:50:15, whywhat_slow_PST_till_Feb_6 wrote: > will this event fire only once during this test? maybe you should remove the > handler on the first fire? Done.
	48 video.addEventListener('play', t.step_func(_ => runStepsWhenVisible(t)));
	whywhat 2017/02/13 16:50:15 I'd rather have a visibility observer to check the I'd rather have a visibility observer to check the paused state instead of relying on the code under test. When the video is visible, check it's not paused and make it invisible, when invisible, check it's paused and make it visible. Zhiqiang Zhang (Slow) 2017/02/13 17:48:05 I prefer not to use visibility observer as the int Show quoted text On 2017/02/13 16:50:15, whywhat_slow_PST_till_Feb_6 wrote: > I'd rather have a visibility observer to check the paused state instead of > relying on the code under test. When the video is visible, check it's not paused > and make it invisible, when invisible, check it's paused and make it visible. I prefer not to use visibility observer as the internal observer in HTMLMediaElement might receive the event later, so it's not guaranteed that onVisibilityChangedForAutoplay() be called. Listening to "play" and "pause" is fine. If there's a bug, the test should time out.
	49 video.addEventListener('pause', t.step_func(_ => runStepsWhenInvisible(t)));

	50 });

	51 </script>

OLD	NEW

« no previous file with comments | « no previous file | third_party/WebKit/Source/core/html/HTMLMediaElement.h » ('j') | no next file with comments »