Remove TextTrackClient interface to simplify TextTrack code.

Removing the TextTrackClient interface because it was obscuring calls
made to the HTMLMediaElement and wasn't really providing much value.
Calls made to the HTMLMediaElement are much clearer now and it is
easier to follow what happens when tracks are removed from the
TextTrackList.

BUG=249427
TEST=All existing LayoutTests still pass
R=adamk@chromium.org

Review URL: https://codereview.chromium.org/177003025

git-svn-id: svn://svn.chromium.org/blink/trunk@168534 bbb929c8-8fbe-4397-9dbb-9b2b20218538

third_party/WebKit/Source/core/html/HTMLMediaElement.cpp

@@ -333,7 +333,7 @@ HTMLMediaElement::~HTMLMediaElement()
     setShouldDelayLoadEvent(false);
 
     if (m_textTracks)
-        m_textTracks->clearOwnerAndClients();
+        m_textTracks->clearOwner();
 
     if (m_mediaController) {
         m_mediaController->removeMediaElement(this);
@@ -2491,7 +2491,7 @@ void HTMLMediaElement::mediaPlayerDidAddTextTrack(WebInbandTextTrack* webTrack)
 
     // 4.8.10.12.2 Sourcing in-band text tracks
     // 1. Associate the relevant data with a new text track and its corresponding new TextTrack object.
-    RefPtr<InbandTextTrack> textTrack = InbandTextTrack::create(document(), this, webTrack);
+    RefPtr<InbandTextTrack> textTrack = InbandTextTrack::create(document(), webTrack);
 
     // 2. Set the new text track's kind, label, and language based on the semantics of the relevant data,
     // as defined by the relevant specification. If there is no label in that data, then the label must
@@ -2585,7 +2585,7 @@ PassRefPtr<TextTrack> HTMLMediaElement::addTextTrack(const AtomicString& kind, c
 
     // 5. Create a new text track corresponding to the new object, and set its text track kind to kind, its text
     // track label to label, its text track language to language...
-    RefPtr<TextTrack> textTrack = TextTrack::create(document(), this, kind, label, language);
+    RefPtr<TextTrack> textTrack = TextTrack::create(document(), kind, label, language);
 
     // Note, due to side effects when changing track parameters, we have to
     // first append the track to the text track list.

third_party/WebKit/Source/core/html/HTMLMediaElement.h

@@ -73,7 +73,6 @@ typedef Vector<CueInterval> CueList;
 // no longer depends on typecasting a MediaPlayerClient to an HTMLMediaElement.
 
 class HTMLMediaElement : public HTMLElement, public MediaPlayerClient, public ActiveDOMObject, public MediaControllerInterface
-    , private TextTrackClient
 {
 public:
     static blink::WebMimeRegistry::SupportsType supportsType(const ContentType&, const String& keySystem = String());
@@ -233,13 +232,12 @@ public:
     void updateTextTrackDisplay();
     void textTrackReadyStateChanged(TextTrack*);
 
-    // TextTrackClient
-    virtual void textTrackKindChanged(TextTrack*) OVERRIDE FINAL;
-    virtual void textTrackModeChanged(TextTrack*) OVERRIDE FINAL;
-    virtual void textTrackAddCues(TextTrack*, const TextTrackCueList*) OVERRIDE FINAL;
-    virtual void textTrackRemoveCues(TextTrack*, const TextTrackCueList*) OVERRIDE FINAL;
-    virtual void textTrackAddCue(TextTrack*, PassRefPtr<TextTrackCue>) OVERRIDE FINAL;
-    virtual void textTrackRemoveCue(TextTrack*, PassRefPtr<TextTrackCue>) OVERRIDE FINAL;
+    void textTrackKindChanged(TextTrack*);
+    void textTrackModeChanged(TextTrack*);
+    void textTrackAddCues(TextTrack*, const TextTrackCueList*);
+    void textTrackRemoveCues(TextTrack*, const TextTrackCueList*);
+    void textTrackAddCue(TextTrack*, PassRefPtr<TextTrackCue>);
+    void textTrackRemoveCue(TextTrack*, PassRefPtr<TextTrackCue>);
 
     // EventTarget function.
     // Both Node (via HTMLElement) and ActiveDOMObject define this method, which

third_party/WebKit/Source/core/html/HTMLTrackElement.cpp

@@ -61,7 +61,7 @@ inline HTMLTrackElement::HTMLTrackElement(Document& document)
 HTMLTrackElement::~HTMLTrackElement()
 {
     if (m_track)
-        m_track->clearClient();
+        m_track->clearTrackElement();
 }
 
 PassRefPtr<HTMLTrackElement> HTMLTrackElement::create(Document& document)
@@ -264,46 +264,6 @@ const AtomicString& HTMLTrackElement::mediaElementCrossOriginAttribute() const
     return nullAtom;
 }
 
-void HTMLTrackElement::textTrackKindChanged(TextTrack* track)
-{
-    if (HTMLMediaElement* parent = mediaElement())
-        return parent->textTrackKindChanged(track);
-}
-
-void HTMLTrackElement::textTrackModeChanged(TextTrack* track)
-{
-    // Since we've moved to a new parent, we may now be able to load.
-    if (readyState() == HTMLTrackElement::NONE)
-        scheduleLoad();
-
-    if (HTMLMediaElement* parent = mediaElement())
-        return parent->textTrackModeChanged(track);
-}
-
-void HTMLTrackElement::textTrackAddCues(TextTrack* track, const TextTrackCueList* cues)
-{
-    if (HTMLMediaElement* parent = mediaElement())
-        return parent->textTrackAddCues(track, cues);
-}
-
-void HTMLTrackElement::textTrackRemoveCues(TextTrack* track, const TextTrackCueList* cues)
-{
-    if (HTMLMediaElement* parent = mediaElement())
-        return parent->textTrackRemoveCues(track, cues);
-}
-
-void HTMLTrackElement::textTrackAddCue(TextTrack* track, PassRefPtr<TextTrackCue> cue)
-{
-    if (HTMLMediaElement* parent = mediaElement())
-        return parent->textTrackAddCue(track, cue);
-}
-
-void HTMLTrackElement::textTrackRemoveCue(TextTrack* track, PassRefPtr<TextTrackCue> cue)
-{
-    if (HTMLMediaElement* parent = mediaElement())
-        return parent->textTrackRemoveCue(track, cue);
-}
-
 HTMLMediaElement* HTMLTrackElement::mediaElement() const
 {
     Element* parent = parentElement();

third_party/WebKit/Source/core/html/HTMLTrackElement.h

@@ -34,7 +34,7 @@ namespace WebCore {
 
 class HTMLMediaElement;
 
-class HTMLTrackElement FINAL : public HTMLElement, public TextTrackClient {
+class HTMLTrackElement FINAL : public HTMLElement {
 public:
     static PassRefPtr<HTMLTrackElement> create(Document&);
 
@@ -69,14 +69,6 @@ private:
 
     HTMLMediaElement* mediaElement() const;
 
-    // TextTrackClient
-    virtual void textTrackModeChanged(TextTrack*) OVERRIDE;
-    virtual void textTrackKindChanged(TextTrack*) OVERRIDE;
-    virtual void textTrackAddCues(TextTrack*, const TextTrackCueList*) OVERRIDE;
-    virtual void textTrackRemoveCues(TextTrack*, const TextTrackCueList*) OVERRIDE;
-    virtual void textTrackAddCue(TextTrack*, PassRefPtr<TextTrackCue>) OVERRIDE;
-    virtual void textTrackRemoveCue(TextTrack*, PassRefPtr<TextTrackCue>) OVERRIDE;
-
     LoadableTextTrack* ensureTrack();
     bool canLoadUrl(const KURL&);
 

third_party/WebKit/Source/core/html/track/InbandTextTrack.cpp

@@ -38,13 +38,13 @@ using blink::WebString;
 
 namespace WebCore {
 
-PassRefPtr<InbandTextTrack> InbandTextTrack::create(Document& document, TextTrackClient* client, WebInbandTextTrack* webTrack)
+PassRefPtr<InbandTextTrack> InbandTextTrack::create(Document& document, WebInbandTextTrack* webTrack)
 {
-    return adoptRef(new InbandTextTrack(document, client, webTrack));
+    return adoptRef(new InbandTextTrack(document, webTrack));
 }
 
-InbandTextTrack::InbandTextTrack(Document& document, TextTrackClient* client, WebInbandTextTrack* webTrack)
-    : TextTrack(document, client, emptyAtom, webTrack->label(), webTrack->language(), webTrack->id(), InBand)
+InbandTextTrack::InbandTextTrack(Document& document, WebInbandTextTrack* webTrack)
+    : TextTrack(document, emptyAtom, webTrack->label(), webTrack->language(), webTrack->id(), InBand)
     , m_webTrack(webTrack)
 {
     m_webTrack->setClient(this);
@@ -93,7 +93,6 @@ void InbandTextTrack::setTrackList(TextTrackList* trackList)
     ASSERT(m_webTrack);
     m_webTrack->setClient(0);
     m_webTrack = 0;
-    clearClient();
 }
 
 void InbandTextTrack::addWebVTTCue(double start, double end, const WebString& id, const WebString& content, const WebString& settings)

third_party/WebKit/Source/core/html/track/InbandTextTrack.h

@@ -43,14 +43,14 @@ class TextTrackCue;
 
 class InbandTextTrack FINAL : public TextTrack, public blink::WebInbandTextTrackClient {
 public:
-    static PassRefPtr<InbandTextTrack> create(Document&, TextTrackClient*, blink::WebInbandTextTrack*);
+    static PassRefPtr<InbandTextTrack> create(Document&, blink::WebInbandTextTrack*);
     virtual ~InbandTextTrack();
 
     size_t inbandTrackIndex();
     virtual void setTrackList(TextTrackList*) OVERRIDE FINAL;
 
 private:
-    InbandTextTrack(Document&, TextTrackClient*, blink::WebInbandTextTrack*);
+    InbandTextTrack(Document&, blink::WebInbandTextTrack*);
 
     virtual void addWebVTTCue(double, double, const blink::WebString&, const blink::WebString&, const blink::WebString&) OVERRIDE;
 

third_party/WebKit/Source/core/html/track/LoadableTextTrack.cpp

@@ -27,6 +27,7 @@
 #include "core/html/track/LoadableTextTrack.h"
 
 #include "core/dom/ElementTraversal.h"
+#include "core/html/HTMLMediaElement.h"
 #include "core/html/HTMLTrackElement.h"
 #include "core/html/track/TextTrackCueList.h"
 #include "core/html/track/vtt/VTTRegionList.h"
@@ -36,7 +37,7 @@ namespace WebCore {
 using namespace HTMLNames;
 
 LoadableTextTrack::LoadableTextTrack(HTMLTrackElement* track)
-    : TextTrack(track->document(), track, emptyAtom, emptyAtom, emptyAtom, emptyAtom, TrackElement)
+    : TextTrack(track->document(), emptyAtom, emptyAtom, emptyAtom, emptyAtom, TrackElement)
     , m_trackElement(track)
     , m_loadTimer(this, &LoadableTextTrack::loadTimerFired)
     , m_isDefault(false)
@@ -45,12 +46,22 @@ LoadableTextTrack::LoadableTextTrack(HTMLTrackElement* track)
 
 LoadableTextTrack::~LoadableTextTrack()
 {
+    ASSERT(!m_trackElement);
 }
 
-void LoadableTextTrack::clearClient()
+void LoadableTextTrack::clearTrackElement()
 {
     m_trackElement = 0;
-    TextTrack::clearClient();
+}
+
+void LoadableTextTrack::setMode(const AtomicString& mode)
+{
+    TextTrack::setMode(mode);
+    if (!m_trackElement)
+        return;
+
+    if (m_trackElement->readyState() == HTMLTrackElement::NONE)
+        m_trackElement->scheduleLoad();
 }
 
 void LoadableTextTrack::scheduleLoad(const KURL& url)
@@ -111,8 +122,8 @@ void LoadableTextTrack::newCuesAvailable(TextTrackLoader* loader)
         m_cues->add(newCues[i]);
     }
 
-    if (client())
-        client()->textTrackAddCues(this, m_cues.get());
+    if (mediaElement())
+        mediaElement()->textTrackAddCues(this, m_cues.get());
 }
 
 void LoadableTextTrack::cueLoadingCompleted(TextTrackLoader* loader, bool loadingFailed)
@@ -157,4 +168,3 @@ size_t LoadableTextTrack::trackElementIndex()
 }
 
 } // namespace WebCore
-

third_party/WebKit/Source/core/html/track/LoadableTextTrack.h

@@ -45,11 +45,12 @@ public:
 
     void scheduleLoad(const KURL&);
 
-    // This shadows TextTrack::clearClient, but need not be virtual.
-    void clearClient();
+    // TextTrack method.
+    virtual void setMode(const AtomicString&) OVERRIDE;
 
     size_t trackElementIndex();
     HTMLTrackElement* trackElement() { return m_trackElement; }
+    void clearTrackElement();
 
     virtual bool isDefault() const OVERRIDE { return m_isDefault; }
     virtual void setIsDefault(bool isDefault) OVERRIDE  { m_isDefault = isDefault; }

third_party/WebKit/Source/core/html/track/TextTrack.cpp

@@ -95,14 +95,13 @@ const AtomicString& TextTrack::showingKeyword()
     return ended;
 }
 
-TextTrack::TextTrack(Document& document, TextTrackClient* client, const AtomicString& kind, const AtomicString& label, const AtomicString& language, const AtomicString& id, TextTrackType type)
+TextTrack::TextTrack(Document& document, const AtomicString& kind, const AtomicString& label, const AtomicString& language, const AtomicString& id, TextTrackType type)
     : TrackBase(TrackBase::TextTrack, label, language, id)
     , m_cues(nullptr)
     , m_regions(nullptr)
     , m_document(&document)
     , m_trackList(0)
     , m_mode(disabledKeyword())
-    , m_client(client)
     , m_trackType(type)
     , m_readinessState(NotLoaded)
     , m_trackIndex(invalidTrackIndex)
@@ -115,10 +114,9 @@ TextTrack::TextTrack(Document& document, TextTrackClient* client, const AtomicSt
 
 TextTrack::~TextTrack()
 {
-    if (m_cues) {
-        if (m_client)
-            m_client->textTrackRemoveCues(this, m_cues.get());
+    ASSERT(!m_trackList);
 
+    if (m_cues) {
         for (size_t i = 0; i < m_cues->length(); ++i)
             m_cues->item(i)->setTrack(0);
     }
@@ -127,7 +125,6 @@ TextTrack::~TextTrack()
         for (size_t i = 0; i < m_regions->length(); ++i)
             m_regions->item(i)->setTrack(0);
     }
-    clearClient();
 }
 
 bool TextTrack::isValidKindKeyword(const AtomicString& value)
@@ -148,13 +145,11 @@ bool TextTrack::isValidKindKeyword(const AtomicString& value)
 
 void TextTrack::setTrackList(TextTrackList* trackList)
 {
-    // NOTE: We are using m_trackList->owner() instead of m_client here because
-    // when a HTMLTrackElement is reparented, HTMLTrackElement::textTrackRemoveCues()
-    // will forward the call to the new parent instead of the element the track is being
-    // removed from.
-    if (!trackList && m_trackList && m_trackList->owner() && m_cues)
-        m_trackList->owner()->textTrackRemoveCues(this, m_cues.get());
+    if (!trackList && mediaElement() && m_cues)
+        mediaElement()->textTrackRemoveCues(this, m_cues.get());
+
     m_trackList = trackList;
+    invalidateTrackIndex();
 }
 
 void TextTrack::setKind(const AtomicString& newKind)
@@ -162,8 +157,8 @@ void TextTrack::setKind(const AtomicString& newKind)
     AtomicString oldKind = kind();
     TrackBase::setKind(newKind);
 
-    if (m_client && oldKind != kind())
-        m_client->textTrackKindChanged(this);
+    if (mediaElement() && oldKind != kind())
+        mediaElement()->textTrackKindChanged(this);
 }
 
 void TextTrack::setMode(const AtomicString& mode)
@@ -177,8 +172,8 @@ void TextTrack::setMode(const AtomicString& mode)
 
     // If mode changes to disabled, remove this track's cues from the client
     // because they will no longer be accessible from the cues() function.
-    if (mode == disabledKeyword() && m_client && m_cues)
-        m_client->textTrackRemoveCues(this, m_cues.get());
+    if (mode == disabledKeyword() && mediaElement() && m_cues)
+        mediaElement()->textTrackRemoveCues(this, m_cues.get());
 
     if (mode != showingKeyword() && m_cues)
         for (size_t i = 0; i < m_cues->length(); ++i)
@@ -186,8 +181,8 @@ void TextTrack::setMode(const AtomicString& mode)
 
     m_mode = mode;
 
-    if (m_client)
-        m_client->textTrackModeChanged(this);
+    if (mediaElement())
+        mediaElement()->textTrackModeChanged(this);
 }
 
 TextTrackCueList* TextTrack::cues()
@@ -207,8 +202,8 @@ void TextTrack::removeAllCues()
     if (!m_cues)
         return;
 
-    if (m_client)
-        m_client->textTrackRemoveCues(this, m_cues.get());
+    if (mediaElement())
+        mediaElement()->textTrackRemoveCues(this, m_cues.get());
 
     for (size_t i = 0; i < m_cues->length(); ++i)
         m_cues->item(i)->setTrack(0);
@@ -254,8 +249,8 @@ void TextTrack::addCue(PassRefPtr<TextTrackCue> prpCue)
     cue->setTrack(this);
     ensureTextTrackCueList()->add(cue);
 
-    if (m_client)
-        m_client->textTrackAddCue(this, cue.get());
+    if (mediaElement())
+        mediaElement()->textTrackAddCue(this, cue.get());
 }
 
 void TextTrack::removeCue(TextTrackCue* cue, ExceptionState& exceptionState)
@@ -281,8 +276,8 @@ void TextTrack::removeCue(TextTrackCue* cue, ExceptionState& exceptionState)
     }
 
     cue->setTrack(0);
-    if (m_client)
-        m_client->textTrackRemoveCue(this, cue);
+    if (mediaElement())
+        mediaElement()->textTrackRemoveCue(this, cue);
 }
 
 VTTRegionList* TextTrack::ensureVTTRegionList()
