Title: [258434] trunk/Source/WebCore
Revision
258434
Author
peng.l...@apple.com
Date
2020-03-13 14:49:31 -0700 (Fri, 13 Mar 2020)

Log Message

Safari sometimes crashes when switch video into PiP mode
https://bugs.webkit.org/show_bug.cgi?id=208904

Reviewed by Simon Fraser.

With this patch, MediaControlTextTrackContainerElement won't paint its subtree
to an image buffer (for the captions in video fullscreen or picture-in-picture mode)
when the cues are updated. Instead, it only sets the flag m_needsGenerateTextTrackRepresentation
to true after running layout based on the new cues. After that, it paints its subtree
to an image buffer if needed at the end of Page::updateRendering() when the layout is clean.
TextTrackRepresentationCocoa will use the image buffer to set the content of the layer
for captions in video fullscreen or picture-in-picture mode.

MediaControlTextTrackContainerElement class is responsible for rendering the captions in both:
1) a video player in the inline mode.
2) a video player in "video fullscreen" or picture-in-picture mode.
This patch refactors some functions to make their responsibilities clear.

* Modules/mediacontrols/MediaControlsHost.cpp:
(WebCore::MediaControlsHost::updateTextTrackRepresentationImageIfNeeded):
* Modules/mediacontrols/MediaControlsHost.h:
* dom/Document.cpp:
(WebCore::Document::setMediaElementShowingTextTrack):
(WebCore::Document::clearMediaElementShowingTextTrack):
(WebCore::Document::updateTextTrackRepresentationImageIfNeeded):
* dom/Document.h:

Functions textTracksAreReady(), textTrackReadyStateChanged() and configureTextTrackDisplay()
should be wrapped with "#if ENABLE(VIDEO_TRACK)".
* html/HTMLMediaElement.cpp:
(WebCore::HTMLMediaElement::setTextTrackRepresentation):
(WebCore::HTMLMediaElement::textTracksAreReady const):
(WebCore::HTMLMediaElement::textTrackReadyStateChanged):
(WebCore::HTMLMediaElement::configureTextTrackDisplay):
(WebCore::HTMLMediaElement::updateTextTrackRepresentationImageIfNeeded):
* html/HTMLMediaElement.h:

* html/shadow/MediaControlElements.cpp:
(WebCore::MediaControlTextTrackContainerElement::createElementRenderer):
(WebCore::MediaControlTextTrackContainerElement::updateDisplay):
(WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentationImageIfNeeded):
(WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentationIfNeeded):
(WebCore::MediaControlTextTrackContainerElement::clearTextTrackRepresentation):
(WebCore::MediaControlTextTrackContainerElement::updateTextTrackStyle):
(WebCore::MediaControlTextTrackContainerElement::enteredFullscreen):
(WebCore::MediaControlTextTrackContainerElement::updateVideoDisplaySize):
(WebCore::MediaControlTextTrackContainerElement::updateSizes):
(WebCore::MediaControlTextTrackContainerElement::createTextTrackRepresentationImage):
(WebCore::MediaControlTextTrackContainerElement::textTrackRepresentationBoundsChanged):
(WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentation): Deleted.
(WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentationStyle): Deleted.
(WebCore::MediaControlTextTrackContainerElement::layoutIfNecessary): Deleted.
(WebCore::MediaControlTextTrackContainerElement::updateCueStyles): Deleted.
* html/shadow/MediaControlElements.h:
* html/shadow/MediaControls.cpp:
(WebCore::MediaControls::updateTextTrackRepresentationImageIfNeeded):
* html/shadow/MediaControls.h:
* page/Page.cpp:
(WebCore::Page::updateRendering):
* rendering/RenderMediaControlElements.cpp:
(WebCore::RenderMediaControlTextTrackContainer::RenderMediaControlTextTrackContainer):
(WebCore::RenderMediaControlTextTrackContainer::layout):
(WebCore::RenderTextTrackContainerElement::RenderTextTrackContainerElement): Deleted.
(WebCore::RenderTextTrackContainerElement::layout): Deleted.
* rendering/RenderMediaControlElements.h:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (258433 => 258434)


