LayoutTests/media/media-controller-media-fragment-uri-consecutive-append.html - Issue 539103002: Seeking media fragment URI before loadeddata event

Side by Side Diff: LayoutTests/media/media-controller-media-fragment-uri-consecutive-append.html

Issue 539103002: Seeking media fragment URI before loadeddata event (Closed) Base URL: https://chromium.googlesource.com/chromium/blink.git@HAVE_NOTHING

Patch Set: Rebase Created 6 years, 3 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:

View unified diff | Download patch

« LayoutTests/media/media-controller-media-fragment-uri.html ('K') | « LayoutTests/media/media-controller-media-fragment-uri.html ('k') | LayoutTests/media/media-controller-media-fragment-uri-consecutive-append-expected.txt » ('j') | LayoutTests/media/video-currentTime-before-have-metadata-media-fragment-uri.html » ('J')
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <head>

	4 <title>Test currentTime for media controller when media fragments are ad ded consicutively.</title>
	philipj_slow 2014/09/16 12:27:15 Media fragments aren't being added consecutively i Media fragments aren't being added consecutively in this test, because there's only one media fragment added anywhere. I think what this was intended to test is adding a media element which hasn't even reached HAVE_METADATA yet to a media controller with a currentTime > 0 and seeing that the new media element ends up in sync with the media controller. If this is the intention, the test doesn't need to involve media fragments at all. Instead, create a video and set currentTime to 2. In the seeked event, create a media controller and attach it to that element. Then create a second video element and attach it to the media controller. Wait for the seeked event on the second media element and pass the test if in that seeked event all currentTimes are 2. If this was intended to test something else, please rename or otherwise clarify. Typo: consecutively. amogh.bihani 2014/09/16 13:46:01 I wanted to test whether the new media element see Show quoted text On 2014/09/16 12:27:15, philipj wrote: > Media fragments aren't being added consecutively in this test, because there's > only one media fragment added anywhere. I think what this was intended to test > is adding a media element which hasn't even reached HAVE_METADATA yet to a media > controller with a currentTime > 0 and seeing that the new media element ends up > in sync with the media controller. If this is the intention, the test doesn't > need to involve media fragments at all. Instead, create a video and set > currentTime to 2. In the seeked event, create a media controller and attach it > to that element. Then create a second video element and attach it to the media > controller. Wait for the seeked event on the second media element and pass the > test if in that seeked event all currentTimes are 2. > > If this was intended to test something else, please rename or otherwise clarify. > > Typo: consecutively. I wanted to test whether the new media element seeks to the media controller's time when added after a media fragment URI. (Earlier, I had a confusion regarding this in the spec, hence had created the test at that time.) So this test basically is for intitial playback position. Should we have another test for seek in media group as well? philipj_slow 2014/09/16 19:18:15 Thanks, the updated test is easier to understand. Show quoted text On 2014/09/16 13:46:01, amogh.bihani wrote: > On 2014/09/16 12:27:15, philipj wrote: > > Media fragments aren't being added consecutively in this test, because there's > > only one media fragment added anywhere. I think what this was intended to test > > is adding a media element which hasn't even reached HAVE_METADATA yet to a > media > > controller with a currentTime > 0 and seeing that the new media element ends > up > > in sync with the media controller. If this is the intention, the test doesn't > > need to involve media fragments at all. Instead, create a video and set > > currentTime to 2. In the seeked event, create a media controller and attach it > > to that element. Then create a second video element and attach it to the media > > controller. Wait for the seeked event on the second media element and pass the > > test if in that seeked event all currentTimes are 2. > > > > If this was intended to test something else, please rename or otherwise > clarify. > > > > Typo: consecutively. > > I wanted to test whether the new media element seeks to the media controller's > time when added after a media fragment URI. (Earlier, I had a confusion > regarding this in the spec, hence had created the test at that time.) > > So this test basically is for intitial playback position. > > Should we have another test for seek in media group as well? Thanks, the updated test is easier to understand. Since you tested that all of the seeks were covered by some test it's OK by me, but if there is some directed test that you think would add value go ahead.
	5 </head>

	6 <body>

	7 <video controls></video>
	philipj_slow 2014/09/16 12:27:15 The controls attribute is probably not needed here The controls attribute is probably not needed here. amogh.bihani 2014/09/16 13:46:01 Done. Show quoted text On 2014/09/16 12:27:15, philipj wrote: > The controls attribute is probably not needed here. Done.
	8 <video controls></video>

	9 <script src=media-file.js></script>

	10 <script src=video-test.js></script>

	11 <script>

	12 videos = document.getElementsByTagName("video");

	13 video = videos[0];

	14 video2 = videos[1];

	15

	16 var src = findMediaFile("video", "content/test");

	17 video.src = src + "#t=2";

	18 video.mediaGroup = "group";

	19 video2.mediaGroup = "group";

	20 controller = video.controller;

	21

	22 setTimeout(function() {
	philipj_slow 2014/09/16 12:27:15 In general, never use setTimeout unless there's no In general, never use setTimeout unless there's no other way, it's a good way to create flaky tests. In this case I think you want to wait until the first video is loaded and has seeked, in which case waiting for the seeked event is more direct. amogh.bihani 2014/09/16 13:46:01 Done. Show quoted text On 2014/09/16 12:27:15, philipj wrote: > In general, never use setTimeout unless there's no other way, it's a good way to > create flaky tests. In this case I think you want to wait until the first video > is loaded and has seeked, in which case waiting for the seeked event is more > direct. Done.
	23 video2.src= src;

	24 }, 500);

	25

	26 video.addEventListener("loadeddata", function() {

	27 testExpected("video.currentTime", 2);

	28 testExpected("video2.currentTime", 0);

	29 });

	30 video2.addEventListener("loadeddata", function() {

	31 testExpected("video.currentTime", 2);

	32 testExpected("video2.currentTime", 2);

	33 endTest();

	34 })

	35 </script>

	36 </body>

	37 </html>

OLD	NEW