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-track*, track-active* 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/88c6b18fb999f84fef076e8789f0845b3e81c7b0
Cr-Commit-Position: refs/heads/master@{#386650}

[Srirama, 2016-04-08 11:45:24]

Description was changed from

==========
fix

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

to

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

Cleaning up some of the 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-08 11:47:17]

Description was changed from

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

Cleaning up some of the 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 some of the 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-08 11:58:35]

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

[Srirama, 2016-04-08 11:58:36]

PTAL

https://codereview.chromium.org/1867303002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-active-cues.html (left):

https://codereview.chromium.org/1867303002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:40:
waitForEventsAndCall([[video, 'canplaythrough'], [trackElement, 'load'],
[trackElement, 'cuechange']], clearSrc);
I removed all these events and just added 'oncuechange' and ran 1000 times and
it works fine. So is it ok or do i need to check all these 3 events?

[Srirama, 2016-04-08 13:43:00]

https://codereview.chromium.org/1867303002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-active-cues.html (left):

https://codereview.chromium.org/1867303002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:40:
waitForEventsAndCall([[video, 'canplaythrough'], [trackElement, 'load'],
[trackElement, 'cuechange']], clearSrc);
On 2016/04/08 11:58:36, Srirama wrote:
> I removed all these events and just added 'oncuechange' and ran 1000 times and
> it works fine. So is it ok or do i need to check all these 3 events?

Sorry, looks like the test is failing with latest code, i am working at home and
my codebase is 4months old. I will update and check with latest code.

[Srirama, 2016-04-09 09:07:33]

https://codereview.chromium.org/1867303002/diff/1/third_party/WebKit/LayoutTe...
File third_party/WebKit/LayoutTests/media/track/track-active-cues.html (left):

https://codereview.chromium.org/1867303002/diff/1/third_party/WebKit/LayoutTe...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:40:
waitForEventsAndCall([[video, 'canplaythrough'], [trackElement, 'load'],
[trackElement, 'cuechange']], clearSrc);
On 2016/04/08 13:43:00, Srirama wrote:
> On 2016/04/08 11:58:36, Srirama wrote:
> > I removed all these events and just added 'oncuechange' and ran 1000 times
and
> > it works fine. So is it ok or do i need to check all these 3 events?
> 
> Sorry, looks like the test is failing with latest code, i am working at home
and
> my codebase is 4months old. I will update and check with latest code.

Fixed test failure.

[Srirama, 2016-04-09 13:08:39]

Patchset #2 (id:20001) has been deleted

[Srirama, 2016-04-11 11:04:42]

Description was changed from

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

Cleaning up some of the 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-track*, track-active* 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
==========

[philipj_slow, 2016-04-11 11:50:03]

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

https://codereview.chromium.org/1867303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:7: var
eventCount = 0;
Can you put this the helper inside the async_test instead? It was presumably
external only because testExpected runs eval.

https://codereview.chromium.org/1867303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:11: function
clearSrc() {
clearSrc doesn't quite work as a name when you've made the event counting
internal. Maybe just callback or something similarly generic? I can't think of a
good name at least.

https://codereview.chromium.org/1867303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:24:
trackElement.onload = t.step_func(function() {
Just t.step_func(clearSrc) should work.

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

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

https://codereview.chromium.org/1867303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:7: var
eventCount = 0;
On 2016/04/11 11:50:03, philipj wrote:
> Can you put this the helper inside the async_test instead? It was presumably
> external only because testExpected runs eval.

Done.

https://codereview.chromium.org/1867303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:11: function
clearSrc() {
On 2016/04/11 11:50:02, philipj wrote:
> clearSrc doesn't quite work as a name when you've made the event counting
> internal. Maybe just callback or something similarly generic? I can't think of
a
> good name at least.

Done.

https://codereview.chromium.org/1867303002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-active-cues.html:24:
trackElement.onload = t.step_func(function() {
On 2016/04/11 11:50:02, philipj wrote:
> Just t.step_func(clearSrc) should work.

Done.

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

lgtm. I'm just concerned that the current GC simulation (when running outside of
a layout test) might be a no-op. Though, it's good that the test would run
outside of Chromium's layout test :)

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html
(right):

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html:12:
var str = new String('1234');
Does that actually work? IOW, will GC run without any sort of timeout?

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/text-track-is-reachable.html
(right):

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/text-track-is-reachable.html:12: var
str = new String('1234');
ditto

[philipj_slow, 2016-04-11 14:39:22]

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html
(right):

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html:12:
var str = new String('1234');
On 2016/04/11 13:12:32, Mounir Lamouri wrote:
> Does that actually work? IOW, will GC run without any sort of timeout?

The comment asyncGC method in js-test.js talks about this. A helper like that
that isn't tied to js-test.js would be nice.

[Srirama, 2016-04-12 05:52:53]

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html
(right):

https://codereview.chromium.org/1867303002/diff/60001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html:12:
var str = new String('1234');
On 2016/04/11 14:39:22, philipj wrote:
> On 2016/04/11 13:12:32, Mounir Lamouri wrote:
> > Does that actually work? IOW, will GC run without any sort of timeout?
> 
> The comment asyncGC method in js-test.js talks about this. A helper like that
> that isn't tied to js-test.js would be nice.

Added async GC functionality (forceAsyncGC). Is this fine? If it is, do you want
me to move it to testharness.js file?

[philipj_slow, 2016-04-12 08:43:34]

https://codereview.chromium.org/1867303002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html
(right):

https://codereview.chromium.org/1867303002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html:23:
function forceAsyncGC() {
I'd say just include resources/gc.js, and do this:

gc();
setTimeout(t.step_func_done(function() {
  assert_equals(video.textTracks[0].cues.length, 4);
  assert_equals(video.textTracks[0].cues[1].myProperty, 'tuna salad?');
}, 0);

[Srirama, 2016-04-12 09:54:57]

https://codereview.chromium.org/1867303002/diff/80001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html
(right):

https://codereview.chromium.org/1867303002/diff/80001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/text-track-cue-is-reachable.html:23:
function forceAsyncGC() {
On 2016/04/12 08:43:34, philipj wrote:
> I'd say just include resources/gc.js, and do this:
> 
> gc();
> setTimeout(t.step_func_done(function() {
>   assert_equals(video.textTracks[0].cues.length, 4);
>   assert_equals(video.textTracks[0].cues[1].myProperty, 'tuna salad?');
> }, 0);

Done. Thanks, looks much simpler and cleaner.

[philipj_slow, 2016-04-12 09:56:23]

lgtm

[Srirama, 2016-04-12 10:09:52]

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

[Srirama, 2016-04-12 10:09:53]

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/1867303002/#ps100001
(title: "Simplify GC invocation as per comments")

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

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

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

Description was changed from

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

Cleaning up text-track*, track-active* 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-track*, track-active* 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 11:11:06]

Committed patchset #5 (id:100001)

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

Description was changed from

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

Cleaning up text-track*, track-active* 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-track*, track-active* 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/88c6b18fb999f84fef076e8789f0845b3e81c7b0
Cr-Commit-Position: refs/heads/master@{#386650}
==========

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

Patchset 5 (id:??) landed as
https://crrev.com/88c6b18fb999f84fef076e8789f0845b3e81c7b0
Cr-Commit-Position: refs/heads/master@{#386650}