Convert track-webvtt-tc[019-021] tests to testharness.js

Convert track-webvtt-tc[019-021] tests to testharness.js

Cleaning up track-webvtt-tc[019-021] 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/97ae3bc8dd59a5e1a6243460b94ed12dbe66e4f7
Cr-Commit-Position: refs/heads/master@{#397389}

[Srirama, 2016-06-01 16:44:37]

Description was changed from

==========
fix


fix

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

to

==========
Convert track-webvtt-tc[019-021] tests to testharness.js

Cleaning up track-webvtt-tc[019-021] 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-01 16:47:22]

Patchset #1 (id:1) has been deleted

[Srirama, 2016-06-01 16:53:23]

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

[Srirama, 2016-06-01 16:53:24]

PTAL.
@fs, hope I have followed your suggestion from the previous review
(https://codereview.chromium.org/2012833002/#msg7) properly.

[fs, 2016-06-01 17:02:46]

On 2016/06/01 at 16:53:24, srirama.m wrote:
> PTAL.
> @fs, hope I have followed your suggestion from the previous review
(https://codereview.chromium.org/2012833002/#msg7) properly.

Yes, this looks nice! Bonus points will be awarded if named functions are turned
into anonymous ones when they only have a single reference (like in the two
first tests.)

LGTM!

[Srirama, 2016-06-01 17:23:09]

On 2016/06/01 17:02:46, fs wrote:
> On 2016/06/01 at 16:53:24, srirama.m wrote:
> > PTAL.
> > @fs, hope I have followed your suggestion from the previous review
> (https://codereview.chromium.org/2012833002/#msg7) properly.
> 
> Yes, this looks nice! Bonus points will be awarded if named functions are
turned
> into anonymous ones when they only have a single reference (like in the two
> first tests.)
> 
> LGTM!

Thanks, done :)

[foolip, 2016-06-02 09:14:19]

lgtm

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-helpers.js (right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-helpers.js:24: for (var
property of properties)
Is the properties array really needed? Can't this use
Object.getOwnPropertyNames(expected) instead? At the call sites it looks a bit
redundant.

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-webvtt-tc019-cue-size.html
(right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc019-cue-size.html:11:
{ size:    0}
Weird indentation, I'd just go with one space everywhere.

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc020-cue-size-align.html
(right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc020-cue-size-align.html:11:
{ size:   0, align: "middle" }
Ditto I guess, doesn't matter much.

[fs, 2016-06-02 09:32:46]

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-helpers.js (right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-helpers.js:24: for (var
property of properties)
On 2016/06/02 at 09:14:18, Philip Jägenstedt wrote:
> Is the properties array really needed? Can't this use
Object.getOwnPropertyNames(expected) instead? At the call sites it looks a bit
redundant.

Yes, that ought to work (with s/expected/expectedItem/ presumably.)

[Srirama, 2016-06-02 09:59:28]

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-helpers.js (right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-helpers.js:24: for (var
property of properties)
On 2016/06/02 09:32:46, fs wrote:
> On 2016/06/02 at 09:14:18, Philip Jägenstedt wrote:
> > Is the properties array really needed? Can't this use
> Object.getOwnPropertyNames(expected) instead? At the call sites it looks a bit
> redundant.
> 
> Yes, that ought to work (with s/expected/expectedItem/ presumably.)

Done.

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/media/track/track-webvtt-tc019-cue-size.html
(right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc019-cue-size.html:11:
{ size:    0}
On 2016/06/02 09:14:19, Philip Jägenstedt wrote:
> Weird indentation, I'd just go with one space everywhere.

Done.

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc020-cue-size-align.html
(right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc020-cue-size-align.html:11:
{ size:   0, align: "middle" }
On 2016/06/02 09:14:19, Philip Jägenstedt wrote:
> Ditto I guess, doesn't matter much.

Done. Is this fine(for better alignment of properties) or just go ahead and
remove extra spaces everywhere?

[foolip, 2016-06-02 11:04:59]

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc020-cue-size-align.html
(right):

https://codereview.chromium.org/2029663002/diff/40001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/media/track/track-webvtt-tc020-cue-size-align.html:11:
{ size:   0, align: "middle" }
On 2016/06/02 09:59:27, Srirama wrote:
> On 2016/06/02 09:14:19, Philip Jägenstedt wrote:
> > Ditto I guess, doesn't matter much.
> 
> Done. Is this fine(for better alignment of properties) or just go ahead and
> remove extra spaces everywhere?

I'd just remove all the extra spaces everywhere, but it's not important, this is
OK.

[Srirama, 2016-06-02 11:45:26]

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

[Srirama, 2016-06-02 11:45:27]

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

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

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

[commit-bot: I haz the power, 2016-06-02 13:59:49]

Description was changed from

==========
Convert track-webvtt-tc[019-021] tests to testharness.js

Cleaning up track-webvtt-tc[019-021] 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-webvtt-tc[019-021] tests to testharness.js

Cleaning up track-webvtt-tc[019-021] 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-02 13:59:50]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2016-06-02 14:01:57]

Description was changed from

==========
Convert track-webvtt-tc[019-021] tests to testharness.js

Cleaning up track-webvtt-tc[019-021] 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-webvtt-tc[019-021] tests to testharness.js

Cleaning up track-webvtt-tc[019-021] 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/97ae3bc8dd59a5e1a6243460b94ed12dbe66e4f7
Cr-Commit-Position: refs/heads/master@{#397389}
==========

[commit-bot: I haz the power, 2016-06-02 14:01:58]

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