@@ -358,24 +353,24 @@ void TextTrack::removeRegion(VTTRegion* region, ExceptionState &exceptionState)
 
 void TextTrack::cueWillChange(TextTrackCue* cue)
 {
-    if (!m_client)
+    if (!mediaElement())
         return;
 
     // The cue may need to be repositioned in the media element's interval tree, may need to
     // be re-rendered, etc, so remove it before the modification...
-    m_client->textTrackRemoveCue(this, cue);
+    mediaElement()->textTrackRemoveCue(this, cue);
 }
 
 void TextTrack::cueDidChange(TextTrackCue* cue)
 {
-    if (!m_client)
+    if (!mediaElement())
         return;
 
     // Make sure the TextTrackCueList order is up-to-date.
     ensureTextTrackCueList()->updateCueIndex(cue);
 
     // ... and add it back again.
-    m_client->textTrackAddCue(this, cue);
+    mediaElement()->textTrackAddCue(this, cue);
 }
 
 int TextTrack::trackIndex()
@@ -433,4 +428,9 @@ ExecutionContext* TextTrack::executionContext() const
     return m_document;
 }
 
+HTMLMediaElement* TextTrack::mediaElement()
+{
+    return m_trackList ? m_trackList->owner() : 0;
+}
+
 } // namespace WebCore