--- trunk/Source/WebCore/ChangeLog	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/ChangeLog	2020-03-13 21:49:31 UTC (rev 258434)
@@ -1,3 +1,71 @@
+2020-03-13  Peng Liu  <peng.l...@apple.com>
+
+        Safari sometimes crashes when switch video into PiP mode
+        https://bugs.webkit.org/show_bug.cgi?id=208904
+
+        Reviewed by Simon Fraser.
+
+        With this patch, MediaControlTextTrackContainerElement won't paint its subtree
+        to an image buffer (for the captions in video fullscreen or picture-in-picture mode)
+        when the cues are updated. Instead, it only sets the flag m_needsGenerateTextTrackRepresentation
+        to true after running layout based on the new cues. After that, it paints its subtree
+        to an image buffer if needed at the end of Page::updateRendering() when the layout is clean.
+        TextTrackRepresentationCocoa will use the image buffer to set the content of the layer
+        for captions in video fullscreen or picture-in-picture mode.
+
+        MediaControlTextTrackContainerElement class is responsible for rendering the captions in both:
+        1) a video player in the inline mode.
+        2) a video player in "video fullscreen" or picture-in-picture mode.
+        This patch refactors some functions to make their responsibilities clear.
+
+        * Modules/mediacontrols/MediaControlsHost.cpp:
+        (WebCore::MediaControlsHost::updateTextTrackRepresentationImageIfNeeded):
+        * Modules/mediacontrols/MediaControlsHost.h:
+        * dom/Document.cpp:
+        (WebCore::Document::setMediaElementShowingTextTrack):
+        (WebCore::Document::clearMediaElementShowingTextTrack):
+        (WebCore::Document::updateTextTrackRepresentationImageIfNeeded):
+        * dom/Document.h:
+
+        Functions textTracksAreReady(), textTrackReadyStateChanged() and configureTextTrackDisplay()
+        should be wrapped with "#if ENABLE(VIDEO_TRACK)".
+        * html/HTMLMediaElement.cpp:
+        (WebCore::HTMLMediaElement::setTextTrackRepresentation):
+        (WebCore::HTMLMediaElement::textTracksAreReady const):
+        (WebCore::HTMLMediaElement::textTrackReadyStateChanged):
+        (WebCore::HTMLMediaElement::configureTextTrackDisplay):
+        (WebCore::HTMLMediaElement::updateTextTrackRepresentationImageIfNeeded):
+        * html/HTMLMediaElement.h:
+
+        * html/shadow/MediaControlElements.cpp:
+        (WebCore::MediaControlTextTrackContainerElement::createElementRenderer):
+        (WebCore::MediaControlTextTrackContainerElement::updateDisplay):
+        (WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentationImageIfNeeded):
+        (WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentationIfNeeded):
+        (WebCore::MediaControlTextTrackContainerElement::clearTextTrackRepresentation):
+        (WebCore::MediaControlTextTrackContainerElement::updateTextTrackStyle):
+        (WebCore::MediaControlTextTrackContainerElement::enteredFullscreen):
+        (WebCore::MediaControlTextTrackContainerElement::updateVideoDisplaySize):
+        (WebCore::MediaControlTextTrackContainerElement::updateSizes):
+        (WebCore::MediaControlTextTrackContainerElement::createTextTrackRepresentationImage):
+        (WebCore::MediaControlTextTrackContainerElement::textTrackRepresentationBoundsChanged):
+        (WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentation): Deleted.
+        (WebCore::MediaControlTextTrackContainerElement::updateTextTrackRepresentationStyle): Deleted.
+        (WebCore::MediaControlTextTrackContainerElement::layoutIfNecessary): Deleted.
+        (WebCore::MediaControlTextTrackContainerElement::updateCueStyles): Deleted.
+        * html/shadow/MediaControlElements.h:
+        * html/shadow/MediaControls.cpp:
+        (WebCore::MediaControls::updateTextTrackRepresentationImageIfNeeded):
+        * html/shadow/MediaControls.h:
+        * page/Page.cpp:
+        (WebCore::Page::updateRendering):
+        * rendering/RenderMediaControlElements.cpp:
+        (WebCore::RenderMediaControlTextTrackContainer::RenderMediaControlTextTrackContainer):
+        (WebCore::RenderMediaControlTextTrackContainer::layout):
+        (WebCore::RenderTextTrackContainerElement::RenderTextTrackContainerElement): Deleted.
+        (WebCore::RenderTextTrackContainerElement::layout): Deleted.
+        * rendering/RenderMediaControlElements.h:
+
 2020-03-13  Ryan Haddad  <ryanhad...@apple.com>
 
         Unreviewed, reverting r258391.

Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp (258433 => 258434)


--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.cpp	2020-03-13 21:49:31 UTC (rev 258434)
@@ -172,6 +172,12 @@
         m_textTrackContainer->updateDisplay();
 }
 
+void MediaControlsHost::updateTextTrackRepresentationImageIfNeeded()
+{
+    if (m_textTrackContainer)
+        m_textTrackContainer->updateTextTrackRepresentationImageIfNeeded();
+}
+
 void MediaControlsHost::enteredFullscreen()
 {
     if (m_textTrackContainer)

Modified: trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h (258433 => 258434)


--- trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/Modules/mediacontrols/MediaControlsHost.h	2020-03-13 21:49:31 UTC (rev 258434)
@@ -73,6 +73,7 @@
 
     enum class ForceUpdate { Yes, No };
     void updateCaptionDisplaySizes(ForceUpdate = ForceUpdate::No);
+    void updateTextTrackRepresentationImageIfNeeded();
     void enteredFullscreen();
     void exitedFullscreen();
 

Modified: trunk/Source/WebCore/dom/Document.cpp (258433 => 258434)


--- trunk/Source/WebCore/dom/Document.cpp	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/dom/Document.cpp	2020-03-13 21:49:31 UTC (rev 258434)
@@ -5451,6 +5451,22 @@
         element->captionPreferencesChanged();
 }
 
