Attachment #406871 for bug #215594

View Details Raw Unified Return to Bug 215594

| Differences between



2020-08-19  Peng Liu  <peng.liu6@apple.com>

        A PiP window is closed when the video element is removed from DOM
        https://bugs.webkit.org/show_bug.cgi?id=215594

        Reviewed by Eric Carlson.

        Test: media/remove-video-element-in-pip-from-document.html

        * html/HTMLMediaElement.cpp:
        (WebCore::HTMLMediaElement::pauseAfterDetachedTask):
        When a video is playing in the picture-in-picture mode, do not pause it or exit the
        picture-in-picture mode when detaching the video element from the DOM.
        (WebCore::HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated):
        Always return true when the video is playing in the picture-in-picture mode.
        * rendering/RenderVideo.cpp:
        (WebCore::RenderVideo::willBeDestroyed):
        Don't hide the player when destroying the renderer of an inline video.

2020-08-17  David Kilzer  <ddkilzer@apple.com>

        Clean up DragApplicationFlags after switch to OptionSet<>



    if (m_inActiveDocument)
        return;

    if (m_videoFullscreenMode != VideoFullscreenModePictureInPicture && m_networkState > NETWORK_EMPTY)
        pause();
    if (m_videoFullscreenMode == VideoFullscreenModeStandard)
        exitFullscreen();

    if (!m_player)