third_party/WebKit/Source/core/html/track/TextTrack.h

@@ -44,23 +44,12 @@ class TextTrackList;
 class VTTRegion;
 class VTTRegionList;
 
-class TextTrackClient {
-public:
-    virtual ~TextTrackClient() { }
-    virtual void textTrackKindChanged(TextTrack*) = 0;
-    virtual void textTrackModeChanged(TextTrack*) = 0;
-    virtual void textTrackAddCues(TextTrack*, const TextTrackCueList*) = 0;
-    virtual void textTrackRemoveCues(TextTrack*, const TextTrackCueList*) = 0;
-    virtual void textTrackAddCue(TextTrack*, PassRefPtr<TextTrackCue>) = 0;
-    virtual void textTrackRemoveCue(TextTrack*, PassRefPtr<TextTrackCue>) = 0;
-};
-
 class TextTrack : public TrackBase, public ScriptWrappable, public EventTargetWithInlineData {
     REFCOUNTED_EVENT_TARGET(TrackBase);
 public:
-    static PassRefPtr<TextTrack> create(Document& document, TextTrackClient* client, const AtomicString& kind, const AtomicString& label, const AtomicString& language)
+    static PassRefPtr<TextTrack> create(Document& document, const AtomicString& kind, const AtomicString& label, const AtomicString& language)
     {
-        return adoptRef(new TextTrack(document, client, kind, label, language, emptyAtom, AddTrack));
+        return adoptRef(new TextTrack(document, kind, label, language, emptyAtom, AddTrack));
     }
     virtual ~TextTrack();
 
@@ -81,7 +70,7 @@ public:
     static const AtomicString& showingKeyword();
 
     AtomicString mode() const { return m_mode; }
-    void setMode(const AtomicString&);
+    virtual void setMode(const AtomicString&);
 
     enum ReadinessState { NotLoaded = 0, Loading = 1, Loaded = 2, FailedToLoad = 3 };
     ReadinessState readinessState() const { return m_readinessState; }
@@ -90,8 +79,7 @@ public:
     TextTrackCueList* cues();
     TextTrackCueList* activeCues() const;
 
-    void clearClient() { m_client = 0; }
-    TextTrackClient* client() { return m_client; }
+    HTMLMediaElement* mediaElement();
 
     void addCue(PassRefPtr<TextTrackCue>);
     void removeCue(TextTrackCue*, ExceptionState&);
@@ -129,7 +117,7 @@ public:
     virtual ExecutionContext* executionContext() const OVERRIDE;
 
 protected:
-    TextTrack(Document&, TextTrackClient*, const AtomicString& kind, const AtomicString& label, const AtomicString& language, const AtomicString& id, TextTrackType);
+    TextTrack(Document&, const AtomicString& kind, const AtomicString& label, const AtomicString& language, const AtomicString& id, TextTrackType);
 
     virtual bool isValidKind(const AtomicString& kind) const OVERRIDE { return isValidKindKeyword(kind); }
     virtual AtomicString defaultKind() const OVERRIDE { return subtitlesKeyword(); }
@@ -147,7 +135,6 @@ private:
 
     TextTrackList* m_trackList;
     AtomicString m_mode;
-    TextTrackClient* m_client;
     TextTrackType m_trackType;
     ReadinessState m_readinessState;
     int m_trackIndex;

third_party/WebKit/Source/core/html/track/TextTrackList.cpp

@@ -46,7 +46,13 @@ TextTrackList::TextTrackList(HTMLMediaElement* owner)
 
 TextTrackList::~TextTrackList()
 {
+    ASSERT(!m_owner);
+
     m_asyncEventQueue->close();
+
+    for (unsigned i = 0; i < length(); ++i) {
+        item(i)->setTrackList(0);
+    }
 }
 
 unsigned TextTrackList::length() const
@@ -229,7 +235,6 @@ void TextTrackList::remove(TextTrack* track)
 void TextTrackList::removeAllInbandTracks()
 {
     for (unsigned i = 0; i < m_inbandTracks.size(); ++i) {
-        m_inbandTracks[i]->invalidateTrackIndex();
         m_inbandTracks[i]->setTrackList(0);
     }
     m_inbandTracks.clear();
@@ -262,12 +267,9 @@ ExecutionContext* TextTrackList::executionContext() const
     return m_owner->executionContext();
 }
 
-void TextTrackList::clearOwnerAndClients()
+void TextTrackList::clearOwner()
 {
     m_owner = 0;
-
-    for (unsigned i = 0; i < length(); ++i)
-        item(i)->clearClient();
 }
 
 void TextTrackList::scheduleTrackEvent(const AtomicString& eventName, PassRefPtr<TextTrack> track)

third_party/WebKit/Source/core/html/track/TextTrackList.h

@@ -67,7 +67,7 @@ public:
     DEFINE_ATTRIBUTE_EVENT_LISTENER(change);
     DEFINE_ATTRIBUTE_EVENT_LISTENER(removetrack);
 
-    void clearOwnerAndClients();
+    void clearOwner();
     HTMLMediaElement* owner() const;
 
     void scheduleChangeEvent();