HTMLMediaElement play promise sometimes never resolve |
||||
Issue descriptionWhen cancelPendingEventsAndCallbacks() is called, we clear the tasks and the promises list but never actually resolve the promises. Because they are no longer in the list, if a promise was ready to be resolved, it will no longer be held alive by a HeapVector<Member<ScriptPromiseResolver>> and will get GC'd. On top of my head, I think we could move them back to m_pendingPromiseResolvers so they can be resolved as soon as possible. I need to sit down and figure out if there are no edge cases.
,
Jul 25 2016
After thinking about it, it's a bit more complex than I hoped. I think there are three solutions. 1. We don't cancel the promises reject/resolve tasks and we change the spec. If we do that, calling load() will have no impact on the promises that were already on there way to be resolved/rejected. I personally think it would make the most sense but might not be very consistent with how events are treated. 2. We cancel the tasks and reject all the promises. In term of spec, we could say that the tasks to resolve promises should be cancelled and instead, the promises should be rejected and the tasks to reject should be run immediately. 3. We don't cancel the tasks but we actually run them immediately. In other words, we cancel the queuing. It's kind of a compromise between solutions 1 and 2 given that the promises are still resolved/rejected as expected but `load()` will still stop the normal behaviour of the element. Feedback on this would be welcome :)
,
Jul 25 2016
,
Aug 1 2016
Looks like a problem in the same area as issue 592158 and https://github.com/whatwg/html/issues/869 I added https://www.chromestatus.com/metrics/feature/timeline/popularity/1265 to get an idea about the risk of one particular solution, but the usage is high enough that I think we should avoid that, since this is old behavior by now and the risk is that some videos won't play at all. Given that, I think the least crazy approach is to just leave pending play promises alone in the load algorithm, since they may eventually resolve. With that fix, only one rejection case that doesn't change the paused attribute would remain, in https://html.spec.whatwg.org/multipage/embedded-content.html#dedicated-media-source-failure-steps. It would be nice to investigate if it's safe to set the paused attribute in that case, and if not drop that as well :/
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e2c683031c305abcb6dab162832f95fa9b71545e commit e2c683031c305abcb6dab162832f95fa9b71545e Author: mlamouri <mlamouri@chromium.org> Date: Thu Aug 04 15:32:36 2016 Change play promises behaviour in the load algorithm. This is implementing the changes from the following whatwg/html PR: https://github.com/whatwg/html/pull/1621 BUG= 592158 , 630622 R=foolip@chromium.org Review-Url: https://codereview.chromium.org/2199363002 Cr-Commit-Position: refs/heads/master@{#409793} [modify] https://crrev.com/e2c683031c305abcb6dab162832f95fa9b71545e/third_party/WebKit/LayoutTests/media/media-play-promise.html [modify] https://crrev.com/e2c683031c305abcb6dab162832f95fa9b71545e/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp
,
Aug 4 2016
|
||||
►
Sign in to add a comment |
||||
Comment 1 by sheriffbot@chromium.org
, Jul 23 2016