HTMLTrackElement should be pending while it is waiting for LoadableTextTrack request
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2020 21:07:33 +0000 (21:07 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 27 Mar 2020 21:07:33 +0000 (21:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=208798
<rdar://problem/60325421>

Reviewed by Geoffrey Garen.

Have HTMLTrackElement and subclass ActiveDOMObject::hasPendingActivity() to keeps its
wrapper alive if its in LOADING state and the page's script has relevant load events
event listeners registered.

No new tests, covered by media/track/track-disabled-addcue.html.

* html/HTMLTrackElement.cpp:
(WebCore::HTMLTrackElement::HTMLTrackElement):
(WebCore::HTMLTrackElement::create):
(WebCore::HTMLTrackElement::didCompleteLoad):
(WebCore::HTMLTrackElement::readyState const):
(WebCore::HTMLTrackElement::activeDOMObjectName const):
(WebCore::HTMLTrackElement::eventListenersDidChange):
(WebCore::HTMLTrackElement::hasPendingActivity const):
(WebCore::HTMLTrackElement::readyState): Deleted.
* html/HTMLTrackElement.h:
* html/HTMLTrackElement.idl:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@259138 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/html/HTMLTrackElement.cpp
Source/WebCore/html/HTMLTrackElement.h
Source/WebCore/html/HTMLTrackElement.idl

index 014622c..17f1a0b 100644 (file)
@@ -1,3 +1,29 @@
+2020-03-27  Chris Dumez  <cdumez@apple.com>
+
+        HTMLTrackElement should be pending while it is waiting for LoadableTextTrack request
+        https://bugs.webkit.org/show_bug.cgi?id=208798
+        <rdar://problem/60325421>
+
+        Reviewed by Geoffrey Garen.
+
+        Have HTMLTrackElement and subclass ActiveDOMObject::hasPendingActivity() to keeps its
+        wrapper alive if its in LOADING state and the page's script has relevant load events
+        event listeners registered.
+
+        No new tests, covered by media/track/track-disabled-addcue.html.
+
+        * html/HTMLTrackElement.cpp:
+        (WebCore::HTMLTrackElement::HTMLTrackElement):
+        (WebCore::HTMLTrackElement::create):
+        (WebCore::HTMLTrackElement::didCompleteLoad):
+        (WebCore::HTMLTrackElement::readyState const):
+        (WebCore::HTMLTrackElement::activeDOMObjectName const):
+        (WebCore::HTMLTrackElement::eventListenersDidChange):
+        (WebCore::HTMLTrackElement::hasPendingActivity const):
+        (WebCore::HTMLTrackElement::readyState): Deleted.
+        * html/HTMLTrackElement.h:
+        * html/HTMLTrackElement.idl:
+
 2020-03-27  Simon Fraser  <simon.fraser@apple.com>
 
         Hovering over countries at https://covidinc.io/ shows bizarre rendering artifacts
index d33f8d5..67f766e 100644 (file)
@@ -59,6 +59,7 @@ 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);
@@ -75,7 +76,9 @@ HTMLTrackElement::~HTMLTrackElement()
 
 Ref<HTMLTrackElement> HTMLTrackElement::create(const QualifiedName& tagName, Document& document)
 {
-    return adoptRef(*new HTMLTrackElement(tagName, document));
+    auto trackElement = adoptRef(*new HTMLTrackElement(tagName, document));
+    trackElement->suspendIfNeeded();
+    return trackElement;
 }
 
 Node::InsertedIntoAncestorResult HTMLTrackElement::insertedIntoAncestor(InsertionType insertionType, ContainerNode& parentOfInsertedTree)
@@ -232,6 +235,10 @@ bool HTMLTrackElement::canLoadURL(const URL& url)
 
 void HTMLTrackElement::didCompleteLoad(LoadStatus status)
 {
+    // Make sure the JS wrapper stays alive until the end of this method, even though we update the
+    // readyState to no longer be LOADING.
+    auto wrapperProtector = makePendingActivity(*this);
+
     // 4.8.10.12.3 Sourcing out-of-band text tracks (continued)
     
     // 4. Download: ...
@@ -271,9 +278,11 @@ void HTMLTrackElement::setReadyState(ReadyState state)
         parent->textTrackReadyStateChanged(m_track.get());
 }
 
-HTMLTrackElement::ReadyState HTMLTrackElement::readyState() 
+HTMLTrackElement::ReadyState HTMLTrackElement::readyState() const
 {
-    return static_cast<ReadyState>(track().readinessState());
+    if (!m_track)
+        return HTMLTrackElement::NONE;
+    return static_cast<ReadyState>(m_track->readinessState());
 }
 
 const AtomString& HTMLTrackElement::mediaElementCrossOriginAttribute() const
@@ -331,6 +340,25 @@ RefPtr<HTMLMediaElement> HTMLTrackElement::mediaElement() const
     return downcast<HTMLMediaElement>(parent.get());
 }
 
