Adding HTML5 track(caption) test case in PyAuto media functional tests.

Adding HTML5 track(caption test) case in PyAuto media functional tests.
The track specification can be found in 
http://www.w3.org/TR/html5/video.html#the-track-element. This is the first
version of the tests so there is no verification yet. -g option is used in the media_test_runner to specify the track(caption) file location. If it is not set, default track file is used (the same location as video and file name is 'title_en.vtt'.


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=88940

[imasaki1, 2011-06-11 02:06:09]

Thanks!

[annacc, 2011-06-13 18:51:36]

Thanks Kenji!

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
File chrome/test/data/media/html/media_track.html (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:2: This HTML file contains player a
div which is used for track testing
"contains a div for a player..." ?

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:10: track (required): the name of
the track file (inband).
should say 'out-of-band'

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:39: player.play();
If you are only testing the attributes of <track> and the contents of the
out-of-band file, you may not need to actually play the video.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/pla...
File chrome/test/data/media/html/player.js (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/pla...
chrome/test/data/media/html/player.js:86: 
extra lines?

[Nirnimesh, 2011-06-13 22:07:37]

I have only minor comments. LGTM

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
File chrome/test/data/media/html/media_track.html (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:32: ta.value +=
tr.track.cues[i].startTime + " --> " + tr.track.cues[i].endTime + "\n";
nit: 80+ lines

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
File chrome/test/functional/media/media_test_runner.py (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:143: # Otherwise, it exits.
remove this line

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:144: if len(opts) != 4:
print out some helpful error message

[dennis_jeffrey, 2011-06-13 22:51:49]

LGTM

Just a set of minor nits to address before submitting. Thanks!

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
File chrome/test/data/media/html/media_track.html (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:9: t (optional): add the "t"
parameter to disable the media cache.
"add the "t" parameter to disable the media cache." -->
"if specified, disables the media cache."

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:30: for (i = 0; i <
tr.track.cues.length; i++) {
nit: "var i = 0;"

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:44: 
nit: remove these two blank lines.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:46: 
nit: remove this blank line.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/pla...
File chrome/test/data/media/html/player.js (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/pla...
chrome/test/data/media/html/player.js:101: track_file = queryString('track')
nit: "var track_file".  Also put semi-colon at end of stmt.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
File chrome/test/functional/media/media_test_runner.py (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:112: 'from data/html/
directory.'
Nit: put a space right before the closing quote.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:113: 'The data should have the
following format:'
Nit: put a space right before the closing quote.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:141: # media file should have
the following format:
nit: "media file" --> "The media file"

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:195: # The default name for
track vtt file is the name of the video tile
"tile" -->
"title"

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:198: # For example,
bear_en.vtt used
Remove "used"?

[imasaki1, 2011-06-13 23:43:09]

All issues are addressed. I am going to commit... Thanks.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
File chrome/test/data/media/html/media_track.html (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:2: This HTML file contains player a
div which is used for track testing
On 2011/06/13 18:51:37, annacc wrote:
> "contains a div for a player..." ?

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:9: t (optional): add the "t"
parameter to disable the media cache.
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> "add the "t" parameter to disable the media cache." -->
> "if specified, disables the media cache."

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:10: track (required): the name of
the track file (inband).
On 2011/06/13 18:51:37, annacc wrote:
> should say 'out-of-band'

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:30: for (i = 0; i <
tr.track.cues.length; i++) {
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> nit: "var i = 0;"

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:32: ta.value +=
tr.track.cues[i].startTime + " --> " + tr.track.cues[i].endTime + "\n";
On 2011/06/13 22:07:37, Nirnimesh wrote:
> nit: 80+ lines

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:39: player.play();
On 2011/06/13 18:51:37, annacc wrote:
> If you are only testing the attributes of <track> and the contents of the
> out-of-band file, you may not need to actually play the video.

I want to play the video since it is used for performance test.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:44: 
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> nit: remove these two blank lines.

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/med...
chrome/test/data/media/html/media_track.html:46: 
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> nit: remove this blank line.

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/pla...
File chrome/test/data/media/html/player.js (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/pla...
chrome/test/data/media/html/player.js:86: 
On 2011/06/13 18:51:37, annacc wrote:
> extra lines?

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/data/media/html/pla...
chrome/test/data/media/html/player.js:101: track_file = queryString('track')
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> nit: "var track_file".  Also put semi-colon at end of stmt.

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
File chrome/test/functional/media/media_test_runner.py (right):

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:112: 'from data/html/
directory.'
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> Nit: put a space right before the closing quote.

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:113: 'The data should have the
following format:'
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> Nit: put a space right before the closing quote.

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:141: # media file should have
the following format:
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> nit: "media file" --> "The media file"

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:143: # Otherwise, it exits.
On 2011/06/13 22:07:37, Nirnimesh wrote:
> remove this line

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:144: if len(opts) != 4:
On 2011/06/13 22:07:37, Nirnimesh wrote:
> print out some helpful error message

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:195: # The default name for
track vtt file is the name of the video tile
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> "tile" -->
> "title"

Done.

http://codereview.chromium.org/7003131/diff/9/chrome/test/functional/media/me...
chrome/test/functional/media/media_test_runner.py:198: # For example,
bear_en.vtt used
On 2011/06/13 22:51:49, dennis_jeffrey wrote:
> Remove "used"?

Done.

[commit-bot: I haz the power, 2011-06-13 23:44:26]

Presubmit check for 7003131-6003 failed and returned exit status 1.

Running presubmit commit checks ...

** Presubmit Messages **
If this change has an associated bug, add BUG=[bug number].

** Presubmit Warnings **
License must match:
.*? Copyright \(c\) 2011 The Chromium Authors\. All rights reserved\.\n.*? Use
of this source code is governed by a BSD-style license that can be\n.*? found in
the LICENSE file\.\n
Found a bad license header in these files:
  chrome/test/data/media/html/player.js

[commit-bot: I haz the power, 2011-06-14 01:20:12]

Change committed as 88940

[M-A Ruel, 2011-06-15 12:50:04]

On 2011/06/13 23:44:26, I haz the power (commit-bot) wrote:
> Presubmit check for 7003131-6003 failed and returned exit status 1.
> 
> Running presubmit commit checks ...
> 
> ** Presubmit Messages **
> If this change has an associated bug, add BUG=[bug number].
> 
> ** Presubmit Warnings **
> License must match:
> .*? Copyright \(c\) 2011 The Chromium Authors\. All rights reserved\.\n.*? Use
> of this source code is governed by a BSD-style license that can be\n.*? found
in
> the LICENSE file\.\n
> Found a bad license header in these files:
>   chrome/test/data/media/html/player.js

For the record, the auto-copyright fixer will update the date but for obvious
reasons it won't add a copyright to a file that doesn't have one already.