[webkit-reviews] review granted: [Bug 220660] PiP video subtitles stop updating when Safari is backgrounded : [Attachment 417714] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Peng Liu <peng.liu6 at apple.com>'s
request for review:
Bug 220660: PiP video subtitles stop updating when Safari is backgrounded
https://bugs.webkit.org/show_bug.cgi?id=220660

Attachment 417714: Patch

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




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 417714
  --> https://bugs.webkit.org/attachment.cgi?id=417714
Patch

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

>
Source/WebKit/WebProcess/WebPage/RemoteLayerTree/RemoteLayerTreeDrawingArea.mm:
219
> +    return m_isRenderingSuspended &&
!m_webPage.videoFullscreenManager().hasVideo();

This code is a little bit mysterious, not obvious. Hard to understand this is
about PIP and why it’s correct. There’s not even a why comment!

And worse, we have no regression test for this, so someone could break it and
commit the change without ever knowing it.

That’s bad combination: seems like we may be unable to keep this working and
might make the same kind of mistake again in the future.