+void Document::setMediaElementShowingTextTrack(const HTMLMediaElement& element)
+{
+    m_mediaElementShowingTextTrack = makeWeakPtr(element);
+}
+
+void Document::clearMediaElementShowingTextTrack()
+{
+    m_mediaElementShowingTextTrack = nullptr;
+}
+
+void Document::updateTextTrackRepresentationImageIfNeeded()
+{
+    if (m_mediaElementShowingTextTrack)
+        m_mediaElementShowingTextTrack->updateTextTrackRepresentationImageIfNeeded();
+}
+
 #endif
 
 void Document::setShouldCreateRenderers(bool f)

Modified: trunk/Source/WebCore/dom/Document.h (258433 => 258434)


--- trunk/Source/WebCore/dom/Document.h	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/dom/Document.h	2020-03-13 21:49:31 UTC (rev 258434)
@@ -1115,6 +1115,9 @@
     void registerForCaptionPreferencesChangedCallbacks(HTMLMediaElement&);
     void unregisterForCaptionPreferencesChangedCallbacks(HTMLMediaElement&);
     void captionPreferencesChanged();
+    void setMediaElementShowingTextTrack(const HTMLMediaElement&);
+    void clearMediaElementShowingTextTrack();
+    void updateTextTrackRepresentationImageIfNeeded();
 #endif
 
     void registerForVisibilityStateChangedCallbacks(VisibilityChangeClient&);
@@ -1801,6 +1804,7 @@
 
 #if ENABLE(VIDEO_TRACK)
     HashSet<HTMLMediaElement*> m_captionPreferencesChangedElements;
+    WeakPtr<HTMLMediaElement> m_mediaElementShowingTextTrack;
 #endif
 
     Element* m_mainArticleElement { nullptr };

Modified: trunk/Source/WebCore/html/HTMLMediaElement.cpp (258433 => 258434)


--- trunk/Source/WebCore/html/HTMLMediaElement.cpp	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/html/HTMLMediaElement.cpp	2020-03-13 21:49:31 UTC (rev 258434)
@@ -1876,45 +1876,6 @@
         updateTextTrackDisplay();
 }
 
