third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-attach-preload.html - Issue 1881733004: MSE, MS, and any blob URL: Ignore preload 'none' on resource fetching

Side by Side Diff: third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-attach-preload.html

Issue 1881733004: MSE, MS, and any blob URL: Ignore preload 'none' on resource fetching (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Fixes indentation in test script Created 4 years, 8 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 <script src="mediasource-util.js"></script>

	7 <link rel='stylesheet' href='/w3c/resources/testharness.css'>
	Srirama 2016/04/14 05:13:07 nit: may be use double quotes here to be consisten nit: may be use double quotes here to be consistent. wolenetz 2016/04/21 21:37:30 Copy-paste from another media-source layout test. Show quoted text On 2016/04/14 05:13:07, Srirama wrote: > nit: may be use double quotes here to be consistent. Copy-paste from another media-source layout test. I'll fix those in a different cleanup CL (possibly as part of the round-tripping of these layout test to w3c/web-platform-tests and back into blink imported.)
	8 </head>

	9 <body>

	10 <div id="log"></div>
	Srirama 2016/04/14 05:13:07 do we need this div? do we need this div? philipj_slow 2016/04/14 13:16:30 Nope :) Show quoted text On 2016/04/14 05:13:07, Srirama wrote: > do we need this div? Nope :) wolenetz 2016/04/21 21:37:30 Yet another copy-paste from another media-source l Show quoted text On 2016/04/14 13:16:30, philipj_slow wrote: > On 2016/04/14 05:13:07, Srirama wrote: > > do we need this div? > > Nope :) Yet another copy-paste from another media-source layout test. I'll fix those in a different cleanup CL (possibly as part of the round-tripping of these layout test to w3c/web-platform-tests and back into blink imported.)
	11 <script>

	12 function attachWithPreloadTest(preload)

	13 {

	14 media_test(function(test)

	15 {

	16 var mediaTag = document.createElement('video');
	philipj_slow 2016/04/14 13:16:30 s/mediaTag/video/ since it's always a video s/mediaTag/video/ since it's always a video wolenetz 2016/04/21 21:37:30 Done. Show quoted text On 2016/04/14 13:16:30, philipj_slow wrote: > s/mediaTag/video/ since it's always a video Done.
	17 if (preload)

	18 mediaTag.preload = preload;

	19 document.body.appendChild(mediaTag);

	20

	21 // Overload done() so that elements added to the document can be

	22 // removed.

	23 var removeMediaElement = true;

	24 var oldTestDone = test.done.bind(test);

	25 test.done = function()

	26 {

	27 if (removeMediaElement) {
	philipj_slow 2016/04/14 13:16:30 It looks like what you want for this is test.add_c It looks like what you want for this is test.add_cleanup, you shouldn't need to replace the done method. You also shouldn't need the removeMediaElement bookkeeping with that. wolenetz 2016/04/21 21:37:30 Yet another copy-paste from another media-source l Show quoted text On 2016/04/14 13:16:30, philipj_slow wrote: > It looks like what you want for this is test.add_cleanup, you shouldn't need to > replace the done method. You also shouldn't need the removeMediaElement > bookkeeping with that. Yet another copy-paste from another media-source layout test (and from similar in mediasource-util.js). I'll fix those in a different cleanup CL (possibly as part of the round-tripping of these layout test to w3c/web-platform-tests and back into blink imported.)
	28 document.body.removeChild(mediaTag);

	29 removeMediaElements = false;

	30 }

	31 oldTestDone();

	32 };

	33

	34 var mediaSource = new MediaSource();

	35 var mediaSourceURL = URL.createObjectURL(mediaSource);

	36 test.expectEvent(mediaSource, 'sourceopen');

	37 mediaTag.src = mediaSourceURL;

	38 test.waitForExpectedEvents(function()
	philipj_slow 2016/04/14 13:16:30 If this is the only reason you need yo use media_t If this is the only reason you need yo use media_test, then just mediaSource.onsourceopen = test.step_func(...) would save a line and remove the extra dependency :) wolenetz 2016/04/21 21:37:30 Done, though "mediaSource.onsourceopen = ..." didn Show quoted text On 2016/04/14 13:16:30, philipj_slow wrote: > If this is the only reason you need yo use media_test, then just > mediaSource.onsourceopen = test.step_func(...) would save a line and remove the > extra dependency :) Done, though "mediaSource.onsourceopen = ..." didn't work. We've used addEventListener/removeEventListener solely throughout media-source layout tests. Is "onsourceopen" attribute setting expected to work for MSE? If so, seems there's a bug to fix here too :) philipj_slow 2016/04/22 09:22:15 Ah, I just assumed that the MediaSource interface Show quoted text On 2016/04/21 21:37:30, wolenetz wrote: > On 2016/04/14 13:16:30, philipj_slow wrote: > > If this is the only reason you need yo use media_test, then just > > mediaSource.onsourceopen = test.step_func(...) would save a line and remove > the > > extra dependency :) > > Done, though "mediaSource.onsourceopen = ..." didn't work. We've used > addEventListener/removeEventListener solely throughout media-source layout > tests. Is "onsourceopen" attribute setting expected to work for MSE? If so, > seems there's a bug to fix here too :) Ah, I just assumed that the MediaSource interface would have event handler attributes, but it doesn't. Filed https://github.com/w3c/media-source/issues/66 so addEventListener will do for now. wolenetz 2016/04/25 19:43:21 Acknowledged. Show quoted text On 2016/04/22 09:22:15, philipj_slow wrote: > On 2016/04/21 21:37:30, wolenetz wrote: > > On 2016/04/14 13:16:30, philipj_slow wrote: > > > If this is the only reason you need yo use media_test, then just > > > mediaSource.onsourceopen = test.step_func(...) would save a line and remove > > the > > > extra dependency :) > > > > Done, though "mediaSource.onsourceopen = ..." didn't work. We've used > > addEventListener/removeEventListener solely throughout media-source layout > > tests. Is "onsourceopen" attribute setting expected to work for MSE? If so, > > seems there's a bug to fix here too :) > > Ah, I just assumed that the MediaSource interface would have event handler > attributes, but it doesn't. Filed https://github.com/w3c/media-source/issues/66 > so addEventListener will do for now. Acknowledged.
	39 {

	40 URL.revokeObjectURL(mediaSourceURL);
	philipj_slow 2016/04/14 13:16:30 Maybe put this in the cleanup callback in case the Maybe put this in the cleanup callback in case the test times out or otherwise fails before this point? wolenetz 2016/04/21 21:37:30 Done. Show quoted text On 2016/04/14 13:16:30, philipj_slow wrote: > Maybe put this in the cleanup callback in case the test times out or otherwise > fails before this point? Done.
	41 test.done();

	42 });

	43 }, 'sourceopen occurs with element preload="' + preload + '"');

	44 }

	45

	46 attachWithPreloadTest();
	philipj_slow 2016/04/14 13:16:30 I don't think it's necessary to test the default b I don't think it's necessary to test the default behavior, since it's going to be one of auto/metadata/none. Also makes test slightly simpler above. wolenetz 2016/04/21 21:37:30 Done. Show quoted text On 2016/04/14 13:16:30, philipj_slow wrote: > I don't think it's necessary to test the default behavior, since it's going to > be one of auto/metadata/none. Also makes test slightly simpler above. Done.
	47 attachWithPreloadTest('auto');

	48 attachWithPreloadTest('metadata');

	49 attachWithPreloadTest('none');

	50 </script>

	51 </body>

	52 </html>

OLD	NEW

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