Changeset 271531 in webkit

Timestamp:

Jan 15, 2021 12:53:32 PM (18 months ago)

Author:

jer.noble@apple.com

Message:

Playback fails at marketwatch.com
​https://bugs.webkit.org/show_bug.cgi?id=220646
<rdar://72950166>

Reviewed by Xabier Rodriguez-Calvar.

Source/WebCore:

Test: media/media-play-promise-reject-play-notallowed-audio.html

When audio playback is blocked by settings, the HTMLMediaElement must load its source
media's metadata in order to determine whether the media should be allowed to play without a
user gesture. If a play promise is pending, the expectation is that those promises will
reject with a NotAllowedError to indicate that a user gesture is needed. However, by calling
pauseInternal() to block (possibly) existing playback, this causes those promises to be
rejected with an AbortError, as if the pause() method had been called. Call
scheduleRejectPendingPlayPromises() with NotAllowedError to ensure the correct error is used
to reject.

Drive-by fix: no reason to dispatch and call rejectPendingPlayPromises() or
resolvePendingPlayPromises() if there are no promises to reject or resolve, and not calling
these methods makes the logs less noisy.

• html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::scheduleResolvePendingPlayPromises):
(WebCore::HTMLMediaElement::scheduleRejectPendingPlayPromises):
(WebCore::HTMLMediaElement::setVolume):
(WebCore::HTMLMediaElement::mediaPlayerDidAddAudioTrack):
(WebCore::HTMLMediaElement::mediaPlayerCharacteristicChanged):
(WebCore::HTMLMediaElement::updateShouldPlay):

LayoutTests:

• media/media-play-promise-reject-play-notallowed-audio-expected.txt: Added.
• media/media-play-promise-reject-play-notallowed-audio.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/media-play-promise-reject-play-notallowed-audio-expected.txt (added)
• LayoutTests/media/media-play-promise-reject-play-notallowed-audio.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLMediaElement.cpp (modified) (6 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-01-15  Jer Noble  <jer.noble@apple.com>
+
+        Playback fails at marketwatch.com
+        https://bugs.webkit.org/show_bug.cgi?id=220646
+        <rdar://72950166>
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        * media/media-play-promise-reject-play-notallowed-audio-expected.txt: Added.
+        * media/media-play-promise-reject-play-notallowed-audio.html: Added.
+
 2021-01-15  Antoine Quint  <graouts@webkit.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-01-15  Jer Noble  <jer.noble@apple.com>
+
+        Playback fails at marketwatch.com
+        https://bugs.webkit.org/show_bug.cgi?id=220646
+        <rdar://72950166>
+
+        Reviewed by Xabier Rodriguez-Calvar.
+
+        Test: media/media-play-promise-reject-play-notallowed-audio.html
+
+        When audio playback is blocked by settings, the HTMLMediaElement must load its source
+        media's metadata in order to determine whether the media should be allowed to play without a
+        user gesture. If a play promise is pending, the expectation is that those promises will
+        reject with a NotAllowedError to indicate that a user gesture is needed. However, by calling
+        pauseInternal() to block (possibly) existing playback, this causes those promises to be
+        rejected with an AbortError, as if the pause() method had been called. Call
+        scheduleRejectPendingPlayPromises() with NotAllowedError to ensure the correct error is used
+        to reject.
+
+        Drive-by fix: no reason to dispatch and call rejectPendingPlayPromises() or
+        resolvePendingPlayPromises() if there are no promises to reject or resolve, and not calling
+        these methods makes the logs less noisy.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::scheduleResolvePendingPlayPromises):
+        (WebCore::HTMLMediaElement::scheduleRejectPendingPlayPromises):
+        (WebCore::HTMLMediaElement::setVolume):
+        (WebCore::HTMLMediaElement::mediaPlayerDidAddAudioTrack):
+        (WebCore::HTMLMediaElement::mediaPlayerCharacteristicChanged):
+        (WebCore::HTMLMediaElement::updateShouldPlay):
+
 2021-01-15  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/html/HTMLMediaElement.cpp

@@ -909 +909 @@
 void HTMLMediaElement::scheduleResolvePendingPlayPromises()
 {
+    if (m_pendingPlayPromises.isEmpty())
+        return;
+
     m_promiseTaskQueue.enqueueTask([this, pendingPlayPromises = WTFMove(m_pendingPlayPromises)] () mutable {
         resolvePendingPlayPromises(WTFMove(pendingPlayPromises));
@@ -916 +919 @@
 void HTMLMediaElement::scheduleRejectPendingPlayPromises(Ref<DOMException>&& error)
 {
+    if (m_pendingPlayPromises.isEmpty())
+        return;
+
     m_promiseTaskQueue.enqueueTask([this, error = WTFMove(error), pendingPlayPromises = WTFMove(m_pendingPlayPromises)] () mutable {
         rejectPendingPlayPromises(WTFMove(pendingPlayPromises), WTFMove(error));
@@ -3610 +3616 @@
 
     if (isPlaying() && !m_mediaSession->playbackPermitted()) {
+        scheduleRejectPendingPlayPromises(DOMException::create(NotAllowedError));
         pauseInternal();
         setAutoplayEventPlaybackState(AutoplayEventPlaybackState::PreventedAutoplay);
@@ -3877 +3884 @@
 {
     if (isPlaying() && !m_mediaSession->playbackPermitted()) {
+        scheduleRejectPendingPlayPromises(DOMException::create(NotAllowedError));
         pauseInternal();
         setAutoplayEventPlaybackState(AutoplayEventPlaybackState::PreventedAutoplay);
@@ -5051 +5059 @@
 
     if (!paused() && !m_mediaSession->playbackPermitted()) {
+        scheduleRejectPendingPlayPromises(DOMException::create(NotAllowedError));
         pauseInternal();
         setAutoplayEventPlaybackState(AutoplayEventPlaybackState::PreventedAutoplay);
@@ -7803 +7812 @@
 {
     if (!paused() && !m_mediaSession->playbackPermitted()) {
+        scheduleRejectPendingPlayPromises(DOMException::create(NotAllowedError));
         pauseInternal();
         setAutoplayEventPlaybackState(AutoplayEventPlaybackState::PreventedAutoplay);