-bool HTMLMediaElement::textTracksAreReady() const
-{
-    // 4.8.10.12.1 Text track model
-    // ...
-    // The text tracks of a media element are ready if all the text tracks whose mode was not
-    // in the disabled state when the element's resource selection algorithm last started now
-    // have a text track readiness state of loaded or failed to load.
-    for (unsigned i = 0; i < m_textTracksWhenResourceSelectionBegan.size(); ++i) {
-        if (m_textTracksWhenResourceSelectionBegan[i]->readinessState() == TextTrack::Loading
-            || m_textTracksWhenResourceSelectionBegan[i]->readinessState() == TextTrack::NotLoaded)
-            return false;
-    }
-
-    return true;
-}
-
-void HTMLMediaElement::textTrackReadyStateChanged(TextTrack* track)
-{
-    if (track->readinessState() != TextTrack::Loading
-        && track->mode() != TextTrack::Mode::Disabled) {
-        // The display trees exist as long as the track is active, in this case,
-        // and if the same track is loaded again (for example if the src attribute was changed),
-        // cues can be accumulated with the old ones, that's why they needs to be flushed
-        if (hasMediaControls())
-            mediaControls()->clearTextDisplayContainer();
-        updateTextTrackDisplay();
-    }
-    if (m_player && m_textTracksWhenResourceSelectionBegan.contains(track)) {
-        if (track->readinessState() != TextTrack::Loading)
-            setReadyState(m_player->readyState());
-    } else {
-        // The track readiness state might have changed as a result of the user
-        // clicking the captions button. In this case, a check whether all the
-        // resources have failed loading should be done in order to hide the CC button.
-        if (hasMediaControls() && track->readinessState() == TextTrack::FailedToLoad)
-            mediaControls()->refreshClosedCaptionsButtonVisibility();
-    }
-}
-
 void HTMLMediaElement::audioTrackEnabledChanged(AudioTrack& track)
 {
     if (m_audioTracks && m_audioTracks->contains(track))
@@ -5851,6 +5812,11 @@
 {
     if (m_player)
         m_player->setTextTrackRepresentation(representation);
+
+    if (representation)
+        document().setMediaElementShowingTextTrack(*this);
+    else
+        document().clearMediaElementShowingTextTrack();
 }
 
 void HTMLMediaElement::syncTextTrackBounds()
@@ -6345,6 +6311,89 @@
 
 #if ENABLE(VIDEO_TRACK)
 
+bool HTMLMediaElement::textTracksAreReady() const
+{
+    // 4.8.10.12.1 Text track model
+    // ...
+    // The text tracks of a media element are ready if all the text tracks whose mode was not
+    // in the disabled state when the element's resource selection algorithm last started now
+    // have a text track readiness state of loaded or failed to load.
+    for (unsigned i = 0; i < m_textTracksWhenResourceSelectionBegan.size(); ++i) {
+        if (m_textTracksWhenResourceSelectionBegan[i]->readinessState() == TextTrack::Loading
+            || m_textTracksWhenResourceSelectionBegan[i]->readinessState() == TextTrack::NotLoaded)
+            return false;
+    }
+
+    return true;
+}
+
+void HTMLMediaElement::textTrackReadyStateChanged(TextTrack* track)
+{
+    if (track->readinessState() != TextTrack::Loading
+        && track->mode() != TextTrack::Mode::Disabled) {
+        // The display trees exist as long as the track is active, in this case,
+        // and if the same track is loaded again (for example if the src attribute was changed),
+        // cues can be accumulated with the old ones, that's why they needs to be flushed
+        if (hasMediaControls())
+            mediaControls()->clearTextDisplayContainer();
+        updateTextTrackDisplay();
+    }
+    if (m_player && m_textTracksWhenResourceSelectionBegan.contains(track)) {
+        if (track->readinessState() != TextTrack::Loading)
+            setReadyState(m_player->readyState());
+    } else {
+        // The track readiness state might have changed as a result of the user
+        // clicking the captions button. In this case, a check whether all the
+        // resources have failed loading should be done in order to hide the CC button.
+        if (hasMediaControls() && track->readinessState() == TextTrack::FailedToLoad)
+            mediaControls()->refreshClosedCaptionsButtonVisibility();
+    }
+}
+
+void HTMLMediaElement::configureTextTrackDisplay(TextTrackVisibilityCheckType checkType)
+{
+    ALWAYS_LOG(LOGIDENTIFIER, checkType);
+    ASSERT(m_textTracks);
+
+    if (m_processingPreferenceChange)
+        return;
+
+    if (document().activeDOMObjectsAreStopped())
+        return;
+
+    bool haveVisibleTextTrack = false;
+    for (unsigned i = 0; i < m_textTracks->length(); ++i) {
+        if (m_textTracks->item(i)->mode() == TextTrack::Mode::Showing) {
+            haveVisibleTextTrack = true;
+            break;
+        }
+    }
+
+    if (checkType == CheckTextTrackVisibility && m_haveVisibleTextTrack == haveVisibleTextTrack) {
+        updateActiveTextTrackCues(currentMediaTime());
+        return;
+    }
+
+    m_haveVisibleTextTrack = haveVisibleTextTrack;
+    m_closedCaptionsVisible = m_haveVisibleTextTrack;
+
+#if ENABLE(MEDIA_CONTROLS_SCRIPT)
+    if (!m_haveVisibleTextTrack)
+        return;
+
+    ensureMediaControlsShadowRoot();
+    updateTextTrackDisplay();
+#else
+    if (!m_haveVisibleTextTrack && !hasMediaControls() && !createMediaControls())
+        return;
+
+    mediaControls()->changedClosedCaptionsVisibility();
+
+    updateTextTrackDisplay();
+    updateActiveTextTrackCues(currentMediaTime());
+#endif
+}
+
 void HTMLMediaElement::updateTextTrackDisplay()
 {
 #if ENABLE(MEDIA_CONTROLS_SCRIPT)
@@ -6360,8 +6409,23 @@
 #endif
 }
 
+void HTMLMediaElement::updateTextTrackRepresentationImageIfNeeded()
+{
+#if ENABLE(MEDIA_CONTROLS_SCRIPT)
+    ensureMediaControlsShadowRoot();
+    if (!m_mediaControlsHost)
+        m_mediaControlsHost = MediaControlsHost::create(*this);
+    m_mediaControlsHost->updateTextTrackRepresentationImageIfNeeded();
+#else
+    if (!hasMediaControls() && !createMediaControls())
+        return;
+
+    mediaControls()->updateTextTrackRepresentationImageIfNeeded();
 #endif
+}
 
+#endif
+
 void HTMLMediaElement::setClosedCaptionsVisible(bool closedCaptionVisible)
 {
     INFO_LOG(LOGIDENTIFIER, closedCaptionVisible);
@@ -6600,52 +6664,7 @@
 }
 
 #if ENABLE(VIDEO_TRACK)
-void HTMLMediaElement::configureTextTrackDisplay(TextTrackVisibilityCheckType checkType)
-{
-    ALWAYS_LOG(LOGIDENTIFIER, checkType);
-    ASSERT(m_textTracks);
 
-    if (m_processingPreferenceChange)
-        return;
-
-    if (document().activeDOMObjectsAreStopped())
-        return;
-
-    bool haveVisibleTextTrack = false;
-    for (unsigned i = 0; i < m_textTracks->length(); ++i) {
-        if (m_textTracks->item(i)->mode() == TextTrack::Mode::Showing) {
-            haveVisibleTextTrack = true;
-            break;
-        }
-    }
-
-    if (checkType == CheckTextTrackVisibility && m_haveVisibleTextTrack == haveVisibleTextTrack) {
-        updateActiveTextTrackCues(currentMediaTime());
-        return;
-    }
-
-    m_haveVisibleTextTrack = haveVisibleTextTrack;
-    m_closedCaptionsVisible = m_haveVisibleTextTrack;
-
-#if ENABLE(MEDIA_CONTROLS_SCRIPT)
-    if (!m_haveVisibleTextTrack)
-        return;
-
-    ensureMediaControlsShadowRoot();
-    updateTextTrackDisplay();
-#else
-    if (!m_haveVisibleTextTrack && !hasMediaControls())
-        return;
-    if (!hasMediaControls() && !createMediaControls())
-        return;
-
-    mediaControls()->changedClosedCaptionsVisibility();
-
-    updateTextTrackDisplay();
-    updateActiveTextTrackCues(currentMediaTime());
-#endif
-}
-
 void HTMLMediaElement::captionPreferencesChanged()
 {
     if (!isVideo())

Modified: trunk/Source/WebCore/html/HTMLMediaElement.h (258433 => 258434)


--- trunk/Source/WebCore/html/HTMLMediaElement.h	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/html/HTMLMediaElement.h	2020-03-13 21:49:31 UTC (rev 258434)
@@ -379,16 +379,9 @@
 
     void setSelectedTextTrack(TextTrack*);
 
-    bool textTracksAreReady() const;
-    using HTMLMediaElementEnums::TextTrackVisibilityCheckType;
-    void configureTextTrackDisplay(TextTrackVisibilityCheckType checkType = CheckTextTrackVisibility);
-    void updateTextTrackDisplay();
-
     // AudioTrackClient
     void audioTrackEnabledChanged(AudioTrack&) final;
 
-    void textTrackReadyStateChanged(TextTrack*);
-
     // TextTrackClient
     void textTrackKindChanged(TextTrack&) override;
     void textTrackModeChanged(TextTrack&) override;
@@ -405,6 +398,9 @@
     void syncTextTrackBounds();
 
     void captionPreferencesChanged();
+    using HTMLMediaElementEnums::TextTrackVisibilityCheckType;
+    void textTrackReadyStateChanged(TextTrack*);
+    void updateTextTrackRepresentationImageIfNeeded();
 #endif
 
 #if ENABLE(WIRELESS_PLAYBACK_TARGET)
@@ -811,6 +807,10 @@
     enum ReconfigureMode { Immediately, AfterDelay };
     void markCaptionAndSubtitleTracksAsUnconfigured(ReconfigureMode);
     CaptionUserPreferences::CaptionDisplayMode captionDisplayMode();
+
+    bool textTracksAreReady() const;
+    void configureTextTrackDisplay(TextTrackVisibilityCheckType = CheckTextTrackVisibility);
+    void updateTextTrackDisplay();
 #endif
 
     // These "internal" functions do not check user gesture restrictions.

Modified: trunk/Source/WebCore/html/shadow/MediaControlElements.cpp (258433 => 258434)


--- trunk/Source/WebCore/html/shadow/MediaControlElements.cpp	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/html/shadow/MediaControlElements.cpp	2020-03-13 21:49:31 UTC (rev 258434)
@@ -1106,7 +1106,7 @@
 
 RenderPtr<RenderElement> MediaControlTextTrackContainerElement::createElementRenderer(RenderStyle&& style, const RenderTreePosition&)
 {
-    return createRenderer<RenderTextTrackContainerElement>(*this, WTFMove(style));
+    return createRenderer<RenderMediaControlTextTrackContainer>(*this, WTFMove(style));
 }
 
 static bool compareCueIntervalForDisplay(const CueInterval& one, const CueInterval& two)
@@ -1184,7 +1184,7 @@
     std::sort(activeCues.begin(), activeCues.end(), &compareCueIntervalForDisplay);
 
     if (mediaController()->closedCaptionsVisible()) {
-        // 10. For each text track cue cue in cues that has not yet had
+        // 10. For each text track cue in cues that has not yet had
         // corresponding CSS boxes added to output, in text track cue order, run the
         // following substeps:
         for (auto& interval : activeCues) {
@@ -1206,10 +1206,24 @@
     else
         hide();
 
-    if (m_textTrackRepresentation || video.requiresTextTrackRepresentation())
-        updateTextTrackRepresentation();
+    updateTextTrackRepresentationIfNeeded();
+    updateTextTrackStyle();
+    m_needsGenerateTextTrackRepresentation = true;
 }
 
+void MediaControlTextTrackContainerElement::updateTextTrackRepresentationImageIfNeeded()
+{
+    if (!m_needsGenerateTextTrackRepresentation)
+        return;
+
+    m_needsGenerateTextTrackRepresentation = false;
+
+    // We should call m_textTrackRepresentation->update() to paint the subtree of
+    // the RenderTextTrackContainerElement after the layout is clean.
+    if (m_textTrackRepresentation)
+        m_textTrackRepresentation->update();
+}
+
 void MediaControlTextTrackContainerElement::processActiveVTTCue(VTTCue& cue)
 {
     DEBUG_LOG(LOGIDENTIFIER, "adding and positioning cue: \"", cue.text(), "\", start=", cue.startTime(), ", end=", cue.endTime(), ", line=", cue.line());
@@ -1287,7 +1301,7 @@
         setInlineStyleProperty(CSSPropertyStrokeWidth, strokeWidth, CSSUnitType::CSS_PX, important);
 }
 
-void MediaControlTextTrackContainerElement::updateTextTrackRepresentation()
+void MediaControlTextTrackContainerElement::updateTextTrackRepresentationIfNeeded()
 {
     auto mediaElement = parentMediaElement(this);
     if (!mediaElement)
@@ -1296,10 +1310,9 @@
     auto requiresTextTrackRepresentation = mediaElement->requiresTextTrackRepresentation();
     if (!hasChildNodes() || !requiresTextTrackRepresentation) {
         if (m_textTrackRepresentation) {
-            if (!requiresTextTrackRepresentation) {
+            if (!requiresTextTrackRepresentation)
                 clearTextTrackRepresentation();
-                updateSizes(ForceUpdate::Yes);
-            } else
+            else
                 m_textTrackRepresentation->setHidden(true);
         }
         return;
@@ -1308,18 +1321,13 @@
     if (!m_textTrackRepresentation) {
         ALWAYS_LOG(LOGIDENTIFIER);
 
-        m_waitingForFirstLayout = true;
         m_textTrackRepresentation = TextTrackRepresentation::create(*this);
         if (document().page())
             m_textTrackRepresentation->setContentScale(document().page()->deviceScaleFactor());
         mediaElement->setTextTrackRepresentation(m_textTrackRepresentation.get());
-        updateSizes();
-        updateTextTrackRepresentationStyle();
     }
 
     m_textTrackRepresentation->setHidden(false);
-    if (!m_waitingForFirstLayout)
-        m_textTrackRepresentation->update();
 }
 
 void MediaControlTextTrackContainerElement::clearTextTrackRepresentation()
@@ -1329,20 +1337,17 @@
 
     ALWAYS_LOG(LOGIDENTIFIER);
 
-    m_waitingForFirstLayout = true;
     m_textTrackRepresentation = nullptr;
     if (auto mediaElement = parentMediaElement(this))
         mediaElement->setTextTrackRepresentation(nullptr);
-    updateTextTrackRepresentationStyle();
-    updateActiveCuesFontSize();
 }
 
-void MediaControlTextTrackContainerElement::updateTextTrackRepresentationStyle()
+void MediaControlTextTrackContainerElement::updateTextTrackStyle()
 {
     if (m_textTrackRepresentation) {
+        setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
         setInlineStyleProperty(CSSPropertyWidth, m_videoDisplaySize.size().width(), CSSUnitType::CSS_PX);
         setInlineStyleProperty(CSSPropertyHeight, m_videoDisplaySize.size().height(), CSSUnitType::CSS_PX);
-        setInlineStyleProperty(CSSPropertyPosition, CSSValueAbsolute);
         setInlineStyleProperty(CSSPropertyLeft, 0, CSSUnitType::CSS_PX);
         setInlineStyleProperty(CSSPropertyTop, 0, CSSUnitType::CSS_PX);
         return;
@@ -1357,8 +1362,7 @@
 
 void MediaControlTextTrackContainerElement::enteredFullscreen()
 {
-    if (hasChildNodes())
-        updateTextTrackRepresentation();
+    updateTextTrackRepresentationIfNeeded();
     updateSizes(ForceUpdate::Yes);
 }
 
@@ -1368,23 +1372,6 @@
     updateSizes(ForceUpdate::Yes);
 }
 
-void MediaControlTextTrackContainerElement::layoutIfNecessary()
-{
-    m_waitingForFirstLayout = false;
-
-    auto sizeChanged = updateVideoDisplaySize();
-    if (m_textTrackRepresentation)
-        m_textTrackRepresentation->update();
-
-    if (!sizeChanged)
-        return;
-
-    // FIXME (121170): This function is called during layout, and should lay out the text tracks immediately.
-    m_taskQueue.enqueueTask([this] () {
-        updateCueStyles();
-    });
-}
-
 bool MediaControlTextTrackContainerElement::updateVideoDisplaySize()
 {
     if (!document().page())
@@ -1410,19 +1397,14 @@
         return false;
 
     m_videoDisplaySize = videoBox;
-    updateTextTrackRepresentationStyle();
-
     return true;
 }
 
 void MediaControlTextTrackContainerElement::updateSizes(ForceUpdate force)
 {
-    if (updateVideoDisplaySize() || force == ForceUpdate::Yes)
-        updateCueStyles();
-}
+    if (!updateVideoDisplaySize() && force != ForceUpdate::Yes)
+        return;
 
-void MediaControlTextTrackContainerElement::updateCueStyles()
-{
     if (!document().page())
         return;
 
@@ -1432,12 +1414,14 @@
 
     mediaElement->syncTextTrackBounds();
 
+    updateActiveCuesFontSize();
+    updateTextStrokeStyle();
     for (auto& activeCue : mediaElement->currentlyActiveCues())
         activeCue.data()->recalculateStyles();
 
-    updateActiveCuesFontSize();
-    updateDisplay();
-    updateTextStrokeStyle();
+    m_taskQueue.enqueueTask([this] () {
+        updateDisplay();
+    });
 }
 
 RefPtr<Image> MediaControlTextTrackContainerElement::createTextTrackRepresentationImage()
@@ -1471,7 +1455,9 @@
     if (!buffer)
         return nullptr;
 
-    layer->paint(buffer->context(), paintingRect, LayoutSize(), { PaintBehavior::FlattenCompositingLayers, PaintBehavior::Snapshotting }, nullptr, RenderLayer::paintLayerPaintingCompositingAllPhasesFlags());
+    auto paintFlags = RenderLayer::paintLayerPaintingCompositingAllPhasesFlags();
+    paintFlags.add(RenderLayer::PaintLayerTemporaryClipRects);
+    layer->paint(buffer->context(), paintingRect, LayoutSize(), { PaintBehavior::FlattenCompositingLayers, PaintBehavior::Snapshotting }, nullptr, paintFlags);
 
     return ImageBuffer::sinkIntoImage(WTFMove(buffer));
 }
@@ -1478,8 +1464,7 @@
 
 void MediaControlTextTrackContainerElement::textTrackRepresentationBoundsChanged(const IntRect&)
 {
-    if (hasChildNodes())
-        updateTextTrackRepresentation();
+    updateTextTrackRepresentationIfNeeded();
     updateSizes();
 }
 

Modified: trunk/Source/WebCore/html/shadow/MediaControlElements.h (258433 => 258434)


--- trunk/Source/WebCore/html/shadow/MediaControlElements.h	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/html/shadow/MediaControlElements.h	2020-03-13 21:49:31 UTC (rev 258434)
@@ -483,9 +483,10 @@
 
     enum class ForceUpdate { Yes, No };
     void updateSizes(ForceUpdate force = ForceUpdate::No);
-    void layoutIfNecessary();
+    void updateDisplay();
 
-    void updateDisplay();
+    void updateTextTrackRepresentationImageIfNeeded();
+
     void enteredFullscreen();
     void exitedFullscreen();
 
@@ -492,19 +493,22 @@
 private:
     explicit MediaControlTextTrackContainerElement(Document&);
 
-    bool updateVideoDisplaySize();
-    void updateCueStyles();
-    void updateActiveCuesFontSize();
-    void updateTextStrokeStyle();
-    void processActiveVTTCue(VTTCue&);
+    // Element
     RenderPtr<RenderElement> createElementRenderer(RenderStyle&&, const RenderTreePosition&) override;
 
+    // TextTrackRepresentationClient
     RefPtr<Image> createTextTrackRepresentationImage() override;
     void textTrackRepresentationBoundsChanged(const IntRect&) override;
-    void updateTextTrackRepresentation();
+
+    void updateTextTrackRepresentationIfNeeded();
     void clearTextTrackRepresentation();
-    void updateTextTrackRepresentationStyle();
 
+    bool updateVideoDisplaySize();
+    void updateActiveCuesFontSize();
+    void updateTextStrokeStyle();
+    void processActiveVTTCue(VTTCue&);
+    void updateTextTrackStyle();
+
 #if !RELEASE_LOG_DISABLED
     const Logger& logger() const final;
     const void* logIdentifier() const final;
@@ -520,7 +524,7 @@
     IntRect m_videoDisplaySize;
     int m_fontSize { 0 };
     bool m_fontSizeIsImportant { false };
-    bool m_waitingForFirstLayout { true };
+    bool m_needsGenerateTextTrackRepresentation { false };
 };
 
 #endif

Modified: trunk/Source/WebCore/html/shadow/MediaControls.cpp (258433 => 258434)


--- trunk/Source/WebCore/html/shadow/MediaControls.cpp	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/html/shadow/MediaControls.cpp	2020-03-13 21:49:31 UTC (rev 258434)
@@ -405,7 +405,7 @@
 
     m_textDisplayContainer->updateDisplay();
 }
-    
+
 void MediaControls::textTrackPreferencesChanged()
 {
     closedCaptionTracksChanged();
@@ -419,6 +419,12 @@
         m_textDisplayContainer->removeChildren();
 }
 
+void MediaControls::updateTextTrackRepresentationImageIfNeeded()
+{
+    if (m_textDisplayContainer)
+        m_textDisplayContainer->updateTextTrackRepresentationImageIfNeeded();
+}
+
 #endif
 
 void MediaControls::setSliderVolume()

Modified: trunk/Source/WebCore/html/shadow/MediaControls.h (258433 => 258434)


--- trunk/Source/WebCore/html/shadow/MediaControls.h	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/html/shadow/MediaControls.h	2020-03-13 21:49:31 UTC (rev 258434)
@@ -98,6 +98,7 @@
     virtual void updateTextTrackDisplay();
     virtual void textTrackPreferencesChanged();
     virtual void clearTextDisplayContainer();
+    virtual void updateTextTrackRepresentationImageIfNeeded();
 #endif
 
 protected:

Modified: trunk/Source/WebCore/page/Page.cpp (258433 => 258434)


--- trunk/Source/WebCore/page/Page.cpp	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/page/Page.cpp	2020-03-13 21:49:31 UTC (rev 258434)
@@ -1355,10 +1355,16 @@
 #endif
 
     layoutIfNeeded();
-    
+
     forEachDocument([] (Document& document) {
         document.updateHighlightPositions();
     });
+
+#if ENABLE(VIDEO_TRACK)
+    forEachDocument([] (Document& document) {
+        document.updateTextTrackRepresentationImageIfNeeded();
+    });
+#endif
 }
 
 void Page::suspendScriptedAnimations()

