diff --git a/third_party/WebKit/LayoutTests/media/media-play-promise-expected.txt b/third_party/WebKit/LayoutTests/media/media-play-promise-expected.txt index 3cf066835ddd1..66d8677c324ac 100644 --- a/third_party/WebKit/LayoutTests/media/media-play-promise-expected.txt +++ b/third_party/WebKit/LayoutTests/media/media-play-promise-expected.txt @@ -167,5 +167,20 @@ play() RUN(mediaElement.volume = 0.2) arguments.length: 1 Promise failed with NotSupportedError: The element has no supported sources. + +pausePlayAfterPlaybackStarted() +RUN(mediaElement = document.createElement('audio')) +RUN(mediaElement.src = 'content/test.wav') +EVENT(volumechange) +EVENT(volumechange) +EVENT(canplaythrough) +RUN(mediaElement.play()) +EVENT(playing) +EXPECTED (mediaElement.readyState == '4') OK +EXPECTED (mediaElement.paused == 'false') OK +RUN(mediaElement.pause()) +play() +arguments.length: 1 +Promise resolved with undefined END OF TEST diff --git a/third_party/WebKit/LayoutTests/media/media-play-promise.html b/third_party/WebKit/LayoutTests/media/media-play-promise.html index 77b4ac76d873b..992b0b0654d02 100644 --- a/third_party/WebKit/LayoutTests/media/media-play-promise.html +++ b/third_party/WebKit/LayoutTests/media/media-play-promise.html @@ -65,7 +65,7 @@ var TESTS = [ // Test that play() on an element that doesn't have enough data will - // return a promise that resolves successfuly. + // return a promise that resolves successfully. function playBeforeCanPlay() { consoleWrite("playBeforeCanPlay()"); @@ -85,7 +85,7 @@ }, // Test that play() on an element that has enough data will return a - // promise that resolves successfuly. + // promise that resolves successfully. function playWhenCanPlay() { consoleWrite("playWhenCanPlay()"); @@ -105,6 +105,8 @@ }); }, + // Test that play() on an element that is already playing returns a + // promise that resolves successfully. function playAfterPlaybackStarted() { consoleWrite("playAfterPlaybackStarted()"); @@ -391,7 +393,32 @@ run("mediaElement.volume = 0.2"); }); - } + }, + + // Test that calling pause() then play() on an element that is already + // playing returns a promise that resolves successfully. + function pausePlayAfterPlaybackStarted() + { + consoleWrite("pausePlayAfterPlaybackStarted()"); + internals.settings.setMediaPlaybackRequiresUserGesture(false); + + run("mediaElement = document.createElement('audio')"); + mediaElement.preload = "auto"; + var mediaFile = findMediaFile("audio", "content/test"); + run("mediaElement.src = '" + mediaFile + "'"); + + waitForEvent('playing', function() { + testExpected("mediaElement.readyState", HTMLMediaElement.HAVE_ENOUGH_DATA); + testExpected("mediaElement.paused", false); + + run("mediaElement.pause()"); + play(); + }); + + waitForEvent('canplaythrough', function() { + run("mediaElement.play()"); + }); + }, ]; function start() diff --git a/third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit-expected.txt b/third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit-expected.txt index 4deaded51e524..7fd3e15c55da3 100644 --- a/third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit-expected.txt +++ b/third_party/WebKit/LayoutTests/media/track/track-cues-pause-on-exit-expected.txt @@ -1,4 +1,3 @@ -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). Tests that the video is paused after cues that have pause-on-exit flag are processed EVENT(canplaythrough) diff --git a/third_party/WebKit/LayoutTests/media/video-controls-overlay-play-button-expected.txt b/third_party/WebKit/LayoutTests/media/video-controls-overlay-play-button-expected.txt index eb10c6a2cb2e7..277b5dba4e119 100644 --- a/third_party/WebKit/LayoutTests/media/video-controls-overlay-play-button-expected.txt +++ b/third_party/WebKit/LayoutTests/media/video-controls-overlay-play-button-expected.txt @@ -1,4 +1,3 @@ -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). Test that the overlay play button respects the controls attribute EXPECTED (getComputedStyle(button).display == 'flex') OK diff --git a/third_party/WebKit/LayoutTests/media/video-played-collapse-expected.txt b/third_party/WebKit/LayoutTests/media/video-played-collapse-expected.txt index 7af7151890405..02533c6861d11 100644 --- a/third_party/WebKit/LayoutTests/media/video-played-collapse-expected.txt +++ b/third_party/WebKit/LayoutTests/media/video-played-collapse-expected.txt @@ -1,6 +1,3 @@ -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). Test of the media element 'played' attribute EVENT(loadstart) diff --git a/third_party/WebKit/LayoutTests/media/video-played-ranges-1-expected.txt b/third_party/WebKit/LayoutTests/media/video-played-ranges-1-expected.txt index 9bcc4894e9832..eeb5fd07896bd 100644 --- a/third_party/WebKit/LayoutTests/media/video-played-ranges-1-expected.txt +++ b/third_party/WebKit/LayoutTests/media/video-played-ranges-1-expected.txt @@ -1,7 +1,3 @@ -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). -CONSOLE ERROR: Uncaught (in promise) AbortError: The play() request was interrupted by a call to pause(). Test of the media element 'played' attribute, ranges part 1. EVENT(loadstart) diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp index 246e0c42c49ca..497fc9d504d7e 100644 --- a/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp +++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.cpp @@ -444,8 +444,8 @@ HTMLMediaElement::HTMLMediaElement(const QualifiedName& tagName, Document& docum , m_audioTracks(AudioTrackList::create(*this)) , m_videoTracks(VideoTrackList::create(*this)) , m_textTracks(nullptr) - , m_playPromiseResolveTask(CancellableTaskFactory::create(this, &HTMLMediaElement::resolvePlayPromises)) - , m_playPromiseRejectTask(CancellableTaskFactory::create(this, &HTMLMediaElement::rejectPlayPromises)) + , m_playPromiseResolveTask(CancellableTaskFactory::create(this, &HTMLMediaElement::resolveScheduledPlayPromises)) + , m_playPromiseRejectTask(CancellableTaskFactory::create(this, &HTMLMediaElement::rejectScheduledPlayPromises)) , m_audioSourceNode(nullptr) , m_autoplayHelperClient(AutoplayHelperClientImpl::create(this)) , m_autoplayHelper(AutoplayExperimentHelper::create(m_autoplayHelperClient.get())) @@ -1404,7 +1404,9 @@ void HTMLMediaElement::cancelPendingEventsAndCallbacks() source->cancelPendingErrorEvent(); m_playPromiseResolveTask->cancel(); + m_playPromiseResolveList.clear(); m_playPromiseRejectTask->cancel(); + m_playPromiseRejectList.clear(); } void HTMLMediaElement::networkStateChanged() @@ -2029,8 +2031,20 @@ WebMediaPlayer::Preload HTMLMediaElement::effectivePreloadType() const ScriptPromise HTMLMediaElement::playForBindings(ScriptState* scriptState) { + // We have to share the same logic for internal and external callers. The + // internal callers do not want to receive a Promise back but when ::play() + // is called, |m_playPromiseResolvers| needs to be populated. What this code + // does is to populate |m_playPromiseResolvers| before calling ::play() and + // remove the Promise if ::play() failed. + ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState); + ScriptPromise promise = resolver->promise(); + m_playPromiseResolvers.append(resolver); + Nullable code = play(); if (!code.isNull()) { + DCHECK(!m_playPromiseResolvers.isEmpty()); + m_playPromiseResolvers.removeLast(); + String message; switch (code.get()) { case NotAllowedError: @@ -2042,13 +2056,10 @@ ScriptPromise HTMLMediaElement::playForBindings(ScriptState* scriptState) default: ASSERT_NOT_REACHED(); } - return ScriptPromise::rejectWithDOMException(scriptState, DOMException::create(code.get(), message)); + resolver->reject(DOMException::create(code.get(), message)); + return promise; } - ScriptPromiseResolver* resolver = ScriptPromiseResolver::create(scriptState); - ScriptPromise promise = resolver->promise(); - - m_playResolvers.append(resolver); return promise; } @@ -3611,7 +3622,9 @@ DEFINE_TRACE(HTMLMediaElement) visitor->trace(m_cueTimeline); visitor->trace(m_textTracks); visitor->trace(m_textTracksWhenResourceSelectionBegan); - visitor->trace(m_playResolvers); + visitor->trace(m_playPromiseResolvers); + visitor->trace(m_playPromiseResolveList); + visitor->trace(m_playPromiseRejectList); visitor->trace(m_audioSourceProvider); visitor->trace(m_autoplayHelperClient); visitor->trace(m_autoplayHelper); @@ -3708,10 +3721,19 @@ void HTMLMediaElement::triggerAutoplayViewportCheckForTesting() void HTMLMediaElement::scheduleResolvePlayPromises() { - // Per spec, if there are two tasks in the queue, the first task will remove - // all the pending promises making the second task useless unless a promise - // can be added between the first and second task being run which is not - // possible at the moment. + // TODO(mlamouri): per spec, we should create a new task but we can't create + // a new cancellable task without cancelling the previous one. There are two + // approaches then: cancel the previous task and create a new one with the + // appended promise list or append the new promise to the current list. The + // latter approach is preferred because it might be the less observable + // change. + DCHECK(m_playPromiseResolveList.isEmpty() || m_playPromiseResolveTask->isPending()); + if (m_playPromiseResolvers.isEmpty()) + return; + + m_playPromiseResolveList.appendVector(m_playPromiseResolvers); + m_playPromiseResolvers.clear(); + if (m_playPromiseResolveTask->isPending()) return; @@ -3720,10 +3742,19 @@ void HTMLMediaElement::scheduleResolvePlayPromises() void HTMLMediaElement::scheduleRejectPlayPromises(ExceptionCode code) { - // Per spec, if there are two tasks in the queue, the first task will remove - // all the pending promises making the second task useless unless a promise - // can be added between the first and second task being run which is not - // possible at the moment. + // TODO(mlamouri): per spec, we should create a new task but we can't create + // a new cancellable task without cancelling the previous one. There are two + // approaches then: cancel the previous task and create a new one with the + // appended promise list or append the new promise to the current list. The + // latter approach is preferred because it might be the less observable + // change. + DCHECK(m_playPromiseRejectList.isEmpty() || m_playPromiseRejectTask->isPending()); + if (m_playPromiseResolvers.isEmpty()) + return; + + m_playPromiseRejectList.appendVector(m_playPromiseResolvers); + m_playPromiseResolvers.clear(); + if (m_playPromiseRejectTask->isPending()) return; @@ -3739,34 +3770,41 @@ void HTMLMediaElement::scheduleNotifyPlaying() scheduleResolvePlayPromises(); } -void HTMLMediaElement::resolvePlayPromises() +void HTMLMediaElement::resolveScheduledPlayPromises() { - for (auto& resolver: m_playResolvers) + for (auto& resolver: m_playPromiseResolveList) resolver->resolve(); - m_playResolvers.clear(); + m_playPromiseResolveList.clear(); } -void HTMLMediaElement::rejectPlayPromises() +void HTMLMediaElement::rejectScheduledPlayPromises() { // TODO(mlamouri): the message is generated based on the code because // arguments can't be passed to a cancellable task. In order to save space // used by the object, the string isn't saved. - ASSERT(m_playPromiseErrorCode == AbortError || m_playPromiseErrorCode == NotSupportedError); + DCHECK(m_playPromiseErrorCode == AbortError || m_playPromiseErrorCode == NotSupportedError); if (m_playPromiseErrorCode == AbortError) - rejectPlayPromises(AbortError, "The play() request was interrupted by a call to pause()."); + rejectPlayPromisesInternal(AbortError, "The play() request was interrupted by a call to pause()."); else - rejectPlayPromises(NotSupportedError, "Failed to load because no supported source was found."); + rejectPlayPromisesInternal(NotSupportedError, "Failed to load because no supported source was found."); } void HTMLMediaElement::rejectPlayPromises(ExceptionCode code, const String& message) { - ASSERT(code == AbortError || code == NotSupportedError); + m_playPromiseRejectList.appendVector(m_playPromiseResolvers); + m_playPromiseResolvers.clear(); + rejectPlayPromisesInternal(code, message); +} + +void HTMLMediaElement::rejectPlayPromisesInternal(ExceptionCode code, const String& message) +{ + DCHECK(code == AbortError || code == NotSupportedError); - for (auto& resolver: m_playResolvers) + for (auto& resolver: m_playPromiseRejectList) resolver->reject(DOMException::create(code, message)); - m_playResolvers.clear(); + m_playPromiseRejectList.clear(); } EnumerationHistogram& HTMLMediaElement::showControlsHistogram() const diff --git a/third_party/WebKit/Source/core/html/HTMLMediaElement.h b/third_party/WebKit/Source/core/html/HTMLMediaElement.h index 3aad6e757f7bc..7166e16b7b772 100644 --- a/third_party/WebKit/Source/core/html/HTMLMediaElement.h +++ b/third_party/WebKit/Source/core/html/HTMLMediaElement.h @@ -478,12 +478,10 @@ class CORE_EXPORT HTMLMediaElement : public HTMLElement, public Supplementable m_cueTimeline; - HeapVector> m_playResolvers; + HeapVector> m_playPromiseResolvers; OwnPtr m_playPromiseResolveTask; OwnPtr m_playPromiseRejectTask; + HeapVector> m_playPromiseResolveList; + HeapVector> m_playPromiseRejectList; ExceptionCode m_playPromiseErrorCode; // This is a weak reference, since m_audioSourceNode holds a reference to us.