[webkit-reviews] review granted: [Bug 215594] A PiP window is closed when the video element is removed from DOM : [Attachment 406859] fix build failures again

[bugzilla-daemon at webkit.org]

Eric Carlson <eric.carlson at apple.com> has granted Peng Liu
<peng.liu6 at apple.com>'s request for review:
Bug 215594: A PiP window is closed when the video element is removed from DOM
https://bugs.webkit.org/show_bug.cgi?id=215594

Attachment 406859: fix build failures again

https://bugs.webkit.org/attachment.cgi?id=406859&action=review




--- Comment #5 from Eric Carlson <eric.carlson at apple.com> ---
Comment on attachment 406859
  --> https://bugs.webkit.org/attachment.cgi?id=406859
fix build failures again

View in context: https://bugs.webkit.org/attachment.cgi?id=406859&action=review

> Source/WebCore/html/HTMLMediaElement.cpp:4931
> +    if (m_videoFullscreenMode == VideoFullscreenModePictureInPicture)
> +	   return true;
> +

It took me a minute to figure out why this is necessary, so I think it would be
useful to have a short comment here.

> LayoutTests/media/remove-video-element-in-pip-from-document.html:31
> +		   setTimeout(endTest, 4000);

We shouldn't use a timeout to end tests if at all possible. Four seconds is a
very long time to wait for most test runs, but will probably be too short on a
very heavily loaded bot. 

Luckily, I think you can do what you need with a MutationObserver instead.