bool HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated()
{
    // This function must return "true" when the video is playing in the
    // picture-in-picture window. Otherwise, the MediaPlayerPrivate* may
    // destroy the video layer.
    if (m_videoFullscreenMode == VideoFullscreenModePictureInPicture)
        return true;

    auto* renderer = this->renderer();
    return is<RenderVideo>(renderer)
        && downcast<RenderVideo>(*renderer).view().compositor().canAccelerateVideoRendering(downcast<RenderVideo>(*renderer));

Lines 65-72 RenderVideo::~RenderVideo() a/Source/WebCore/rendering/RenderVideo.cpp_sec1
- a/Source/WebCore/rendering/RenderVideo.cpp +7 lines
65	void RenderVideo::willBeDestroyed()	65	void RenderVideo::willBeDestroyed()
66	{	66	{
67	visibleInViewportStateChanged();	67	visibleInViewportStateChanged();
		68
		69	#if ENABLE(VIDEO_PRESENTATION_MODE)
		70	auto player = videoElement().player();
		71	if (player && videoElement().webkitPresentationMode() != HTMLVideoElement::VideoPresentationMode::PictureInPicture)
		72	player->setVisible(false);
		73	#else
68	if (auto player = videoElement().player())	74	if (auto player = videoElement().player())
69	player->setVisible(false);	75	player->setVisible(false);
		76	#endif
70		77
71	RenderMedia::willBeDestroyed();	78	RenderMedia::willBeDestroyed();
72	}	79	}



2020-08-19  Peng Liu  <peng.liu6@apple.com>

        A PiP window is closed when the video element is removed from DOM
        https://bugs.webkit.org/show_bug.cgi?id=215594

        Reviewed by Eric Carlson.

        * TestExpectations:
        * media/remove-video-element-in-pip-from-document-expected.txt: Added.
        * media/remove-video-element-in-pip-from-document.html: Added.
        * platform/ipad/TestExpectations:
        * platform/mac-wk2/TestExpectations:

2020-08-17  Hector Lopez  <hector_i_lopez@apple.com>

        [ macOS ] media/modern-media-controls/volume-up-support/volume-up-support.html is a flaky timeout

Lines 227-232 webkit.org/b/202617 imported/w3c/web-platform-tests/picture-in-picture [ Skip ] a/LayoutTests/TestExpectations_sec1
- a/LayoutTests/TestExpectations +2 lines
227	# PiP API tests are only relevant on mac and iPad	227	# PiP API tests are only relevant on mac and iPad
228	media/picture-in-picture [ Skip ]	228	media/picture-in-picture [ Skip ]
229		229
		230	media/remove-video-element-in-pip-from-document.html [ Skip ]
		231
230	# Shared Workers are unsupported	232	# Shared Workers are unsupported
231	imported/w3c/web-platform-tests/secure-contexts/basic-shared-worker.html [ Skip ]	233	imported/w3c/web-platform-tests/secure-contexts/basic-shared-worker.html [ Skip ]
232	imported/w3c/web-platform-tests/secure-contexts/basic-shared-worker.https.html [ Skip ]	234	imported/w3c/web-platform-tests/secure-contexts/basic-shared-worker.https.html [ Skip ]



Test that removing a video element from the DOM while the video is in the picture-in-picture mode does not close the PiP window.

RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
RUN(internals.setMockVideoPresentationModeEnabled(true))
EVENT(canplaythrough)
EVENT(enterpictureinpicture)
EXPECTED (pipWindow.width > '0') OK
EXPECTED (pipWindow.height > '0') OK
The video element has been removed from DOM.
END OF TEST




<!DOCTYPE html>
<html>
<head>
    <script src="video-test.js"></script>
    <script src="media-file.js"></script>
    <script>
        window.addEventListener('load', async event => {
            if (!window.internals) {
                failTest('This test requires window.internals.');
                return;
            }

            consoleWrite('');
            run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
            run('internals.setMockVideoPresentationModeEnabled(true)');

            findMediaElement();
            video.src = findMediaFile("video", "content/test");

            await waitFor(video, 'canplaythrough');

            runWithKeyDown(() => {
                video.webkitSetPresentationMode('picture-in-picture');
            });

            waitForEventOnce('enterpictureinpicture', event => {
                window.pipWindow = event.pictureInPictureWindow;
                testExpected('pipWindow.width', 0, '>');
                testExpected('pipWindow.height', 0, '>');

                const observer = new MutationObserver(() => {
                    consoleWrite('The video element has been removed from DOM.');
                    setTimeout(endTest, 1000);
                });
                observer.observe(document.body, { childList: true });
                waitForEvent('webkitpresentationmodechanged', () => {
                    failTest('PiP window is closed!');
                });
                document.body.removeChild(video);
            });
        });
    </script>
</head>
<body>
    <div>Test that removing a video element from the DOM while the video is in the picture-in-picture mode does not close the PiP window.</div>
    <video controls playsinline></video>
</body>
</html>

Lines 66-68 webkit.org/b/203264 editing/pasteboard/smart-paste-paragraph-002.html [ Pass ] a/LayoutTests/platform/ipad/TestExpectations_sec1
- a/LayoutTests/platform/ipad/TestExpectations +1 lines
66	webkit.org/b/203264 editing/pasteboard/smart-paste-paragraph-004.html [ Pass ]	66	webkit.org/b/203264 editing/pasteboard/smart-paste-paragraph-004.html [ Pass ]
67		67
68	media/picture-in-picture [ Pass ]	68	media/picture-in-picture [ Pass ]
		69	media/remove-video-element-in-pip-from-document.html [ Pass ]

Lines 513-518 media/fullscreen-api-enabled-media-with-presentation-mode.html [ Pass ] a/LayoutTests/platform/mac-wk2/TestExpectations_sec1
- a/LayoutTests/platform/mac-wk2/TestExpectations +2 lines
513	webkit.org/b/173199 media/navigate-with-pip-should-not-crash.html [ Pass Failure ]	513	webkit.org/b/173199 media/navigate-with-pip-should-not-crash.html [ Pass Failure ]
514	media/pip-video-going-into-fullscreen.html [ Pass ]	514	media/pip-video-going-into-fullscreen.html [ Pass ]
515	media/picture-in-picture [ Pass ]	515	media/picture-in-picture [ Pass ]
		516	media/remove-video-element-in-pip-from-document.html [ Pass ]
		517
516		518
517	# RTL Scrollbars are enabled on Sierra WebKit2.	519	# RTL Scrollbars are enabled on Sierra WebKit2.
518	webkit.org/b/179455 fast/scrolling/rtl-scrollbars.html [ Pass ImageOnlyFailure ]	520	webkit.org/b/179455 fast/scrolling/rtl-scrollbars.html [ Pass ImageOnlyFailure ]

Return to Bug 215594

Lines 835-843 void HTMLMediaElement::pauseAfterDetachedTask() a/Source/WebCore/html/HTMLMediaElement.cpp_sec1
- a/Source/WebCore/html/HTMLMediaElement.cpp -2 / +8 lines
835	if (m_inActiveDocument)	835	if (m_inActiveDocument)
836	return;	836	return;
837		837
838	if (m_networkState > NETWORK_EMPTY)	838	if (m_videoFullscreenMode != VideoFullscreenModePictureInPicture && m_networkState > NETWORK_EMPTY)
839	pause();	839	pause();
840	if (m_videoFullscreenMode != VideoFullscreenModeNone)	840	if (m_videoFullscreenMode == VideoFullscreenModeStandard)
841	exitFullscreen();	841	exitFullscreen();
842		842
843	if (!m_player)	843	if (!m_player)
Lines 4926-4931 void HTMLMediaElement::mediaPlayerSizeChanged() a/Source/WebCore/html/HTMLMediaElement.cpp_sec2
4926		4926
4927	bool HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated()	4927	bool HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated()
4928	{	4928	{
		4929	// This function must return "true" when the video is playing in the
		4930	// picture-in-picture window. Otherwise, the MediaPlayerPrivate* may
		4931	// destroy the video layer.
		4932	if (m_videoFullscreenMode == VideoFullscreenModePictureInPicture)
		4933	return true;
		4934
4929	auto* renderer = this->renderer();	4935	auto* renderer = this->renderer();
4930	return is<RenderVideo>(renderer)	4936	return is<RenderVideo>(renderer)
4931	&& downcast<RenderVideo>(renderer).view().compositor().canAccelerateVideoRendering(downcast<RenderVideo>(renderer));	4937	&& downcast<RenderVideo>(renderer).view().compositor().canAccelerateVideoRendering(downcast<RenderVideo>(renderer));

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +19 lines
		1	2020-08-19 Peng Liu <peng.liu6@apple.com>
		2
		3	A PiP window is closed when the video element is removed from DOM
		4	https://bugs.webkit.org/show_bug.cgi?id=215594
		5
		6	Reviewed by Eric Carlson.
		7
		8	Test: media/remove-video-element-in-pip-from-document.html
		9
		10	* html/HTMLMediaElement.cpp:
		11	(WebCore::HTMLMediaElement::pauseAfterDetachedTask):
		12	When a video is playing in the picture-in-picture mode, do not pause it or exit the
		13	picture-in-picture mode when detaching the video element from the DOM.
		14	(WebCore::HTMLMediaElement::mediaPlayerRenderingCanBeAccelerated):
		15	Always return true when the video is playing in the picture-in-picture mode.
		16	* rendering/RenderVideo.cpp:
		17	(WebCore::RenderVideo::willBeDestroyed):
		18	Don't hide the player when destroying the renderer of an inline video.
		19
1	2020-08-17 David Kilzer <ddkilzer@apple.com>	20	2020-08-17 David Kilzer <ddkilzer@apple.com>
2		21
3	Clean up DragApplicationFlags after switch to OptionSet<>	22	Clean up DragApplicationFlags after switch to OptionSet<>

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +13 lines
		1	2020-08-19 Peng Liu <peng.liu6@apple.com>
		2
		3	A PiP window is closed when the video element is removed from DOM
		4	https://bugs.webkit.org/show_bug.cgi?id=215594
		5
		6	Reviewed by Eric Carlson.
		7
		8	* TestExpectations:
		9	* media/remove-video-element-in-pip-from-document-expected.txt: Added.
		10	* media/remove-video-element-in-pip-from-document.html: Added.
		11	* platform/ipad/TestExpectations:
		12	* platform/mac-wk2/TestExpectations:
		13
1	2020-08-17 Hector Lopez <hector_i_lopez@apple.com>	14	2020-08-17 Hector Lopez <hector_i_lopez@apple.com>
2		15
3	[ macOS ] media/modern-media-controls/volume-up-support/volume-up-support.html is a flaky timeout	16	[ macOS ] media/modern-media-controls/volume-up-support/volume-up-support.html is a flaky timeout

Line 0 a/LayoutTests/media/remove-video-element-in-pip-from-document-expected.txt_sec1
- a/LayoutTests/media/remove-video-element-in-pip-from-document-expected.txt +11 lines
	1	Test that removing a video element from the DOM while the video is in the picture-in-picture mode does not close the PiP window.
	2
	3	RUN(internals.settings.setAllowsPictureInPictureMediaPlayback(true))
	4	RUN(internals.setMockVideoPresentationModeEnabled(true))
	5	EVENT(canplaythrough)
	6	EVENT(enterpictureinpicture)
	7	EXPECTED (pipWindow.width > '0') OK
	8	EXPECTED (pipWindow.height > '0') OK
	9	The video element has been removed from DOM.
	10	END OF TEST
	11

Line 0 a/LayoutTests/media/remove-video-element-in-pip-from-document.html_sec1
- a/LayoutTests/media/remove-video-element-in-pip-from-document.html +48 lines
	1	<!DOCTYPE html>
	2	<html>
	3	<head>
	4	<script src="video-test.js"></script>
	5	<script src="media-file.js"></script>
	6	<script>
	7	window.addEventListener('load', async event => {
	8	if (!window.internals) {
	9	failTest('This test requires window.internals.');
	10	return;
	11	}
	12
	13	consoleWrite('');
	14	run('internals.settings.setAllowsPictureInPictureMediaPlayback(true)');
	15	run('internals.setMockVideoPresentationModeEnabled(true)');
	16
	17	findMediaElement();
	18	video.src = findMediaFile("video", "content/test");
	19
	20	await waitFor(video, 'canplaythrough');
	21
	22	runWithKeyDown(() => {
	23	video.webkitSetPresentationMode('picture-in-picture');
	24	});
	25
	26	waitForEventOnce('enterpictureinpicture', event => {
	27	window.pipWindow = event.pictureInPictureWindow;
	28	testExpected('pipWindow.width', 0, '>');
	29	testExpected('pipWindow.height', 0, '>');
	30
	31	const observer = new MutationObserver(() => {
	32	consoleWrite('The video element has been removed from DOM.');
	33	setTimeout(endTest, 1000);
	34	});
	35	observer.observe(document.body, { childList: true });
	36	waitForEvent('webkitpresentationmodechanged', () => {
	37	failTest('PiP window is closed!');
	38	});
	39	document.body.removeChild(video);
	40	});
	41	});
	42	</script>
	43	</head>
	44	<body>
	45	<div>Test that removing a video element from the DOM while the video is in the picture-in-picture mode does not close the PiP window.</div>
	46	<video controls playsinline></video>
	47	</body>
	48	</html>