Skip to content

Commit

Permalink
Cherry-pick 84643ce. rdar://123324508
Browse files Browse the repository at this point in the history
    Regression(274445@main) Hang on twitch.tv when playing certain videos
    https://bugs.webkit.org/show_bug.cgi?id=269810
    rdar://123324508

    Unreviewed partial revert of 274445@main and full revert of the follow-up
    fix in 274670@main as it appears to be causing hangs under
    HTMLMediaElement::updateActiveTextTrackCues().

    * Source/WebCore/html/HTMLMediaElement.cpp:
    (WebCore::compareCueInterval):
    (WebCore::HTMLMediaElement::updateActiveTextTrackCues):
    * Source/WebCore/html/HTMLMediaElement.h:
    * Source/WebCore/html/shadow/MediaControlTextTrackContainerElement.cpp:
    (WebCore::compareCueIntervalForDisplay):
    (WebCore::MediaControlTextTrackContainerElement::updateDisplay):
    (WebCore::MediaControlTextTrackContainerElement::updateActiveCuesFontSize):
    * Source/WebCore/platform/PODInterval.h:

    Canonical link: https://commits.webkit.org/275075@main

Identifier: 274471.475@safari-7619.1.5-branch
  • Loading branch information
cdumez authored and Dan Robson committed Feb 21, 2024
1 parent e2cd4b5 commit d5ae5f5
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 25 deletions.
18 changes: 9 additions & 9 deletions Source/WebCore/html/HTMLMediaElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1711,7 +1711,7 @@ void HTMLMediaElement::loadResource(const URL& initialURL, ContentType& contentT

struct HTMLMediaElement::CueData {
WTF_MAKE_STRUCT_FAST_ALLOCATED;
PODIntervalTree<MediaTime, WeakPtr<TextTrackCue, WeakPtrImplWithEventTargetData>> cueTree;
PODIntervalTree<MediaTime, TextTrackCue*> cueTree;
CueList currentlyActiveCues;
};

Expand Down Expand Up @@ -1741,7 +1741,7 @@ static bool eventTimeCueCompare(const std::pair<MediaTime, RefPtr<TextTrackCue>>

static bool compareCueInterval(const CueInterval& one, const CueInterval& two)
{
return RefPtr { one.data().get() }->isOrderedBefore(RefPtr { two.data().get() }.get());
return RefPtr { one.data() }->isOrderedBefore(RefPtr { two.data() }.get());
}

static bool compareCueIntervalEndTime(const CueInterval& one, const CueInterval& two)
Expand Down Expand Up @@ -1777,7 +1777,7 @@ void HTMLMediaElement::updateActiveTextTrackCues(const MediaTime& movieTime)
// The user agent must synchronously unset [the text track cue active] flag
// whenever ... the media element's readyState is changed back to HAVE_NOTHING.
if (m_readyState != HAVE_NOTHING && m_player) {
for (auto& cue : m_cueData->cueTree.allOverlaps({ movieTime, movieTime, WeakPtr<TextTrackCue, WeakPtrImplWithEventTargetData>() })) {
for (auto& cue : m_cueData->cueTree.allOverlaps({ movieTime, movieTime })) {
if (cue.low() <= movieTime && cue.high() > movieTime)
currentCues.append(cue);
}
Expand Down Expand Up @@ -1805,7 +1805,7 @@ void HTMLMediaElement::updateActiveTextTrackCues(const MediaTime& movieTime)
// end times are less than or equal to the current playback position.
// Otherwise, let missed cues be an empty list.
if (lastTime >= MediaTime::zeroTime() && m_lastSeekTime < movieTime) {
for (auto& cue : m_cueData->cueTree.allOverlaps({ lastTime, movieTime, WeakPtr<TextTrackCue, WeakPtrImplWithEventTargetData>() })) {
for (auto& cue : m_cueData->cueTree.allOverlaps({ lastTime, movieTime })) {
// Consider cues that may have been missed since the last seek time.
if (cue.low() > std::max(m_lastSeekTime, lastTime) && cue.high() < movieTime)
missedCues.append(cue);
Expand Down Expand Up @@ -1840,7 +1840,7 @@ void HTMLMediaElement::updateActiveTextTrackCues(const MediaTime& movieTime)
activeSetChanged = true;

for (size_t i = 0; i < currentCuesSize; ++i) {
RefPtr cue = currentCues[i].data().get();
RefPtr cue = currentCues[i].data();
cue->updateDisplayTree(movieTime);
if (!cue->isActive())
activeSetChanged = true;
Expand Down Expand Up @@ -1900,7 +1900,7 @@ void HTMLMediaElement::updateActiveTextTrackCues(const MediaTime& movieTime)
for (size_t i = 0; i < missedCuesSize; ++i) {
// 9 - For each text track cue in missed cues, prepare an event named enter
// for the TextTrackCue object with the text track cue start time.
eventTasks.append({ missedCues[i].data()->startMediaTime(), missedCues[i].data().get() });
eventTasks.append({ missedCues[i].data()->startMediaTime(), missedCues[i].data() });

// 10 - For each text track [...] in missed cues, prepare an event
// named exit for the TextTrackCue object with the with the later of
Expand All @@ -1912,23 +1912,23 @@ void HTMLMediaElement::updateActiveTextTrackCues(const MediaTime& movieTime)
// affect sorting events before dispatch either, because the exit
// event has the same time as the enter event.
if (missedCues[i].data()->startMediaTime() < missedCues[i].data()->endMediaTime())
eventTasks.append({ missedCues[i].data()->endMediaTime(), missedCues[i].data().get() });
eventTasks.append({ missedCues[i].data()->endMediaTime(), missedCues[i].data() });
}

for (size_t i = 0; i < previousCuesSize; ++i) {
// 10 - For each text track cue in other cues that has its text
// track cue active flag set prepare an event named exit for the
// TextTrackCue object with the text track cue end time.
if (!currentCues.contains(previousCues[i]))
eventTasks.append({ previousCues[i].data()->endMediaTime(), previousCues[i].data().get() });
eventTasks.append({ previousCues[i].data()->endMediaTime(), previousCues[i].data() });
}

for (size_t i = 0; i < currentCuesSize; ++i) {
// 11 - For each text track cue in current cues that does not have its
// text track cue active flag set, prepare an event named enter for the
// TextTrackCue object with the text track cue start time.
if (!previousCues.contains(currentCues[i]))
eventTasks.append({ currentCues[i].data()->startMediaTime(), currentCues[i].data().get() });
eventTasks.append({ currentCues[i].data()->startMediaTime(), currentCues[i].data() });
}

// 12 - Sort the tasks in events in ascending time order (tasks with earlier
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/html/HTMLMediaElement.h
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ template<typename, typename> class PODInterval;
class RemotePlayback;
#endif

using CueInterval = PODInterval<MediaTime, WeakPtr<TextTrackCue, WeakPtrImplWithEventTargetData>>;
using CueInterval = PODInterval<MediaTime, TextTrackCue*>;
using CueList = Vector<CueInterval>;

using MediaProvider = std::optional < std::variant <
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ RenderPtr<RenderElement> MediaControlTextTrackContainerElement::createElementRen

static bool compareCueIntervalForDisplay(const CueInterval& one, const CueInterval& two)
{
return one.data()->isPositionedAbove(two.data().get());
return one.data()->isPositionedAbove(two.data());
};

void MediaControlTextTrackContainerElement::updateDisplay()
Expand Down Expand Up @@ -156,7 +156,7 @@ void MediaControlTextTrackContainerElement::updateDisplay()
removeChildren();

activeCues.removeAllMatching([] (CueInterval& cueInterval) {
RefPtr cue = cueInterval.data().get();
RefPtr cue = cueInterval.data();
return !cue->track()
|| !cue->track()->isRendered()
|| cue->track()->mode() == TextTrack::Mode::Disabled
Expand Down Expand Up @@ -256,7 +256,7 @@ void MediaControlTextTrackContainerElement::updateActiveCuesFontSize()
m_fontSize = lroundf(100 * fontScale);

for (auto& activeCue : m_mediaElement->currentlyActiveCues()) {
RefPtr cue = activeCue.data().get();
RefPtr cue = activeCue.data();
if (cue->isRenderable())
cue->setFontSize(m_fontSize, m_fontSizeIsImportant);
}
Expand Down
13 changes: 1 addition & 12 deletions Source/WebCore/platform/PODInterval.h
Original file line number Diff line number Diff line change
Expand Up @@ -143,18 +143,7 @@ template<typename T, typename U, typename WeakPtrImpl> class PODInterval<T, Weak
return true;
if (other.Base::low() < Base::low())
return false;
if (Base::high() < other.Base::high())
return true;
if (other.Base::high() < Base::high())
return false;
return Base::data().get() < other.Base::data().get();
}

bool operator==(const PODInterval& other) const
{
return Base::low() == other.Base::low()
&& Base::high() == other.Base::high()
&& Base::data() == other.Base::data();
return Base::high() < other.Base::high();
}

private:
Expand Down

0 comments on commit d5ae5f5

Please sign in to comment.