+const char* HTMLTrackElement::activeDOMObjectName() const
+{
+    return "HTMLTrackElement";
+}
+
+void HTMLTrackElement::eventListenersDidChange()
+{
+    m_hasRelevantLoadEventsListener = hasEventListeners(eventNames().errorEvent)
+        || hasEventListeners(eventNames().loadEvent);
+}
+
+bool HTMLTrackElement::hasPendingActivity() const
+{
+    if (ActiveDOMObject::hasPendingActivity())
+        return true;
+
+    return m_hasRelevantLoadEventsListener && readyState() == HTMLTrackElement::LOADING;
+}
+
 }
 
 #endif
index c95db6e..756903f 100644 (file)
@@ -28,6 +28,7 @@
 
 #if ENABLE(VIDEO_TRACK)
 
+#include "ActiveDOMObject.h"
 #include "HTMLElement.h"
 #include "LoadableTextTrack.h"
 
@@ -35,7 +36,7 @@ namespace WebCore {
 
 class HTMLMediaElement;
 
-class HTMLTrackElement final : public HTMLElement, public TextTrackClient {
+class HTMLTrackElement final : public HTMLElement, public ActiveDOMObject, public TextTrackClient {
     WTF_MAKE_ISO_ALLOCATED(HTMLTrackElement);
 public:
     static Ref<HTMLTrackElement> create(const QualifiedName&, Document&);
@@ -48,7 +49,7 @@ public:
     bool isDefault() const;
 
     enum ReadyState { NONE = 0, LOADING = 1, LOADED = 2, TRACK_ERROR = 3 };
-    ReadyState readyState();
+    ReadyState readyState() const;
     void setReadyState(ReadyState);
 
     LoadableTextTrack& track();
@@ -61,10 +62,16 @@ public:
     RefPtr<HTMLMediaElement> mediaElement() const;
     const AtomString& mediaElementCrossOriginAttribute() const;
 
+    // ActiveDOMObject.
+    bool hasPendingActivity() const final;
+
 private:
     HTMLTrackElement(const QualifiedName&, Document&);
     virtual ~HTMLTrackElement();
 
+    // ActiveDOMObject.
+    const char* activeDOMObjectName() const final;
+
     void parseAttribute(const QualifiedName&, const AtomString&) final;
 
     InsertedIntoAncestorResult insertedIntoAncestor(InsertionType, ContainerNode&) final;
@@ -72,6 +79,9 @@ private:
 
     bool isURLAttribute(const Attribute&) const final;
 
+    // EventTarget.
+    void eventListenersDidChange() final;
+
     void loadTimerFired();
 
     // TextTrackClient
@@ -86,6 +96,7 @@ private:
 
     RefPtr<LoadableTextTrack> m_track;
     Timer m_loadTimer;
+    bool m_hasRelevantLoadEventsListener { false };
 };
 
 }
index db403da..fe40aa1 100644 (file)
@@ -24,6 +24,7 @@
  */
 
 [
+    ActiveDOMObject,
     Conditional=VIDEO_TRACK,
 ] interface HTMLTrackElement : HTMLElement {
     [CEReactions=NotNeeded] attribute DOMString kind;