Skip to content
Permalink
Browse files
[ Catalina EWS ] media/track/track-disabled-addcue.html is flaky cras…
…hing

https://bugs.webkit.org/show_bug.cgi?id=229462
<rdar://problem/82302915>

Reviewed by Darin Adler.

Source/WebCore:

Use `queueTaskKeepingObjectAlive` instead of a timer when scheduling text track
loading.

* html/HTMLTrackElement.cpp:
(WebCore::HTMLTrackElement::HTMLTrackElement):
(WebCore::HTMLTrackElement::scheduleLoad):
(WebCore::HTMLTrackElement::scheduleTask):
(WebCore::HTMLTrackElement::loadTimerFired): Deleted.
* html/HTMLTrackElement.h:
* html/track/LoadableTextTrack.cpp:
(WebCore::LoadableTextTrack::LoadableTextTrack):
(WebCore::LoadableTextTrack::scheduleLoad):
(WebCore::LoadableTextTrack::element):
(WebCore::LoadableTextTrack::loadTimerFired): Deleted.
* html/track/LoadableTextTrack.h:

Source/WTF:

* wtf/SetForScope.h:
(WTF::SetForScope::SetForScope): Add a constructor that takes the value to set
on scope exit.

LayoutTests:

* platform/mac-wk1/TestExpectations:


Canonical link: https://commits.webkit.org/241069@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@281727 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
eric-carlson committed Aug 27, 2021
1 parent d129693 commit 3aa02efbac32f72ead5b4d619e1ababeaf59c7a4
Showing 9 changed files with 121 additions and 56 deletions.
@@ -1,3 +1,13 @@
2021-08-27 Eric Carlson <eric.carlson@apple.com>

[ Catalina EWS ] media/track/track-disabled-addcue.html is flaky crashing
https://bugs.webkit.org/show_bug.cgi?id=229462
<rdar://problem/82302915>

Reviewed by Darin Adler.

* platform/mac-wk1/TestExpectations:

2021-08-27 Simon Fraser <simon.fraser@apple.com>

[CSS3 Backgrounds and Borders] The border image area should be empty if border-style is none and border-image-width is not set
@@ -646,8 +646,6 @@ media/track/track-cues-enter-exit.html [ Pass Crash ]
# <rdar://problem/61744994> media/track/track-cues-cuechange.html is flaky timing out.
media/track/track-cues-cuechange.html [ Pass Crash Timeout ]

webkit.org/b/229462 [ Catalina Debug ] media/track/track-disabled-addcue.html [ Pass Crash ]

webkit.org/b/167127 pointer-lock/locked-element-removed-from-dom.html

webkit.org/b/167857 pageoverlay/overlay-remove-reinsert-view.html [ Skip ]
@@ -1,3 +1,15 @@
2021-08-27 Eric Carlson <eric.carlson@apple.com>

[ Catalina EWS ] media/track/track-disabled-addcue.html is flaky crashing
https://bugs.webkit.org/show_bug.cgi?id=229462
<rdar://problem/82302915>

Reviewed by Darin Adler.

* wtf/SetForScope.h:
(WTF::SetForScope::SetForScope): Add a constructor that takes the value to set
on scope exit.

2021-08-27 Russell Epstein <repstein@apple.com>

Land Windows build fixes from safari-612.1.29.14-branch
@@ -47,24 +47,33 @@ class SetForScope {
public:
SetForScope(T& scopedVariable)
: m_scopedVariable(scopedVariable)
, m_originalValue(scopedVariable)
, m_valueToRestore(scopedVariable)
{
}

template<typename U>
SetForScope(T& scopedVariable, U&& newValue)
: SetForScope(scopedVariable)
{
m_scopedVariable = std::forward<U>(newValue);
}

template<typename U, typename V>
SetForScope(T& scopedVariable, U&& newValue, V&& restoreValue)
: m_scopedVariable(scopedVariable)
, m_valueToRestore(std::forward<V>(restoreValue))
{
m_scopedVariable = std::forward<U>(newValue);
}

~SetForScope()
{
m_scopedVariable = WTFMove(m_originalValue);
m_scopedVariable = WTFMove(m_valueToRestore);
}

private:
T& m_scopedVariable;
T m_originalValue;
T m_valueToRestore;
};

}
@@ -1,3 +1,27 @@
2021-08-27 Eric Carlson <eric.carlson@apple.com>

[ Catalina EWS ] media/track/track-disabled-addcue.html is flaky crashing
https://bugs.webkit.org/show_bug.cgi?id=229462
<rdar://problem/82302915>

