Diff
Modified: branches/safari-611-branch/Source/WebCore/ChangeLog (272320 => 272321)
--- branches/safari-611-branch/Source/WebCore/ChangeLog 2021-02-03 17:33:36 UTC (rev 272320)
+++ branches/safari-611-branch/Source/WebCore/ChangeLog 2021-02-03 17:36:30 UTC (rev 272321)
@@ -1,5 +1,44 @@
2021-02-03 Russell Epstein <[email protected]>
+ Apply patch. rdar://problem/73890906
+
+ 2021-02-03 Peng Liu <[email protected]>
+
+ Twitter PiP video pauses when scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=220887
+
+ This patch adds two quirks (requiresUserGestureToPauseInPictureInPicture and
+ requiresUserGestureToLoadInPictureInPicture) for twitter.com, so that when we scroll
+ the page while a video is in picture-in-picture, the video won't pause or close.
+
+ This patch also fixes a race condition related to function MediaElementSession::playbackPermitted()
+ by adding a parameter (MediaPlaybackOperation) to it. Because of the race condition, we
+ cannot resume a paused video in a picture-in-picture window on some sites (e.g., twitter.com).
+ That happens because when we click the play button on the picture-in-picture window to resume
+ a video, MediaElementSession::playbackPermitted() will be called by
+ HTMLMediaElement::mediaPlayerDidAddAudioTrack() when HTMLMediaElement::pause() returns false
+ (means the playback has already been resumed), so the request to add audio track will be rejected
+ due to the requiresUserGestureToPauseInPictureInPicture quirk and the video will be paused.
+ This patch fixes this race condition by enabling the requiresUserGestureToPauseInPictureInPicture
+ quirk only when the playback operation is "pause".
+
+ Reviewed by Eric Carlson.
+
+ * html/HTMLMediaElement.cpp:
+ (WebCore::HTMLMediaElement::load):
+ (WebCore::HTMLMediaElement::pause):
+ (WebCore::HTMLMediaElement::suspendPlayback): We should use pauseInternal() instead of pause() here,
+ otherwise, it will be prevented by the requiresUserGestureToPauseInPictureInPicture quirk.
+ * html/MediaElementSession.cpp:
+ (WebCore::MediaElementSession::playbackPermitted const):
+ * html/MediaElementSession.h:
+ * page/Quirks.cpp:
+ (WebCore::Quirks::requiresUserGestureToPauseInPictureInPicture const):
+ (WebCore::Quirks::requiresUserGestureToLoadInPictureInPicture const):
+ * page/Quirks.h:
+
+2021-02-03 Russell Epstein <[email protected]>
+
Apply patch. rdar://problem/73904694
2021-02-03 Jer Noble <[email protected]>
Modified: branches/safari-611-branch/Source/WebCore/html/HTMLMediaElement.cpp (272320 => 272321)
--- branches/safari-611-branch/Source/WebCore/html/HTMLMediaElement.cpp 2021-02-03 17:33:36 UTC (rev 272320)
+++ branches/safari-611-branch/Source/WebCore/html/HTMLMediaElement.cpp 2021-02-03 17:36:30 UTC (rev 272321)
@@ -1074,6 +1074,9 @@
INFO_LOG(LOGIDENTIFIER);
+ if (m_videoFullscreenMode == VideoFullscreenModePictureInPicture && document().quirks().requiresUserGestureToLoadInPictureInPicture() && !document().processingUserGestureForMedia())
+ return;
+
prepareForLoad();
m_resourceSelectionTaskQueue.enqueueTask([this] {
prepareToPlay();
@@ -3492,7 +3495,7 @@
if (m_waitingToEnterFullscreen)
m_waitingToEnterFullscreen = false;
- if (!m_mediaSession->playbackPermitted())
+ if (!m_mediaSession->playbackPermitted(MediaPlaybackOperation::Pause))
return;
if (processingUserGestureForMedia())
@@ -3501,7 +3504,6 @@
pauseInternal();
}
-
void HTMLMediaElement::pauseInternal()
{
ALWAYS_LOG(LOGIDENTIFIER);
@@ -7441,7 +7443,7 @@
{
ALWAYS_LOG(LOGIDENTIFIER, "paused = ", paused());
if (!paused())
- pause();
+ pauseInternal();
}
void HTMLMediaElement::resumeAutoplaying()
Modified: branches/safari-611-branch/Source/WebCore/html/MediaElementSession.cpp (272320 => 272321)
--- branches/safari-611-branch/Source/WebCore/html/MediaElementSession.cpp 2021-02-03 17:33:36 UTC (rev 272320)
+++ branches/safari-611-branch/Source/WebCore/html/MediaElementSession.cpp 2021-02-03 17:36:30 UTC (rev 272321)
@@ -276,7 +276,7 @@
m_restrictions &= ~restriction;
}
-SuccessOr<MediaPlaybackDenialReason> MediaElementSession::playbackPermitted() const
+SuccessOr<MediaPlaybackDenialReason> MediaElementSession::playbackPermitted(MediaPlaybackOperation operation) const
{
if (m_element.isSuspended()) {
ALWAYS_LOG(LOGIDENTIFIER, "Returning FALSE because element is suspended");
@@ -317,7 +317,7 @@
const auto& topDocument = document.topDocument();
if (topDocument.quirks().requiresUserGestureToPauseInPictureInPicture()
&& m_element.fullscreenMode() & HTMLMediaElementEnums::VideoFullscreenModePictureInPicture
- && !m_element.paused()
+ && !m_element.paused() && operation == MediaPlaybackOperation::Pause
&& !document.processingUserGestureForMedia()) {
ALWAYS_LOG(LOGIDENTIFIER, "Returning FALSE because a quirk requires a user gesture to pause while in Picture-in-Picture");
return MediaPlaybackDenialReason::UserGestureRequired;
Modified: branches/safari-611-branch/Source/WebCore/html/MediaElementSession.h (272320 => 272321)
--- branches/safari-611-branch/Source/WebCore/html/MediaElementSession.h 2021-02-03 17:33:36 UTC (rev 272320)
+++ branches/safari-611-branch/Source/WebCore/html/MediaElementSession.h 2021-02-03 17:36:30 UTC (rev 272321)
@@ -42,6 +42,11 @@
Autoplay
};
+enum class MediaPlaybackOperation {
+ All,
+ Pause
+};
+
enum class MediaPlaybackDenialReason {
UserGestureRequired,
FullscreenRequired,
@@ -71,7 +76,8 @@
void isVisibleInViewportChanged();
void inActiveDocumentChanged();
- SuccessOr<MediaPlaybackDenialReason> playbackPermitted() const;
+ // FIXME: <http://webkit.org/b/220939>
+ SuccessOr<MediaPlaybackDenialReason> playbackPermitted(MediaPlaybackOperation = MediaPlaybackOperation::All) const;
bool autoplayPermitted() const;
bool dataLoadingPermitted() const;
MediaPlayer::BufferingPolicy preferredBufferingPolicy() const;
Modified: branches/safari-611-branch/Source/WebCore/page/Quirks.cpp (272320 => 272321)
--- branches/safari-611-branch/Source/WebCore/page/Quirks.cpp 2021-02-03 17:33:36 UTC (rev 272320)
+++ branches/safari-611-branch/Source/WebCore/page/Quirks.cpp 2021-02-03 17:36:30 UTC (rev 272321)
@@ -1307,16 +1307,42 @@
bool Quirks::requiresUserGestureToPauseInPictureInPicture() const
{
- // Facebook will naively pause a <video> element that has scrolled out of the viewport, regardless of whether that element is currently in PiP mode.
+#if ENABLE(VIDEO_PRESENTATION_MODE)
+ // Facebook and Twitter will naively pause a <video> element that has scrolled out of the viewport,
+ // regardless of whether that element is currently in PiP mode.
+ // We should remove the quirk once <rdar://problem/67273166> and <rdar://problem/73369869> have been fixed.
if (!needsQuirks())
return false;
if (!m_requiresUserGestureToPauseInPictureInPicture) {
- auto domain = RegistrableDomain(m_document->topDocument().url());
- m_requiresUserGestureToPauseInPictureInPicture = domain.string() == "facebook.com"_s;
+ auto domain = RegistrableDomain(m_document->topDocument().url()).string();
+ m_requiresUserGestureToPauseInPictureInPicture = domain == "facebook.com"_s || domain == "twitter.com"_s;
}
return *m_requiresUserGestureToPauseInPictureInPicture;
+#else
+ return false;
+#endif
}
+bool Quirks::requiresUserGestureToLoadInPictureInPicture() const
+{
+#if ENABLE(VIDEO_PRESENTATION_MODE)
+ // Twitter will remove the "src" attribute of a <video> element that has scrolled out of the viewport and
+ // load the <video> element with an empty "src" regardless of whether that element is currently in PiP mode.
+ // We should remove the quirk once <rdar://problem/73369869> has been fixed.
+ if (!needsQuirks())
+ return false;
+
+ if (!m_requiresUserGestureToLoadInPictureInPicture) {
+ auto domain = RegistrableDomain(m_document->topDocument().url());
+ m_requiresUserGestureToLoadInPictureInPicture = domain.string() == "twitter.com"_s;
+ }
+
+ return *m_requiresUserGestureToLoadInPictureInPicture;
+#else
+ return false;
+#endif
}
+
+}
Modified: branches/safari-611-branch/Source/WebCore/page/Quirks.h (272320 => 272321)
--- branches/safari-611-branch/Source/WebCore/page/Quirks.h 2021-02-03 17:33:36 UTC (rev 272320)
+++ branches/safari-611-branch/Source/WebCore/page/Quirks.h 2021-02-03 17:36:30 UTC (rev 272321)
@@ -128,6 +128,7 @@
WEBCORE_EXPORT bool blocksReturnToFullscreenFromPictureInPictureQuirk() const;
bool requiresUserGestureToPauseInPictureInPicture() const;
+ bool requiresUserGestureToLoadInPictureInPicture() const;
#if ENABLE(RESOURCE_LOAD_STATISTICS)
static bool isMicrosoftTeamsRedirectURL(const URL&);
@@ -170,6 +171,7 @@
mutable Optional<bool> m_needsHDRPixelDepthQuirk;
mutable Optional<bool> m_needsBlackFullscreenBackgroundQuirk;
mutable Optional<bool> m_requiresUserGestureToPauseInPictureInPicture;
+ mutable Optional<bool> m_requiresUserGestureToLoadInPictureInPicture;
mutable Optional<bool> m_shouldDisableEndFullscreenEventWhenEnteringPictureInPictureFromFullscreenQuirk;
mutable Optional<bool> m_blocksReturnToFullscreenFromPictureInPictureQuirk;
};
