Convert track tests from video-test.js to testharness.js based

Convert track tests from video-test.js to testharness.js based

Cleaning up text-css-all*, track-css-cue* and track-css-matching* tests
in media/track 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/c8cf76ce31a999ccd27562c753260101075df9ce
Cr-Commit-Position: refs/heads/master@{#386660}

[Srirama, 2016-04-11 14:22:12]

Patchset #1 (id:1) has been deleted

[Srirama, 2016-04-12 06:40:17]

Patchset #1 (id:20001) has been deleted

[Srirama, 2016-04-12 06:45:42]

Description was changed from

==========
fix

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

to

==========
Convert track tests from video-test.js to testharness.js based

Cleaning up text-css-all*, track-css-cue* and track-css-matching* tests
in media/track to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

[Srirama, 2016-04-12 07:02:56]

srirama.m@samsung.com changed reviewers:
+ mlamouri@chromium.org, philipj@opera.com

[Srirama, 2016-04-12 07:02:57]

PTAL

[philipj_slow, 2016-04-12 08:39:28]

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html (right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:7: <video
controls></video>
It looks like the controls attribute isn't needed.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:12: /*
TODO(srirama.m): 'text-decoration' shorthand to be handled when available.
// comments are nicer to look at

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-cue-lifetime.html
(right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-cue-lifetime.html:36: if
(++seekTimeIndex * step + initialTime > endTime)
Can you let var seekTime = ++seekTimeIndex * step + initialTime to remove the
duplicated bits? And if the rounding isn't necesary for the test to pass, just
drop it.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/track/track-css-matching-timestamps.html
(right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching-timestamps.html:33:
video.onseeked = t.step_func(function() {
Assert something about currentTime here too.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-matching.html (right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching.html:18: <video
controls></video>
Try dropping the controls attribute here and everywhere else.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching.html:34: video.id
= "testvideo";
There's only one video element, why does it need an id? Is it about selector
specificity, so that if you don't use #testvideo in the selector above the test
results change?

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching.html:42:
video.onseeked = t.step_func(function() {
Can you assert that video.currentTime is seekTimes[seekTimeIndex]? (maybe off by
one)

[mlamouri (slow - plz ping), 2016-04-12 11:12:55]

lgtm with philipj@'s comments applied.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html (right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:12: /*
TODO(srirama.m): 'text-decoration' shorthand to be handled when available.
On 2016/04/12 at 08:39:28, philipj wrote:
> // comments are nicer to look at

Is such a thing possible in CSS?

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:39:
video.currentTime = .5;
nit: 0.5 (I know it was like that but it seems to be the only test in this CL
stripping the 0.

[Srirama, 2016-04-12 11:20:06]

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html (right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:7: <video
controls></video>
On 2016/04/12 08:39:28, philipj wrote:
> It looks like the controls attribute isn't needed.

Done.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:12: /*
TODO(srirama.m): 'text-decoration' shorthand to be handled when available.
On 2016/04/12 11:12:55, Mounir Lamouri wrote:
> On 2016/04/12 at 08:39:28, philipj wrote:
> > // comments are nicer to look at
> 
> Is such a thing possible in CSS?

CSS and HTML doesn't accept "//" comments.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:39:
video.currentTime = .5;
On 2016/04/12 11:12:55, Mounir Lamouri wrote:
> nit: 0.5 (I know it was like that but it seems to be the only test in this CL
> stripping the 0.

Done.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-cue-lifetime.html
(right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-cue-lifetime.html:36: if
(++seekTimeIndex * step + initialTime > endTime)
On 2016/04/12 08:39:28, philipj wrote:
> Can you let var seekTime = ++seekTimeIndex * step + initialTime to remove the
> duplicated bits? And if the rounding isn't necesary for the test to pass, just
> drop it.

Done.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/track/track-css-matching-timestamps.html
(right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching-timestamps.html:33:
video.onseeked = t.step_func(function() {
On 2016/04/12 08:39:28, philipj wrote:
> Assert something about currentTime here too.

Done.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-matching.html (right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching.html:18: <video
controls></video>
On 2016/04/12 08:39:28, philipj wrote:
> Try dropping the controls attribute here and everywhere else.

Done.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching.html:34: video.id
= "testvideo";
On 2016/04/12 08:39:28, philipj wrote:
> There's only one video element, why does it need an id? Is it about selector
> specificity, so that if you don't use #testvideo in the selector above the
test
> results change?

It isn't needed, i removed it in both places and it works fine. Actually this
test is about selectors on cues and they are being used in styling.vtt, so it
isn't necessary here. Id selector is being tested with #testcue.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching.html:42:
video.onseeked = t.step_func(function() {
On 2016/04/12 08:39:28, philipj wrote:
> Can you assert that video.currentTime is seekTimes[seekTimeIndex]? (maybe off
by
> one)

Done.

[philipj_slow, 2016-04-12 11:28:08]

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html (right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-all-cues.html:12: /*
TODO(srirama.m): 'text-decoration' shorthand to be handled when available.
On 2016/04/12 11:20:05, Srirama wrote:
> On 2016/04/12 11:12:55, Mounir Lamouri wrote:
> > On 2016/04/12 at 08:39:28, philipj wrote:
> > > // comments are nicer to look at
> > 
> > Is such a thing possible in CSS?
> 
> CSS and HTML doesn't accept "//" comments.

Uh, sorry, I wasn't paying attention to the context.

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-css-matching.html (right):

https://codereview.chromium.org/1875923002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-css-matching.html:34: video.id
= "testvideo";
On 2016/04/12 11:20:05, Srirama wrote:
> On 2016/04/12 08:39:28, philipj wrote:
> > There's only one video element, why does it need an id? Is it about selector
> > specificity, so that if you don't use #testvideo in the selector above the
> test
> > results change?
> 
> It isn't needed, i removed it in both places and it works fine. Actually this
> test is about selectors on cues and they are being used in styling.vtt, so it
> isn't necessary here. Id selector is being tested with #testcue.

Acknowledged.

[philipj_slow, 2016-04-12 11:29:38]

lgtm

[Srirama, 2016-04-12 11:43:11]

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

[Srirama, 2016-04-12 11:43:11]

The patchset sent to the CQ was uploaded after l-g-t-m from
mlamouri@chromium.org
Link to the patchset: https://codereview.chromium.org/1875923002/#ps60001
(title: "address comments")

[commit-bot: I haz the power, 2016-04-12 11:43:19]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1875923002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1875923002/60001

[commit-bot: I haz the power, 2016-04-12 12:51:27]

Description was changed from

==========
Convert track tests from video-test.js to testharness.js based

Cleaning up text-css-all*, track-css-cue* and track-css-matching* tests
in media/track to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Convert track tests from video-test.js to testharness.js based

Cleaning up text-css-all*, track-css-cue* and track-css-matching* tests
in media/track 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-04-12 12:51:28]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-04-12 12:53:09]

Description was changed from

==========
Convert track tests from video-test.js to testharness.js based

Cleaning up text-css-all*, track-css-cue* and track-css-matching* tests
in media/track to use testharness.js instead of video-test.js.
This will enable to upstream these tests to web-platform-tests.

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

to

==========
Convert track tests from video-test.js to testharness.js based

Cleaning up text-css-all*, track-css-cue* and track-css-matching* tests
in media/track 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/c8cf76ce31a999ccd27562c753260101075df9ce
Cr-Commit-Position: refs/heads/master@{#386660}
==========

[commit-bot: I haz the power, 2016-04-12 12:53:10]

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