Modified: trunk/Source/WebCore/rendering/RenderMediaControlElements.cpp (258433 => 258434)


--- trunk/Source/WebCore/rendering/RenderMediaControlElements.cpp	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/rendering/RenderMediaControlElements.cpp	2020-03-13 21:49:31 UTC (rev 258434)
@@ -41,7 +41,7 @@
 WTF_MAKE_ISO_ALLOCATED_IMPL(RenderMediaVolumeSliderContainer);
 WTF_MAKE_ISO_ALLOCATED_IMPL(RenderMediaControlTimelineContainer);
 #if ENABLE(VIDEO_TRACK)
-WTF_MAKE_ISO_ALLOCATED_IMPL(RenderTextTrackContainerElement);
+WTF_MAKE_ISO_ALLOCATED_IMPL(RenderMediaControlTextTrackContainer);
 #endif
 
 RenderMediaVolumeSliderContainer::RenderMediaVolumeSliderContainer(Element& element, RenderStyle&& style)
@@ -90,12 +90,12 @@
 
 #if ENABLE(VIDEO_TRACK)
 
-RenderTextTrackContainerElement::RenderTextTrackContainerElement(Element& element, RenderStyle&& style)
+RenderMediaControlTextTrackContainer::RenderMediaControlTextTrackContainer(Element& element, RenderStyle&& style)
     : RenderBlockFlow(element, WTFMove(style))
 {
 }
 
