Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(2)

Issue 1854553003: Convert webvtt regions tests from video-test.js to testharness.js based (Closed)

Created:
4 years, 8 months ago by Srirama
Modified:
4 years, 8 months ago
CC:
fs, blink-reviews, chromium-reviews, eric.carlson_apple.com, feature-media-reviews_chromium.org, gasubic, mlamouri+watch-blink_chromium.org, philipj_slow, nessy, vcarbune.chromium
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Convert webvtt regions tests from video-test.js to testharness.js based Cleaning up tests in media/track/regions-webvtt 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/5e5ca9d011d519d3ee29f52a1d2ebeeeff57f685 Cr-Commit-Position: refs/heads/master@{#385722}

Patch Set 1 #

Total comments: 16

Patch Set 2 : address review comments #

Total comments: 28

Patch Set 3 : address review comments #

Total comments: 2

Patch Set 4 : Address more comments #

Unified diffs Side-by-side diffs Delta from patch set Stats (+292 lines, -749 lines) Patch
M third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html View 1 2 1 chunk +28 lines, -49 lines 0 comments Download
D third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute-expected.txt View 1 chunk +0 lines, -17 lines 0 comments Download
M third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor.html View 1 2 1 chunk +59 lines, -75 lines 0 comments Download
D third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor-expected.txt View 1 chunk +0 lines, -122 lines 0 comments Download
M third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html View 1 2 3 1 chunk +69 lines, -111 lines 0 comments Download
D third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display-expected.txt View 1 chunk +0 lines, -51 lines 0 comments Download
M third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-dom-layout.html View 1 2 3 1 chunk +29 lines, -65 lines 0 comments Download
D third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-dom-layout-expected.txt View 1 chunk +0 lines, -19 lines 0 comments Download
M third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-list.html View 1 2 1 chunk +61 lines, -101 lines 0 comments Download
D third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-list-expected.txt View 1 chunk +0 lines, -44 lines 0 comments Download
M third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-parser.html View 1 2 1 chunk +46 lines, -67 lines 0 comments Download
D third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-parser-expected.txt View 1 chunk +0 lines, -28 lines 0 comments Download

Messages

Total messages: 24 (9 generated)
Srirama
PTAL. There are two issues in two tests please let me know how can we ...
4 years, 8 months ago (2016-04-01 09:58:18 UTC) #4
Srirama
4 years, 8 months ago (2016-04-01 10:07:54 UTC) #6
fs
https://codereview.chromium.org/1854553003/diff/1/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor.html (right): https://codereview.chromium.org/1854553003/diff/1/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor.html#newcode26 third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-constructor.html:26: assert_throws(new IndexSizeError, function() { region.viewportAnchorX = invalidPercentageValues[index]; }); On ...
4 years, 8 months ago (2016-04-01 10:39:46 UTC) #8
fs
4 years, 8 months ago (2016-04-01 10:40:31 UTC) #10
philipj_slow
Took a peek at the first two tests only. https://codereview.chromium.org/1854553003/diff/1/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html (right): https://codereview.chromium.org/1854553003/diff/1/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html#newcode6 third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html:6: ...
4 years, 8 months ago (2016-04-01 11:07:50 UTC) #11
Srirama
https://codereview.chromium.org/1854553003/diff/1/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html (right): https://codereview.chromium.org/1854553003/diff/1/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html#newcode6 third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html:6: async_test(function (t) { On 2016/04/01 11:07:49, philipj_UTC7 wrote: > ...
4 years, 8 months ago (2016-04-01 12:14:23 UTC) #12
philipj_slow
https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html (right): https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html#newcode42 third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html:42: testTrack.onload = this.step_func(function() { On 2016/04/01 12:14:23, Srirama wrote: ...
4 years, 8 months ago (2016-04-06 12:19:40 UTC) #13
philipj_slow
LGTM % nits https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html (right): https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html#newcode7 third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html:7: var cue = new VTTCue(0.0, 1.0, ...
4 years, 8 months ago (2016-04-06 12:40:04 UTC) #14
Srirama
https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html (right): https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html#newcode7 third_party/WebKit/LayoutTests/media/track/regions-webvtt/text-track-cue-region-attribute.html:7: var cue = new VTTCue(0.0, 1.0, "sample"); On 2016/04/06 ...
4 years, 8 months ago (2016-04-07 10:40:07 UTC) #15
philipj_slow
https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html (right): https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html#newcode42 third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html:42: testTrack.onload = this.step_func(function() { On 2016/04/07 10:40:07, Srirama wrote: ...
4 years, 8 months ago (2016-04-07 11:07:39 UTC) #16
Srirama
https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html File third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html (right): https://codereview.chromium.org/1854553003/diff/20001/third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html#newcode42 third_party/WebKit/LayoutTests/media/track/regions-webvtt/vtt-region-display.html:42: testTrack.onload = this.step_func(function() { On 2016/04/07 11:07:39, philipj wrote: ...
4 years, 8 months ago (2016-04-07 11:15:52 UTC) #17
philipj_slow
lgtm
4 years, 8 months ago (2016-04-07 11:18:43 UTC) #18
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1854553003/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1854553003/60001
4 years, 8 months ago (2016-04-07 11:24:46 UTC) #20
commit-bot: I haz the power
Committed patchset #4 (id:60001)
4 years, 8 months ago (2016-04-07 12:22:44 UTC) #22
commit-bot: I haz the power
4 years, 8 months ago (2016-04-07 12:24:08 UTC) #24
Message was sent while issue was closed.
Patchset 4 (id:??) landed as
https://crrev.com/5e5ca9d011d519d3ee29f52a1d2ebeeeff57f685
Cr-Commit-Position: refs/heads/master@{#385722}

Powered by Google App Engine
This is Rietveld 408576698