[Perf] HTMLVideoElement is performing synchronous paints; causing mai…

…n thread hangs

https://bugs.webkit.org/show_bug.cgi?id=238707
<rdar://91025299>

Reviewed by Eric Carlson.

Spin trace diagnostics show that the main thread of the WebContent process is often hung,
blocked on a synchronous Paint message to the GPU process; in turn, the GPU process is often
busy performing media-related work, but in each of the cases found, painting is unnecessary.
The media player in question is accelerated, and should only be painted during layer snapshotting
or during a print operation.

Only paint if the renderer is not accelerated, the media element is not accelerated, or if
the paint operation isn't flattening or snapshotting. HTMLMediaElement inappropriately caches
the value of MediaPlayer::supportsAcceleratedRendering(), under the (incorrect) assumption that
the value cannot change during the lifetime of the MediaPlayer, so remove this caching layer.

* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::mediaEngineWasUpdated):
(WebCore::HTMLMediaElement::clearMediaPlayer):
* html/HTMLMediaElement.h:
(WebCore::HTMLMediaElement::supportsAcceleratedRendering const):
* rendering/RenderVideo.cpp:
(WebCore::RenderVideo::paintReplaced):



Canonical link: https://commits.webkit.org/249188@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@292290 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog

@@ -1,3 +1,30 @@
+2022-04-04  Jer Noble  <jer.noble@apple.com>
+
+        [Perf] HTMLVideoElement is performing synchronous paints; causing main thread hangs
+        https://bugs.webkit.org/show_bug.cgi?id=238707
+        <rdar://91025299>
+
+        Reviewed by Eric Carlson.
+
+        Spin trace diagnostics show that the main thread of the WebContent process is often hung,
+        blocked on a synchronous Paint message to the GPU process; in turn, the GPU process is often
+        busy performing media-related work, but in each of the cases found, painting is unnecessary.
+        The media player in question is accelerated, and should only be painted during layer snapshotting
+        or during a print operation.
+
+        Only paint if the renderer is not accelerated, the media element is not accelerated, or if
+        the paint operation isn't flattening or snapshotting. HTMLMediaElement inappropriately caches
+        the value of MediaPlayer::supportsAcceleratedRendering(), under the (incorrect) assumption that
+        the value cannot change during the lifetime of the MediaPlayer, so remove this caching layer.
+
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::mediaEngineWasUpdated):
+        (WebCore::HTMLMediaElement::clearMediaPlayer):
+        * html/HTMLMediaElement.h:
+        (WebCore::HTMLMediaElement::supportsAcceleratedRendering const):
+        * rendering/RenderVideo.cpp:
+        (WebCore::RenderVideo::paintReplaced):
+
 2022-03-28  Antoine Quint  <graouts@webkit.org>
 
         Make it hard to add a new CSS property to WebKit wihtout adding animation support

Source/WebCore/html/HTMLMediaElement.cpp

@@ -5340,7 +5340,6 @@ void HTMLMediaElement::mediaEngineWasUpdated()
     ALWAYS_LOG(LOGIDENTIFIER);
 
     beginProcessingMediaPlayerCallback();
-    m_cachedSupportsAcceleratedRendering = m_player && m_player->supportsAcceleratedRendering();
     updateRenderer();
     endProcessingMediaPlayerCallback();
 
@@ -5899,7 +5898,6 @@ void HTMLMediaElement::clearMediaPlayer()
     if (m_player) {
         m_player->invalidate();
         m_player = nullptr;
-        m_cachedSupportsAcceleratedRendering = false;
     }
     schedulePlaybackControlsManagerUpdate();
 

Source/WebCore/html/HTMLMediaElement.h

@@ -157,7 +157,7 @@ class HTMLMediaElement
     RefPtr<MediaPlayer> player() const { return m_player; }
     WEBCORE_EXPORT std::optional<MediaPlayerIdentifier> playerIdentifier() const;
 
-    bool supportsAcceleratedRendering() const { return m_cachedSupportsAcceleratedRendering; }
+    bool supportsAcceleratedRendering() const { return m_player && m_player->supportsAcceleratedRendering(); }
 
     virtual bool isVideo() const { return false; }
     bool hasVideo() const override { return false; }
@@ -1068,7 +1068,6 @@ class HTMLMediaElement
 #endif
 
     RefPtr<MediaPlayer> m_player;
-    bool m_cachedSupportsAcceleratedRendering { false };
 
     MediaPlayer::Preload m_preload { Preload::Auto };
 

Source/WebCore/rendering/RenderVideo.cpp

@@ -230,14 +230,29 @@ void RenderVideo::paintReplaced(PaintInfo& paintInfo, const LayoutPoint& paintOf
     if (clip)
         context.clip(contentRect);
 
-    if (displayingPoster)
+    if (displayingPoster) {
         paintIntoRect(paintInfo, rect);
-    else if (!videoElement().isFullscreen() || !videoElement().supportsAcceleratedRendering()) {
-        if (paintInfo.paintBehavior.contains(PaintBehavior::FlattenCompositingLayers))
-            context.paintFrameForMedia(*mediaPlayer, rect);
-        else
-            mediaPlayer->paint(context, rect);
+        return;
     }
+
+    if (!mediaPlayer)
+        return;
+
+    // Painting contents during fullscreen playback causes stutters on iOS when the device is rotated.
+    // https://bugs.webkit.org/show_bug.cgi?id=142097
+    if (videoElement().supportsAcceleratedRendering() && videoElement().isFullscreen())
+        return;
+
+    // Avoid unnecessary paints by skipping software painting if
+    // the renderer is accelerated, and the paint operation does
+    // not flatten compositing layers and is not snapshotting.
+    if (hasAcceleratedCompositing()
+        && videoElement().supportsAcceleratedRendering()
+        && !paintInfo.paintBehavior.contains(PaintBehavior::FlattenCompositingLayers)
+        && !paintInfo.paintBehavior.contains(PaintBehavior::Snapshotting))
+        return;
+
+    context.paintFrameForMedia(*mediaPlayer, rect);
 }
 
 void RenderVideo::layout()