- Revision
- 291683
- Author
- repst...@apple.com
- Date
- 2022-03-22 11:37:26 -0700 (Tue, 22 Mar 2022)
Log Message
Cherry-pick r291629. rdar://problem/89495774
YouTube.com - Clicking anywhere on the progress bar pauses the video
https://bugs.webkit.org/show_bug.cgi?id=237750
<rdar://problem/90364846>
Reviewed by Eric Carlson.
Source/WebCore:
Test: media/media-source/media-source-unexpected-pause.html
When calling play() or pause() on a MediaPlayerPrivateMediaSourceAVFObjC, the object will
respond by calling m_player->playbackStateChanged(), which will in turn call
HTMLMediaElement::mediaPlayerPlaybackStateChanged() with the new state. However, HTMLMediaElement
expects this to be called only for unanticipated state changes, not expected state changes. And
when that method is called and the reported state does not match the element's own expected state,
the element calls its own play() or pause() function to update its own state to match the player's.
And because MediaPlayerPrivateMediaSourceAVFObjC calls this method on the next run loop, there is
an opportunity for those states to get out of sync, which happens when YouTube responds to a tap
in its timeline.
Remove the unnecessary "call on next run loop" behavior of MediaPlayerPrivateMediaSourceAVFObjC::
play() and ::pause(). Also, remove the unnecessary notification that the play state has changed. In
the future, this can be accomplished by adding a callback parameter to MediaPlayer::play() rather than
relying on a state change notification.
* platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playInternal):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playAtHostTime):
(WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseAtHostTime):
LayoutTests:
* media/media-source/media-source-unexpected-pause-expected.txt: Added.
* media/media-source/media-source-unexpected-pause.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291629 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-614.1.7-branch/LayoutTests/ChangeLog (291682 => 291683)
--- branches/safari-614.1.7-branch/LayoutTests/ChangeLog 2022-03-22 18:37:22 UTC (rev 291682)
+++ branches/safari-614.1.7-branch/LayoutTests/ChangeLog 2022-03-22 18:37:26 UTC (rev 291683)
@@ -1,3 +1,59 @@
+2022-03-22 Russell Epstein <repst...@apple.com>
+
+ Cherry-pick r291629. rdar://problem/89495774
+
+ YouTube.com - Clicking anywhere on the progress bar pauses the video
+ https://bugs.webkit.org/show_bug.cgi?id=237750
+ <rdar://problem/90364846>
+
+ Reviewed by Eric Carlson.
+
+ Source/WebCore:
+
+ Test: media/media-source/media-source-unexpected-pause.html
+
+ When calling play() or pause() on a MediaPlayerPrivateMediaSourceAVFObjC, the object will
+ respond by calling m_player->playbackStateChanged(), which will in turn call
+ HTMLMediaElement::mediaPlayerPlaybackStateChanged() with the new state. However, HTMLMediaElement
+ expects this to be called only for unanticipated state changes, not expected state changes. And
+ when that method is called and the reported state does not match the element's own expected state,
+ the element calls its own play() or pause() function to update its own state to match the player's.
+ And because MediaPlayerPrivateMediaSourceAVFObjC calls this method on the next run loop, there is
+ an opportunity for those states to get out of sync, which happens when YouTube responds to a tap
+ in its timeline.
+
+ Remove the unnecessary "call on next run loop" behavior of MediaPlayerPrivateMediaSourceAVFObjC::
+ play() and ::pause(). Also, remove the unnecessary notification that the play state has changed. In
+ the future, this can be accomplished by adding a callback parameter to MediaPlayer::play() rather than
+ relying on a state change notification.
+
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playInternal):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playAtHostTime):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseAtHostTime):
+
+ LayoutTests:
+
+ * media/media-source/media-source-unexpected-pause-expected.txt: Added.
+ * media/media-source/media-source-unexpected-pause.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291629 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-03-22 Jer Noble <jer.no...@apple.com>
+
+ YouTube.com - Clicking anywhere on the progress bar pauses the video
+ https://bugs.webkit.org/show_bug.cgi?id=237750
+ <rdar://problem/90364846>
+
+ Reviewed by Eric Carlson.
+
+ * media/media-source/media-source-unexpected-pause-expected.txt: Added.
+ * media/media-source/media-source-unexpected-pause.html: Added.
+
2022-03-18 Ryan Haddad <ryanhad...@apple.com>
[macOS arm64] webrtc/vp9-profile2.html is consistently timing out
Added: branches/safari-614.1.7-branch/LayoutTests/media/media-source/media-source-unexpected-pause-expected.txt (0 => 291683)
--- branches/safari-614.1.7-branch/LayoutTests/media/media-source/media-source-unexpected-pause-expected.txt (rev 0)
+++ branches/safari-614.1.7-branch/LayoutTests/media/media-source/media-source-unexpected-pause-expected.txt 2022-03-22 18:37:26 UTC (rev 291683)
@@ -0,0 +1,17 @@
+
+RUN(video.src = ""
+EVENT(sourceopen)
+RUN(source.duration = loader.duration())
+RUN(sourceBuffer = source.addSourceBuffer(loader.type()))
+RUN(sourceBuffer.appendBuffer(loader.initSegment()))
+EVENT(update)
+Append a media segment.
+RUN(sourceBuffer.appendBuffer(loader.mediaSegment(0)))
+EVENT(update)
+RUN(video.play())
+EVENT(playing)
+RUN(video.pause())
+RUN(video.play())
+EVENT(playing)
+END OF TEST
+
Added: branches/safari-614.1.7-branch/LayoutTests/media/media-source/media-source-unexpected-pause.html (0 => 291683)
--- branches/safari-614.1.7-branch/LayoutTests/media/media-source/media-source-unexpected-pause.html (rev 0)
+++ branches/safari-614.1.7-branch/LayoutTests/media/media-source/media-source-unexpected-pause.html 2022-03-22 18:37:26 UTC (rev 291683)
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <title>media-source-unexpected-pause</title>
+ <script src=""
+ <script src=""
+ <script>
+ var loader;
+ var source;
+ var sourceBuffer;
+
+ function loaderPromise(loader) {
+ return new Promise((resolve, reject) => {
+ loader._onload_ = resolve;
+ loader._onerror_ = reject;
+ });
+ }
+
+ async function runTest() {
+ findMediaElement();
+ loader = new MediaSourceLoader('content/test-fragmented-manifest.json');
+ await loaderPromise(loader);
+
+ source = new MediaSource();
+ run('video.src = ""
+ await waitFor(source, 'sourceopen');
+ waitFor(source, 'error').then(endTest);
+ waitForEventAndFail('error');
+
+ run('source.duration = loader.duration()');
+ run('sourceBuffer = source.addSourceBuffer(loader.type())');
+ run('sourceBuffer.appendBuffer(loader.initSegment())');
+
+ await waitFor(sourceBuffer, 'update');
+
+ consoleWrite('Append a media segment.')
+ run('sourceBuffer.appendBuffer(loader.mediaSegment(0))');
+
+ await waitFor(sourceBuffer, 'update');
+ await sleepFor(100);
+
+ run('video.play()');
+ await waitFor(video, 'playing');
+ await sleepFor(100);
+ run('video.pause()');
+ await sleepFor(0);
+ await sleepFor(0);
+ run('video.play()');
+ waitForAndFail(video, 'pause');
+ await waitFor(video, 'playing');
+ await sleepFor(100);
+ }
+
+ window.addEventListener('load', event => {
+ runTest().then(endTest).catch(failTest);
+ });
+ </script>
+</head>
+<body>
+ <video muted controls></video>
+</body>
+</html>
\ No newline at end of file
Modified: branches/safari-614.1.7-branch/Source/WebCore/ChangeLog (291682 => 291683)
--- branches/safari-614.1.7-branch/Source/WebCore/ChangeLog 2022-03-22 18:37:22 UTC (rev 291682)
+++ branches/safari-614.1.7-branch/Source/WebCore/ChangeLog 2022-03-22 18:37:26 UTC (rev 291683)
@@ -1,3 +1,81 @@
+2022-03-22 Russell Epstein <repst...@apple.com>
+
+ Cherry-pick r291629. rdar://problem/89495774
+
+ YouTube.com - Clicking anywhere on the progress bar pauses the video
+ https://bugs.webkit.org/show_bug.cgi?id=237750
+ <rdar://problem/90364846>
+
+ Reviewed by Eric Carlson.
+
+ Source/WebCore:
+
+ Test: media/media-source/media-source-unexpected-pause.html
+
+ When calling play() or pause() on a MediaPlayerPrivateMediaSourceAVFObjC, the object will
+ respond by calling m_player->playbackStateChanged(), which will in turn call
+ HTMLMediaElement::mediaPlayerPlaybackStateChanged() with the new state. However, HTMLMediaElement
+ expects this to be called only for unanticipated state changes, not expected state changes. And
+ when that method is called and the reported state does not match the element's own expected state,
+ the element calls its own play() or pause() function to update its own state to match the player's.
+ And because MediaPlayerPrivateMediaSourceAVFObjC calls this method on the next run loop, there is
+ an opportunity for those states to get out of sync, which happens when YouTube responds to a tap
+ in its timeline.
+
+ Remove the unnecessary "call on next run loop" behavior of MediaPlayerPrivateMediaSourceAVFObjC::
+ play() and ::pause(). Also, remove the unnecessary notification that the play state has changed. In
+ the future, this can be accomplished by adding a callback parameter to MediaPlayer::play() rather than
+ relying on a state change notification.
+
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playInternal):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playAtHostTime):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseAtHostTime):
+
+ LayoutTests:
+
+ * media/media-source/media-source-unexpected-pause-expected.txt: Added.
+ * media/media-source/media-source-unexpected-pause.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@291629 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2022-03-22 Jer Noble <jer.no...@apple.com>
+
+ YouTube.com - Clicking anywhere on the progress bar pauses the video
+ https://bugs.webkit.org/show_bug.cgi?id=237750
+ <rdar://problem/90364846>
+
+ Reviewed by Eric Carlson.
+
+ Test: media/media-source/media-source-unexpected-pause.html
+
+ When calling play() or pause() on a MediaPlayerPrivateMediaSourceAVFObjC, the object will
+ respond by calling m_player->playbackStateChanged(), which will in turn call
+ HTMLMediaElement::mediaPlayerPlaybackStateChanged() with the new state. However, HTMLMediaElement
+ expects this to be called only for unanticipated state changes, not expected state changes. And
+ when that method is called and the reported state does not match the element's own expected state,
+ the element calls its own play() or pause() function to update its own state to match the player's.
+ And because MediaPlayerPrivateMediaSourceAVFObjC calls this method on the next run loop, there is
+ an opportunity for those states to get out of sync, which happens when YouTube responds to a tap
+ in its timeline.
+
+ Remove the unnecessary "call on next run loop" behavior of MediaPlayerPrivateMediaSourceAVFObjC::
+ play() and ::pause(). Also, remove the unnecessary notification that the play state has changed. In
+ the future, this can be accomplished by adding a callback parameter to MediaPlayer::play() rather than
+ relying on a state change notification.
+
+ * platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm:
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::play):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playInternal):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pause):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::playAtHostTime):
+ (WebCore::MediaPlayerPrivateMediaSourceAVFObjC::pauseAtHostTime):
+
2022-03-21 Russell Epstein <repst...@apple.com>
Cherry-pick r291564. rdar://problem/90463946
Modified: branches/safari-614.1.7-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm (291682 => 291683)
--- branches/safari-614.1.7-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm 2022-03-22 18:37:22 UTC (rev 291682)
+++ branches/safari-614.1.7-branch/Source/WebCore/platform/graphics/avfoundation/objc/MediaPlayerPrivateMediaSourceAVFObjC.mm 2022-03-22 18:37:26 UTC (rev 291683)
@@ -314,11 +314,7 @@
void MediaPlayerPrivateMediaSourceAVFObjC::play()
{
ALWAYS_LOG(LOGIDENTIFIER);
- callOnMainThread([weakThis = WeakPtr { *this }] {
- if (!weakThis)
- return;
- weakThis.get()->playInternal();
- });
+ playInternal();
}
void MediaPlayerPrivateMediaSourceAVFObjC::playInternal(std::optional<MonotonicTime>&& hostTime)
@@ -352,18 +348,12 @@
UNUSED_PARAM(hostTime);
[m_synchronizer setRate:m_rate];
#endif
-
- m_player->playbackStateChanged();
}
void MediaPlayerPrivateMediaSourceAVFObjC::pause()
{
ALWAYS_LOG(LOGIDENTIFIER);
- callOnMainThread([weakThis = WeakPtr { *this }] {
- if (!weakThis)
- return;
- weakThis.get()->pauseInternal();
- });
+ pauseInternal();
}
void MediaPlayerPrivateMediaSourceAVFObjC::pauseInternal(std::optional<MonotonicTime>&& hostTime)
@@ -384,8 +374,6 @@
UNUSED_PARAM(hostTime);
[m_synchronizer setRate:0];
#endif
-
- m_player->playbackStateChanged();
}
bool MediaPlayerPrivateMediaSourceAVFObjC::paused() const
@@ -479,11 +467,7 @@
bool MediaPlayerPrivateMediaSourceAVFObjC::playAtHostTime(const MonotonicTime& time)
{
ALWAYS_LOG(LOGIDENTIFIER);
- callOnMainThread([weakThis = WeakPtr { *this }, time = time] {
- if (!weakThis)
- return;
- weakThis.get()->playInternal(time);
- });
+ playInternal(time);
return true;
}
@@ -490,11 +474,7 @@
bool MediaPlayerPrivateMediaSourceAVFObjC::pauseAtHostTime(const MonotonicTime& time)
{
ALWAYS_LOG(LOGIDENTIFIER);
- callOnMainThread([weakThis = WeakPtr { *this }, time = time] {
- if (!weakThis)
- return;
- weakThis.get()->pauseInternal(time);
- });
+ pauseInternal(time);
return true;
}
#endif