Reviewed by Darin Adler.

Use `queueTaskKeepingObjectAlive` instead of a timer when scheduling text track
loading.

* html/HTMLTrackElement.cpp:
(WebCore::HTMLTrackElement::HTMLTrackElement):
(WebCore::HTMLTrackElement::scheduleLoad):
(WebCore::HTMLTrackElement::scheduleTask):
(WebCore::HTMLTrackElement::loadTimerFired): Deleted.
* html/HTMLTrackElement.h:
* html/track/LoadableTextTrack.cpp:
(WebCore::LoadableTextTrack::LoadableTextTrack):
(WebCore::LoadableTextTrack::scheduleLoad):
(WebCore::LoadableTextTrack::element):
(WebCore::LoadableTextTrack::loadTimerFired): Deleted.
* html/track/LoadableTextTrack.h:

2021-08-27 Russell Epstein <repstein@apple.com>

Land Windows build fixes from safari-612.1.29.14-branch.
@@ -1,6 +1,6 @@
/*
* Copyright (C) 2011, 2013 Google Inc. All rights reserved.
* Copyright (C) 2011-2017 Apple Inc. All rights reserved.
* Copyright (C) 2011-2021 Apple Inc. All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
@@ -60,7 +60,6 @@ static String urlForLoggingTrack(const URL& url)
inline HTMLTrackElement::HTMLTrackElement(const QualifiedName& tagName, Document& document)
: HTMLElement(tagName, document)
, ActiveDOMObject(document)
, m_loadTimer(*this, &HTMLTrackElement::loadTimerFired)
{
LOG(Media, "HTMLTrackElement::HTMLTrackElement - %p", this);
ASSERT(hasTagName(trackTag));
@@ -168,7 +167,7 @@ void HTMLTrackElement::scheduleLoad()
{
// 1. If another occurrence of this algorithm is already running for this text track and its track element,
// abort these steps, letting that other algorithm take care of this element.
if (m_loadTimer.isActive())
if (m_loadPending)
return;

// 2. If the text track's text track mode is not set to one of hidden or showing, abort these steps.
@@ -180,33 +179,41 @@ void HTMLTrackElement::scheduleLoad()
return;

// 4. Run the remainder of these steps asynchronously, allowing whatever caused these steps to run to continue.
m_loadTimer.startOneShot(0_s);
}
m_loadPending = true;
scheduleTask([this]() mutable {

void HTMLTrackElement::loadTimerFired()
{
if (!hasAttributeWithoutSynchronization(srcAttr)) {
track().removeAllCues();
return;
}
SetForScope<bool> loadPending { m_loadPending, true, false };

// 6. Set the text track readiness state to loading.
setReadyState(HTMLTrackElement::LOADING);
if (!hasAttributeWithoutSynchronization(srcAttr)) {
track().removeAllCues();
return;
}

// 7. Let URL be the track URL of the track element.
URL url = getNonEmptyURLAttribute(srcAttr);
// 6. Set the text track readiness state to loading.
setReadyState(HTMLTrackElement::LOADING);

// ... if URL is the empty string, then queue a task to first change the text track readiness state
// to failed to load and then fire an event named error at the track element.
// 8. If the track element's parent is a media element then let CORS mode be the state of the parent media
// element's crossorigin content attribute. Otherwise, let CORS mode be No CORS.
if (url.isEmpty() || !canLoadURL(url)) {
track().removeAllCues();
didCompleteLoad(HTMLTrackElement::Failure);
return;
}
// 7. Let URL be the track URL of the track element.
URL trackURL = getNonEmptyURLAttribute(srcAttr);

// ... if URL is the empty string, then queue a task to first change the text track readiness state
// to failed to load and then fire an event named error at the track element.
// 8. If the track element's parent is a media element then let CORS mode be the state of the parent media
// element's crossorigin content attribute. Otherwise, let CORS mode be No CORS.
if (!canLoadURL(trackURL)) {
track().removeAllCues();
didCompleteLoad(HTMLTrackElement::Failure);
return;
}

track().scheduleLoad(url);
track().scheduleLoad(trackURL);
});
}

void HTMLTrackElement::scheduleTask(Function<void()>&& task)
{
queueTaskKeepingObjectAlive(*this, TaskSource::MediaElement, [task = WTFMove(task)]() mutable {
task();
});
}

bool HTMLTrackElement::canLoadURL(const URL& url)
@@ -62,6 +62,8 @@ class HTMLTrackElement final : public HTMLElement, public ActiveDOMObject, publi
RefPtr<HTMLMediaElement> mediaElement() const;
const AtomString& mediaElementCrossOriginAttribute() const;

void scheduleTask(Function<void()>&&);

private:
HTMLTrackElement(const QualifiedName&, Document&);
virtual ~HTMLTrackElement();
@@ -80,8 +82,6 @@ class HTMLTrackElement final : public HTMLElement, public ActiveDOMObject, publi
// EventTarget.
void eventListenersDidChange() final;

void loadTimerFired();

// TextTrackClient
void textTrackModeChanged(TextTrack&) final;
void textTrackKindChanged(TextTrack&) final;
@@ -93,7 +93,7 @@ class HTMLTrackElement final : public HTMLElement, public ActiveDOMObject, publi
bool canLoadURL(const URL&);

RefPtr<LoadableTextTrack> m_track;
Timer m_loadTimer;
bool m_loadPending { false };
bool m_hasRelevantLoadEventsListener { false };
};

@@ -42,7 +42,6 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(LoadableTextTrack);
LoadableTextTrack::LoadableTextTrack(HTMLTrackElement& track, const String& kind, const String& label, const String& language)
: TextTrack(&track.document(), &track, kind, emptyString(), label, language, TrackElement)
, m_trackElement(&track)
, m_loadTimer(*this, &LoadableTextTrack::loadTimerFired)
{
}

@@ -55,44 +54,50 @@ Ref<LoadableTextTrack> LoadableTextTrack::create(HTMLTrackElement& track, const

void LoadableTextTrack::scheduleLoad(const URL& url)
{
ASSERT(!url.isEmpty());

if (url == m_url)
return;

// When src attribute is changed we need to flush all collected track data
removeAllCues();

if (!m_trackElement)
return;

// 4.8.10.12.3 Sourcing out-of-band text tracks (continued)

// 2. Let URL be the track URL of the track element.
m_url = url;

// 3. Asynchronously run the remaining steps, while continuing with whatever task
if (m_loadPending)
return;

// 3. Asynchronously run the remaining steps, while continuing with whatever task
// was responsible for creating the text track or changing the text track mode.
if (!m_loadTimer.isActive())
m_loadTimer.startOneShot(0_s);
}
m_trackElement->scheduleTask([this]() mutable {
SetForScope<bool> loadPending { m_loadPending, true, false };

Element* LoadableTextTrack::element()
{
return m_trackElement;
}
if (m_loader)
m_loader->cancelLoad();

void LoadableTextTrack::loadTimerFired()
{
if (m_loader)
m_loader->cancelLoad();
if (!m_trackElement)
return;

if (!m_trackElement)
return;
// 4.8.10.12.3 Sourcing out-of-band text tracks (continued)

// 4.8.10.12.3 Sourcing out-of-band text tracks (continued)
// 4. Download: If URL is not the empty string, perform a potentially CORS-enabled fetch of URL, with the
// mode being the state of the media element's crossorigin content attribute, the origin being the
// origin of the media element's Document, and the default origin behaviour set to fail.
m_loader = makeUnique<TextTrackLoader>(static_cast<TextTrackLoaderClient&>(*this), m_trackElement->document());
if (!m_loader->load(m_url, *m_trackElement))
m_trackElement->didCompleteLoad(HTMLTrackElement::Failure);
});
}

// 4. Download: If URL is not the empty string, perform a potentially CORS-enabled fetch of URL, with the
// mode being the state of the media element's crossorigin content attribute, the origin being the
// origin of the media element's Document, and the default origin behaviour set to fail.
m_loader = makeUnique<TextTrackLoader>(static_cast<TextTrackLoaderClient&>(*this), m_trackElement->document());
if (!m_loader->load(m_url, *m_trackElement))
m_trackElement->didCompleteLoad(HTMLTrackElement::Failure);
Element* LoadableTextTrack::element()
{
return m_trackElement;
}

void LoadableTextTrack::newCuesAvailable(TextTrackLoader& loader)
@@ -65,9 +65,9 @@ class LoadableTextTrack final : public TextTrack, private TextTrackLoaderClient
#endif

HTMLTrackElement* m_trackElement;
Timer m_loadTimer;
std::unique_ptr<TextTrackLoader> m_loader;
URL m_url;
bool m_loadPending { false };
};

} // namespace WebCore

0 comments on commit 3aa02ef

Please sign in to comment.