Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2237503002:
Reject play promises when the playback reaches the end. (Closed)
|
Created:
4 years, 4 months ago by mlamouri (slow - plz ping) Modified:
4 years, 4 months ago Reviewers:
foolip CC:
blink-reviews, blink-reviews-html_chromium.org, chromium-reviews, dglazkov+blink, eric.carlson_apple.com, feature-media-reviews_chromium.org, fs, gasubic, mlamouri+watch-blink_chromium.org, nessy, vcarbune.chromium Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionReject play promises when the playback reaches the end.
This is implementing a recent change in the HTML specification.
BUG=636226
TEST=fuzzer
R=foolip@chromium.org
Committed: https://crrev.com/cbed5d5488c6b659aee576611b0e21efc29a0dce
Cr-Commit-Position: refs/heads/master@{#413122}
Patch Set 1 #
Total comments: 2
Messages
Total messages: 15 (7 generated)
The CQ bit was checked by mlamouri@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
PTAL. Writing tests is pretty damn hard given the nature of the issue. Will gladly leave it to the fuzzer :)
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
Description was changed from ========== Reject play promises when the playback reaches the end. BUG=636226 TEST=fuzzer R=foolip@chromium.org ========== to ========== Reject play promises when the playback reaches the end. This is implementing a recent change in the HTML specification. BUG=636226 TEST=fuzzer R=foolip@chromium.org ==========
Review ping?
On 2016/08/12 at 15:52:31, Mounir Lamouri wrote: > Review ping? ditto
lgtm % TODO https://codereview.chromium.org/2237503002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right): https://codereview.chromium.org/2237503002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2916: scheduleRejectPlayPromises(AbortError); Add a TODO to add event firing and rejection in the same task? Maybe you have an umbrella bug for these GenericEventQueue vs. other tasks already.
Thanks for the review :) https://codereview.chromium.org/2237503002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/html/HTMLMediaElement.cpp (right): https://codereview.chromium.org/2237503002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/html/HTMLMediaElement.cpp:2916: scheduleRejectPlayPromises(AbortError); On 2016/08/18 at 20:16:17, foolip wrote: > Add a TODO to add event firing and rejection in the same task? Maybe you have an umbrella bug for these GenericEventQueue vs. other tasks already. It is bug 587871. Given that we never bundle promises and events in one task, I don't think adding TODO all over the code would make much sense and adding TODO in some places and not other would be strange.
The CQ bit was checked by mlamouri@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
Description was changed from ========== Reject play promises when the playback reaches the end. This is implementing a recent change in the HTML specification. BUG=636226 TEST=fuzzer R=foolip@chromium.org ========== to ========== Reject play promises when the playback reaches the end. This is implementing a recent change in the HTML specification. BUG=636226 TEST=fuzzer R=foolip@chromium.org Committed: https://crrev.com/cbed5d5488c6b659aee576611b0e21efc29a0dce Cr-Commit-Position: refs/heads/master@{#413122} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/cbed5d5488c6b659aee576611b0e21efc29a0dce Cr-Commit-Position: refs/heads/master@{#413122} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
