Convert video-prefixed* and video-preload* tests to testharness.js

Convert video-prefixed* and video-preload* tests to testharness.js

Cleaning up video-prefixed* and video-preload* tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
Committed: https://crrev.com/6510fe630ff242a8d4337b718ed79db1c95e329a
Cr-Commit-Position: refs/heads/master@{#402898}

[Srirama, 2016-06-28 06:54:19]

Description was changed from

==========
fix

BUG=
==========

to

==========
Convert video-prefixed* and video-preload* tests to testharness.js

Cleaning up video-prefixed* and video-preload* tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

[Srirama, 2016-06-28 06:56:36]

Patchset #1 (id:1) has been deleted

[Srirama, 2016-06-28 06:56:54]

srirama.m@samsung.com changed reviewers:
+ foolip@chromium.org, fs@opera.com

[Srirama, 2016-06-28 06:56:55]

PTAL

[fs, 2016-06-28 08:56:42]

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/video-preload.html (right):

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:11: { preload : "none",
shouldBuffer : false, autoPlay : false, playInsteadOfLoad : false },
Suggest dropping the space before the ':' ("preload: ...")

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:22: var movie = null;
Could make this local where it's used. Or maybe rewrite the test to use
Array.shift. I guess using one <video> per entry could be a bit too heavy, but
otherwise that wold work too (and likely be a lot easier to manage - i.e one
test() per entry in 'movies'.)

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:47:
video.setAttribute("autoplay", movie.autoPlay);
This is not equivalent, since 'autoplay' is a boolean attribute, so its
existence is what defines whether it does something or not (and not the value.)

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:48: video.src =
findMediaFile("video", "content/test");;
Nit: ;;

[Srirama, 2016-06-29 13:27:07]

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/video-preload.html (right):

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:22: var movie = null;
On 2016/06/28 08:56:42, fs wrote:
> Could make this local where it's used. Or maybe rewrite the test to use
> Array.shift. I guess using one <video> per entry could be a bit too heavy, but
> otherwise that wold work too (and likely be a lot easier to manage - i.e one
> test() per entry in 'movies'.)

You mean write a function like "check_video_preload(movie_info)" which does
async_test inside with "movie_info" and call this function 6 times(once per
entry from movies array)?

[fs, 2016-06-29 13:35:18]

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/video-preload.html (right):

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:22: var movie = null;
On 2016/06/29 at 13:27:07, Srirama wrote:
> On 2016/06/28 08:56:42, fs wrote:
> > Could make this local where it's used. Or maybe rewrite the test to use
> > Array.shift. I guess using one <video> per entry could be a bit too heavy,
but
> > otherwise that wold work too (and likely be a lot easier to manage - i.e one
> > test() per entry in 'movies'.)
> 
> You mean write a function like "check_video_preload(movie_info)" which does
async_test inside with "movie_info" and call this function 6 times(once per
entry from movies array)?

Yes, that would be one way to do it. Could also just keep the array and iterate
it directly. Like:

for (let testInfo of movies)
  async_test(...using testInfo...);

[Srirama, 2016-06-29 17:23:31]

Patchset #2 (id:40001) has been deleted

[Srirama, 2016-06-29 17:27:46]

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/video-preload.html (right):

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:11: { preload : "none",
shouldBuffer : false, autoPlay : false, playInsteadOfLoad : false },
On 2016/06/28 08:56:42, fs wrote:
> Suggest dropping the space before the ':' ("preload: ...")

Done.

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:22: var movie = null;
On 2016/06/29 13:35:18, fs wrote:
> On 2016/06/29 at 13:27:07, Srirama wrote:
> > On 2016/06/28 08:56:42, fs wrote:
> > > Could make this local where it's used. Or maybe rewrite the test to use
> > > Array.shift. I guess using one <video> per entry could be a bit too heavy,
> but
> > > otherwise that wold work too (and likely be a lot easier to manage - i.e
one
> > > test() per entry in 'movies'.)
> > 
> > You mean write a function like "check_video_preload(movie_info)" which does
> async_test inside with "movie_info" and call this function 6 times(once per
> entry from movies array)?
> 
> Yes, that would be one way to do it. Could also just keep the array and
iterate
> it directly. Like:
> 
> for (let testInfo of movies)
>   async_test(...using testInfo...);

Done.

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:47:
video.setAttribute("autoplay", movie.autoPlay);
On 2016/06/28 08:56:42, fs wrote:
> This is not equivalent, since 'autoplay' is a boolean attribute, so its
> existence is what defines whether it does something or not (and not the
value.)

Done.

https://codereview.chromium.org/2104823002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/video-preload.html:48: video.src =
findMediaFile("video", "content/test");;
On 2016/06/28 08:56:42, fs wrote:
> Nit: ;;

Done.

[fs, 2016-06-29 17:52:35]

lgtm

[Srirama, 2016-06-29 18:07:28]

The CQ bit was checked by srirama.m@samsung.com

[commit-bot: I haz the power, 2016-06-29 18:10:31]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-06-29 19:31:20]

Description was changed from

==========
Convert video-prefixed* and video-preload* tests to testharness.js

Cleaning up video-prefixed* and video-preload* tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

to

==========
Convert video-prefixed* and video-preload* tests to testharness.js

Cleaning up video-prefixed* and video-preload* tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

[commit-bot: I haz the power, 2016-06-29 19:31:21]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-06-29 19:35:18]

Description was changed from

==========
Convert video-prefixed* and video-preload* tests to testharness.js

Cleaning up video-prefixed* and video-preload* tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956
==========

to

==========
Convert video-prefixed* and video-preload* tests to testharness.js

Cleaning up video-prefixed* and video-preload* tests
in media/ to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

BUG=588956

Committed: https://crrev.com/6510fe630ff242a8d4337b718ed79db1c95e329a
Cr-Commit-Position: refs/heads/master@{#402898}
==========

[commit-bot: I haz the power, 2016-06-29 19:35:20]

Patchset 2 (id:??) landed as
https://crrev.com/6510fe630ff242a8d4337b718ed79db1c95e329a
Cr-Commit-Position: refs/heads/master@{#402898}