Convert track-cues* and track-word* tests to testharness.js

Convert track-cues* and track-word* tests to testharness.js

Cleaning up track-cues* and track-word* 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/36a611b6e4e30fd8df8e8118df79a851a0bdb510
Cr-Commit-Position: refs/heads/master@{#399697}

[Srirama, 2016-06-13 12:13:14]

Description was changed from

==========
fix

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

to

==========
Convert track-cues* and track-word* tests to testharness.js

Cleaning up track-cues* and track-word* 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-06-13 12:15:18]

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

[Srirama, 2016-06-13 12:15:18]

PTAL

[fs, 2016-06-13 15:20:18]

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html
(right):

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:20:
track.onload = t.step_func(runTests);
See comment on the later test about sequencing.

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:40:
track.track.cues[currentCueNumber].onexit = t.step_func(function(event) {
Maybe these could be installed right after the track is loaded (to avoid needing
the currentCueNumber thingamajib.)

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-cues-seeking.html (right):

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-cues-seeking.html:30: var
eventCount = 0;
Maybe it would be possible to sequence this as:

track.onload = t.step_func(function(t) {
  assert_equals(track.track.cues.length, 3);
  video.src = ...;
  video.oncanplaythrough = t.step_func(function() {
    video.currentTime = ...;
  });
});

or thereabouts? (Slightly slower because of the lack of "parallelism", but
hopefully not too bad.)

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-word-breaking.html
(right):

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-word-breaking.html:23:
singleLineRect =
textTrackDisplayElement(video).firstChild.firstChild.getBoundingClientRect();
Maybe move the right-hand side (+ .height) here into a helper function. (And
"Rect" -> "Height")

[Srirama, 2016-06-13 17:33:38]

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html
(right):

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:20:
track.onload = t.step_func(runTests);
On 2016/06/13 15:20:18, fs wrote:
> See comment on the later test about sequencing.

Done. Removed 'canplaythrough' event, please see my next comments.

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-cues-seeking.html (right):

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-cues-seeking.html:30: var
eventCount = 0;
On 2016/06/13 15:20:18, fs wrote:
> Maybe it would be possible to sequence this as:
> 
> track.onload = t.step_func(function(t) {
>   assert_equals(track.track.cues.length, 3);
>   video.src = ...;
>   video.oncanplaythrough = t.step_func(function() {
>     video.currentTime = ...;
>   });
> });
> 
> or thereabouts? (Slightly slower because of the lack of "parallelism", but
> hopefully not too bad.)

Done. I think the intention of waiting for 'canplaythrough' event is to set the
current time. But that is the case with old code and we no longer need to wait
for that event now to set current time. So i removed it.

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-word-breaking.html
(right):

https://codereview.chromium.org/2061663002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-word-breaking.html:23:
singleLineRect =
textTrackDisplayElement(video).firstChild.firstChild.getBoundingClientRect();
On 2016/06/13 15:20:18, fs wrote:
> Maybe move the right-hand side (+ .height) here into a helper function. (And
> "Rect" -> "Height")

Done.

[fs, 2016-06-13 17:57:20]

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html
(right):

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:13:
track.onload = t.step_func(function() {
Isn't there a risk that this will now race with the playback of the video?

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:24: if
(event.target.id == 2 + "")
Nit: Don't think the + "" is needed here (the "==" means operands will be
converted.)

[Srirama, 2016-06-14 09:33:53]

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html
(right):

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:13:
track.onload = t.step_func(function() {
On 2016/06/13 17:57:19, fs wrote:
> Isn't there a risk that this will now race with the playback of the video?

Isn't it same as before? We set currenttime and then we call runTests() which
may immediately call video.play(). I ran both the test locally 1000 times and it
is passing. So should we go ahead? or shall we take opinion from philip?

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:24: if
(event.target.id == 2 + "")
On 2016/06/13 17:57:19, fs wrote:
> Nit: Don't think the + "" is needed here (the "==" means operands will be
> converted.)

Done.

[fs, 2016-06-14 09:41:49]

lgtm

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html
(right):

https://codereview.chromium.org/2061663002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit.html:13:
track.onload = t.step_func(function() {
On 2016/06/14 at 09:33:53, Srirama wrote:
> On 2016/06/13 17:57:19, fs wrote:
> > Isn't there a risk that this will now race with the playback of the video?
> 
> Isn't it same as before? We set currenttime and then we call runTests() which
may immediately call video.play(). I ran both the test locally 1000 times and it
is passing. So should we go ahead? or shall we take opinion from philip?

Sorry, I must've misread the indentation level at the bottom (where play() is
acalled.) This is ok.

[Srirama, 2016-06-14 10:00:03]

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

[commit-bot: I haz the power, 2016-06-14 10:00:14]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2061663002/40001

[commit-bot: I haz the power, 2016-06-14 13:19:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-06-14 13:19:45]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[Srirama, 2016-06-14 13:20:53]

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

[commit-bot: I haz the power, 2016-06-14 13:21:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2061663002/40001

[commit-bot: I haz the power, 2016-06-14 14:09:42]

Description was changed from

==========
Convert track-cues* and track-word* tests to testharness.js

Cleaning up track-cues* and track-word* 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-cues* and track-word* tests to testharness.js

Cleaning up track-cues* and track-word* 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-06-14 14:09:43]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-06-14 14:11:45]

Description was changed from

==========
Convert track-cues* and track-word* tests to testharness.js

Cleaning up track-cues* and track-word* 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-cues* and track-word* tests to testharness.js

Cleaning up track-cues* and track-word* 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/36a611b6e4e30fd8df8e8118df79a851a0bdb510
Cr-Commit-Position: refs/heads/master@{#399697}
==========

[commit-bot: I haz the power, 2016-06-14 14:11:46]

Patchset 3 (id:??) landed as
https://crrev.com/36a611b6e4e30fd8df8e8118df79a851a0bdb510
Cr-Commit-Position: refs/heads/master@{#399697}