-void RenderTextTrackContainerElement::layout()
+void RenderMediaControlTextTrackContainer::layout()
 {
     RenderBlockFlow::layout();
     if (style().display() == DisplayType::None)
@@ -104,7 +104,6 @@
     ASSERT(mediaControlElementType(element()) == MediaTextTrackDisplayContainer);
 
     LayoutStateDisabler layoutStateDisabler(view().frameView().layoutContext());
-    static_cast<MediaControlTextTrackContainerElement*>(element())->layoutIfNecessary();
 }
 
 #endif // ENABLE(VIDEO_TRACK)

Modified: trunk/Source/WebCore/rendering/RenderMediaControlElements.h (258433 => 258434)


--- trunk/Source/WebCore/rendering/RenderMediaControlElements.h	2020-03-13 21:47:48 UTC (rev 258433)
+++ trunk/Source/WebCore/rendering/RenderMediaControlElements.h	2020-03-13 21:49:31 UTC (rev 258434)
@@ -59,10 +59,10 @@
 
 #if ENABLE(VIDEO_TRACK)
 
-class RenderTextTrackContainerElement final : public RenderBlockFlow {
-    WTF_MAKE_ISO_ALLOCATED(RenderTextTrackContainerElement);
+class RenderMediaControlTextTrackContainer final : public RenderBlockFlow {
+    WTF_MAKE_ISO_ALLOCATED(RenderMediaControlTextTrackContainer);
 public:
-    RenderTextTrackContainerElement(Element&, RenderStyle&&);
+    RenderMediaControlTextTrackContainer(Element&, RenderStyle&&);
 
 private:
     void layout() override;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to