third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-attach-stops-delaying-load-event.html - Issue 2021573002: MSE: Reset delaying-the-load-event-flag on attachment

Side by Side Diff: third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-attach-stops-delaying-load-event.html

Issue 2021573002: MSE: Reset delaying-the-load-event-flag on attachment (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Addressed mlamouri's comment intent: load must occur before sourceopen in test Created 4 years, 6 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

OLD	NEW
(Empty)
	1 <!DOCTYPE html>

	2 <html>

	3 <head>

	4 <script src="/w3c/resources/testharness.js"></script>

	5 <script src="/w3c/resources/testharnessreport.js"></script>

	6 </head>

	7 <body>

	8 <script>

	9 async_test(function(test)

	10 {

	11 var receivedLoadEvent = false;

	12

	13 window.addEventListener("load", test.step_func(function() {

	14 assert_false(receivedLoadEvent, "window should not receive m ultiple load events");

	15 receivedLoadEvent = true;

	16 }), false);
	foolip 2016/06/20 09:30:12 Trailing false is not needed. Trailing false is not needed. wolenetz 2016/06/21 22:25:47 Done. Show quoted text On 2016/06/20 09:30:12, Philip Jägenstedt wrote: > Trailing false is not needed. Done.
	17

	18 assert_equals(document.readyState, "loading", "document should n ot be complete yet");

	19 var video = document.createElement("video");

	20 var mediaSource = new MediaSource();

	21

	22 // \|video\| should stop delaying the load event long before eithe r a

	23 // "progress", "stalled" or "suspend" event are enqueued.

	24 video.addEventListener("suspend", test.unreached_func("unexpecte d 'suspend' event"));

	25 video.addEventListener("stalled", test.unreached_func("unexpecte d 'stalled' event"));

	26 video.addEventListener("progress", test.unreached_func("unexpect ed 'progress' event"));

	27

	28 // No error is expected.

	29 video.addEventListener("error", test.unreached_func("unexpected 'error' event"));

	30

	31 mediaSource.addEventListener("sourceopen", test.step_func(functi on() {

	32 assert_true(receivedLoadEvent, "load event should have been received first");
	foolip 2016/06/20 09:30:12 It looks to me like the implementation would first It looks to me like the implementation would first fire the window load event and then the mediaSource's sourceopen event, but the spec requires the opposite. That's because https://html.spec.whatwg.org/#delay-the-load-event waits until after the task that causes the load event to not be delayed before queuing the task, but Document::checkLoadEventSoon() would start that timer while the task is still running. This seems like a detail that could easily be different in different implementations, so sharing a test like this with others would be great :) wolenetz 2016/06/21 22:25:47 Acknowledged. Show quoted text On 2016/06/20 09:30:12, Philip Jägenstedt wrote: > It looks to me like the implementation would first fire the window load event > and then the mediaSource's sourceopen event, but the spec requires the opposite. > > That's because https://html.spec.whatwg.org/#delay-the-load-event waits until > after the task that causes the load event to not be delayed before queuing the > task, but Document::checkLoadEventSoon() would start that timer while the task > is still running. > > This seems like a detail that could easily be different in different > implementations, so sharing a test like this with others would be great :) Acknowledged. wolenetz 2016/09/02 23:00:57 I've filed https://crbug.com/643846 to follow-up o Show quoted text On 2016/06/21 22:25:47, wolenetz wrote: > On 2016/06/20 09:30:12, Philip Jägenstedt wrote: > > It looks to me like the implementation would first fire the window load event > > and then the mediaSource's sourceopen event, but the spec requires the > opposite. > > > > That's because https://html.spec.whatwg.org/#delay-the-load-event waits until > > after the task that causes the load event to not be delayed before queuing the > > task, but Document::checkLoadEventSoon() would start that timer while the task > > is still running. > > > > This seems like a detail that could easily be different in different > > implementations, so sharing a test like this with others would be great :) > > Acknowledged. I've filed https://crbug.com/643846 to follow-up on at least Chrome's impl which mismatches spec.
	33 assert_equals(video.networkState, video.NETWORK_LOADING);

	34 assert_equals(video.readyState, video.HAVE_NOTHING);

	35 assert_equals(document.readyState, "complete", "document sho uld be complete");

	36 test.done();

	37 }));

	38

	39 var mediaSourceURL = URL.createObjectURL(mediaSource);

	40 document.body.appendChild(video);
	foolip 2016/06/20 09:30:12 Test should run fine without appending unless ther Test should run fine without appending unless there's a GC bug? wolenetz 2016/06/21 22:25:47 Done. And reconfirmed that without the .cpp portio Show quoted text On 2016/06/20 09:30:12, Philip Jägenstedt wrote: > Test should run fine without appending unless there's a GC bug? Done. And reconfirmed that without the .cpp portion of this change, this test fails because load happens after sourceopen.
	41 video.src = mediaSourceURL;

	42 test.add_cleanup(function() {

	43 document.body.removeChild(video);
	foolip 2016/06/20 09:30:12 Removing the video from the document ought not mat Removing the video from the document ought not matter unless there's a leak or something to work around. wolenetz 2016/06/21 22:25:47 Done. Show quoted text On 2016/06/20 09:30:12, Philip Jägenstedt wrote: > Removing the video from the document ought not matter unless there's a leak or > something to work around. Done.
	44 URL.revokeObjectURL(mediaSourceURL);

	45 });

	46 }, "MediaSource attachment should immediately stop delaying the load event");

	47 </script>

	48 </body>

	49 </html>

OLD	NEW

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