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

Issue 17395006: Fix LayoutTests that assume canplay, playing, and canplaythrough will only fire once. (Closed)

Created:
7 years, 6 months ago by acolwell GONE FROM CHROMIUM
Modified:
7 years, 6 months ago
CC:
blink-reviews, eae+blinkwatch, vcarbune.chromium
Visibility:
Public.

Description

Fix LayoutTests that assume canplay, playing, and canplaythrough will only fire once. Several tests are written under the assumption that the canplay, playing, and canplaythrough events will only ever fire once. While this is true for the current implementation, the HTML5 spec allows these events to fire multiple times during the life of an HTMLMediaElement. This change just modifies the tests so that they properly reflect their intent, which is to run some code the first time these events fire. This frees us to change how we do readyState transitions, which cause extra events to fire, without breaking these tests. BUG=144683 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=152683

Patch Set 1 #

Total comments: 3

Patch Set 2 : Fix event-attributes expectations. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+95 lines, -83 lines) Patch
M LayoutTests/http/tests/media/media-source/webkitmediasource-state-changes.html View 1 chunk +0 lines, -1 line 0 comments Download
M LayoutTests/http/tests/media/media-source/webkitmediasource-state-changes-expected.txt View 2 chunks +0 lines, -2 lines 0 comments Download
M LayoutTests/media/audio-garbage-collect.html View 1 chunk +3 lines, -2 lines 0 comments Download
M LayoutTests/media/controls-drag-timebar.html View 2 chunks +1 line, -2 lines 0 comments Download
M LayoutTests/media/event-attributes.html View 1 chunk +3 lines, -1 line 0 comments Download
M LayoutTests/media/media-controls-invalid-url.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/media-element-play-after-eos.html View 3 chunks +7 lines, -5 lines 0 comments Download
M LayoutTests/media/media-fragments/media-fragments.js View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-css-cue-lifetime.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-css-matching.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-css-matching-timestamps.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-cue-nothing-to-render.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-cue-rendering.html View 2 chunks +3 lines, -3 lines 0 comments Download
M LayoutTests/media/track/track-cue-rendering-rtl.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-cue-rendering-snap-to-lines-not-set.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-cues-missed.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-cues-pause-on-exit.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-cues-seeking.html View 2 chunks +8 lines, -8 lines 0 comments Download
M LayoutTests/media/track/track-cues-sorted-before-dispatch.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/track/track-remove-by-setting-innerHTML.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/video-currentTime-delay.html View 1 chunk +9 lines, -9 lines 0 comments Download
M LayoutTests/media/video-currentTime-set.html View 1 chunk +7 lines, -7 lines 0 comments Download
M LayoutTests/media/video-duration-known-after-eos.html View 2 chunks +1 line, -2 lines 0 comments Download
M LayoutTests/media/video-frame-size-change.html View 2 chunks +5 lines, -0 lines 0 comments Download
M LayoutTests/media/video-playbackrate.html View 2 chunks +2 lines, -2 lines 0 comments Download
M LayoutTests/media/video-played.js View 2 chunks +2 lines, -2 lines 0 comments Download
M LayoutTests/media/video-played-reset.html View 4 chunks +10 lines, -8 lines 0 comments Download
M LayoutTests/media/video-seek-by-small-increment.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/video-seek-past-end-playing.html View 1 chunk +1 line, -1 line 0 comments Download
M LayoutTests/media/video-seeking.html View 1 chunk +14 lines, -7 lines 0 comments Download
M LayoutTests/media/video-seeking-expected.txt View 1 chunk +0 lines, -3 lines 0 comments Download
M LayoutTests/media/video-test.js View 2 chunks +6 lines, -5 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
acolwell GONE FROM CHROMIUM
https://codereview.chromium.org/17395006/diff/1/LayoutTests/http/tests/media/media-source/webkitmediasource-state-changes-expected.txt File LayoutTests/http/tests/media/media-source/webkitmediasource-state-changes-expected.txt (left): https://codereview.chromium.org/17395006/diff/1/LayoutTests/http/tests/media/media-source/webkitmediasource-state-changes-expected.txt#oldcode8 LayoutTests/http/tests/media/media-source/webkitmediasource-state-changes-expected.txt:8: EVENT(playing) Removed because they weren't providing any extra value ...
7 years, 6 months ago (2013-06-18 21:35:07 UTC) #1
adamk
lgtm
7 years, 6 months ago (2013-06-18 21:45:21 UTC) #2
scherkus (not reviewing)
lgtm
7 years, 6 months ago (2013-06-18 22:10:35 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/acolwell@chromium.org/17395006/5001
7 years, 6 months ago (2013-06-18 23:07:42 UTC) #4
commit-bot: I haz the power
7 years, 6 months ago (2013-06-19 01:56:08 UTC) #5
Message was sent while issue was closed.
Change committed as 152683

Powered by Google App Engine
This is Rietveld 408576698