Changeset 101185 in webkit

Timestamp:

Nov 25, 2011, 8:28:39 PM (15 years ago)

Author:

eric.carlson@apple.com

Message:

Implement addCue and removeCue in TextTrack
​https://bugs.webkit.org/show_bug.cgi?id=72554

Reviewed by Darin Adler.

Source/WebCore:

Test: media/track/track-add-remove-cue.html

• html/HTMLTrackElement.cpp:

(WebCore::HTMLTrackElement::ensureTrack): Go ahead and allocate a Track even if the feature

is disabled, it just won't load anything.

(WebCore::HTMLTrackElement::scheduleLoad): Early return if the featue is disabled.
(WebCore::HTMLTrackElement::canLoadUrl): Ditto.

• html/LoadableTextTrack.cpp:

(WebCore::LoadableTextTrack::newCuesAvailable): Add new cues one at a time because

cues->add(Vector<TextTrackCue*>&) is gone.

• html/TextTrack.cpp:

(WebCore::TextTrack::addCue): Implement.
(WebCore::TextTrack::removeCue): Ditto.

• html/TextTrack.h:

(WebCore::TextTrackCue::TextTrackCue): Initialize every member variable.
(WebCore::TextTrackCue::track): m_track is now a RefPtr.
(WebCore::TextTrackCue::setTrack): Ditto.

• html/TextTrackCue.h:

• html/TextTrackCueList.cpp:

(WebCore::TextTrackCueList::add): Don't ignore out of order cues, the spec text is not

a conformance requirement. Return bool to indicate success or failure.

(WebCore::TextTrackCueList::remove): Return bool to indicate success or failure.

• html/TextTrackCueList.h:

• loader/TextTrackLoader.cpp:

(WebCore::TextTrackLoader::notifyFinished): Don't change m_state once it is set to Failed.

LayoutTests:

• media/track/track-add-remove-cue-expected.txt: Added.
• media/track/track-add-remove-cue.html: Added.
• media/track/track-webvtt-tc012-out-of-order-expected.txt: Removed.
• media/track/track-webvtt-tc012-out-of-order.html: Removed.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/media/track/track-add-remove-cue-expected.txt (added)
• LayoutTests/media/track/track-add-remove-cue.html (added)
• LayoutTests/media/track/track-webvtt-tc012-out-of-order-expected.txt (deleted)
• LayoutTests/media/track/track-webvtt-tc012-out-of-order.html (deleted)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLTrackElement.cpp (modified) (3 diffs)
• Source/WebCore/html/LoadableTextTrack.cpp (modified) (1 diff)
• Source/WebCore/html/TextTrack.cpp (modified) (1 diff)
• Source/WebCore/html/TextTrack.h (modified) (1 diff)
• Source/WebCore/html/TextTrackCue.cpp (modified) (2 diffs)
• Source/WebCore/html/TextTrackCue.h (modified) (3 diffs)
• Source/WebCore/html/TextTrackCueList.cpp (modified) (2 diffs)
• Source/WebCore/html/TextTrackCueList.h (modified) (1 diff)
• Source/WebCore/loader/TextTrackLoader.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2011-11-25  Eric Carlson  <eric.carlson@apple.com>
+
+        Implement addCue and removeCue in TextTrack
+        https://bugs.webkit.org/show_bug.cgi?id=72554
+
+        Reviewed by Darin Adler.
+
+        * media/track/track-add-remove-cue-expected.txt: Added.
+        * media/track/track-add-remove-cue.html: Added.
+        * media/track/track-webvtt-tc012-out-of-order-expected.txt: Removed.
+        * media/track/track-webvtt-tc012-out-of-order.html: Removed.
+
 2011-11-25  Kentaro Hara  <haraken@chromium.org>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2011-11-25  Eric Carlson  <eric.carlson@apple.com>
+
+        Implement addCue and removeCue in TextTrack
+        https://bugs.webkit.org/show_bug.cgi?id=72554
+
+        Reviewed by Darin Adler.
+
+        Test: media/track/track-add-remove-cue.html
+
+        * html/HTMLTrackElement.cpp:
+        (WebCore::HTMLTrackElement::ensureTrack): Go ahead and allocate a Track even if the feature 
+            is disabled, it just won't load anything.
+        (WebCore::HTMLTrackElement::scheduleLoad): Early return if the featue is disabled.
+        (WebCore::HTMLTrackElement::canLoadUrl): Ditto.
+
+        * html/LoadableTextTrack.cpp:
+        (WebCore::LoadableTextTrack::newCuesAvailable): Add new cues one at a time because 
+            cues->add(Vector<TextTrackCue*>&) is gone.
+
+        * html/TextTrack.cpp:
+        (WebCore::TextTrack::addCue): Implement.
+        (WebCore::TextTrack::removeCue): Ditto.
+        * html/TextTrack.h:
+
+        (WebCore::TextTrackCue::TextTrackCue): Initialize every member variable.
+        (WebCore::TextTrackCue::track): m_track is now a RefPtr.
+        (WebCore::TextTrackCue::setTrack): Ditto.
+        * html/TextTrackCue.h:
+
+        * html/TextTrackCueList.cpp:
+        (WebCore::TextTrackCueList::add): Don't ignore out of order cues, the spec text is not 
+            a conformance requirement. Return bool to indicate success or failure.
+        (WebCore::TextTrackCueList::remove): Return bool to indicate success or failure.
+        * html/TextTrackCueList.h:
+
+        * loader/TextTrackLoader.cpp:
+        (WebCore::TextTrackLoader::notifyFinished): Don't change m_state once it is set to Failed.
+
 2011-11-25  Kentaro Hara  <haraken@chromium.org>
 

trunk/Source/WebCore/html/HTMLTrackElement.cpp

@@ -159 +159 @@
 LoadableTextTrack* HTMLTrackElement::ensureTrack()
 {
-    if (!RuntimeEnabledFeatures::webkitVideoTrackEnabled())
-        return 0;
-
     if (!m_track) {
         // The kind attribute is an enumerated attribute, limited only to know values. It defaults to 'subtitles' if missing or invalid.
@@ -184 +181 @@
 void HTMLTrackElement::scheduleLoad()
 {
+    if (!RuntimeEnabledFeatures::webkitVideoTrackEnabled())
+        return;
+
     if (!mediaElement())
         return;
@@ -195 +195 @@
 bool HTMLTrackElement::canLoadUrl(LoadableTextTrack*, const KURL& url)
 {
+    if (!RuntimeEnabledFeatures::webkitVideoTrackEnabled())
+        return false;
+
     HTMLMediaElement* parent = mediaElement();
     if (!parent)

trunk/Source/WebCore/html/LoadableTextTrack.cpp

@@ -86 +86 @@
     m_loader->getNewCues(newCues);
 
-    for (size_t i = 0; i < newCues.size(); ++i)
-        newCues[i]->setTrack(this);
-
     if (!m_cues)
         m_cues = TextTrackCueList::create();    
-    m_cues->add(newCues);
+
+    for (size_t i = 0; i < newCues.size(); ++i) {
+        newCues[i]->setTrack(this);
+        m_cues->add(newCues[i]);
+    }
 
     if (client())

trunk/Source/WebCore/html/TextTrack.cpp

@@ -167 +167 @@
 }
 
-void TextTrack::addCue(PassRefPtr<TextTrackCue>, ExceptionCode&)
-{
-    // FIXME(62890): Implement.
+void TextTrack::addCue(PassRefPtr<TextTrackCue> prpCue, ExceptionCode& ec)
+{
+    if (!prpCue)
+        return;
+
+    RefPtr<TextTrackCue> cue = prpCue;
+
+    // 4.8.10.12.4 Text track API
+
+    // The addCue(cue) method of TextTrack objects, when invoked, must run the following steps:
+
+    // 1. If the given cue is already associated with a text track other than 
+    // the method's TextTrack object's text track, then throw an InvalidStateError
+    // exception and abort these steps.
+    TextTrack* cueTrack = cue->track();
+    if (cueTrack && cueTrack != this) {
+        ec = INVALID_STATE_ERR;
+        return;
+    }
+
+    // 2. Associate cue with the method's TextTrack object's text track, if it is 
+    // not currently associated with a text track.
+    cue->setTrack(this);
+
+    // 3. If the given cue is already listed in the method's TextTrack object's text
+    // track's text track list of cues, then throw an InvalidStateError exception.
+    // 4. Add cue to the method's TextTrack object's text track's text track list of cues.
+    if (!m_cues->add(cue)) {
+        ec = INVALID_STATE_ERR;
+        return;
+    }
     
-}
-
-void TextTrack::removeCue(PassRefPtr<TextTrackCue>, ExceptionCode&)
-{
-    // FIXME(62890): Implement.
-}
-
-void TextTrack::newCuesLoaded()
-{
-    // FIXME(62890): Implement.
-}
-
-void TextTrack::fetchNewestCues(Vector<TextTrackCue*>&)
-{
-    // FIXME(62890): Implement.
+    if (m_client)
+        m_client->textTrackAddCue(this, cue.get());
+}
+
+void TextTrack::removeCue(TextTrackCue* cue, ExceptionCode& ec)
+{
+    if (!cue)
+        return;
+
+    // 4.8.10.12.4 Text track API
+
+    // The removeCue(cue) method of TextTrack objects, when invoked, must run the following steps:
+
+    // 1. If the given cue is not associated with the method's TextTrack 
+    // object's text track, then throw an InvalidStateError exception.
+    if (cue->track() != this) {
+        ec = INVALID_STATE_ERR;
+        return;
+    }
+    
+    // 2. If the given cue is not currently listed in the method's TextTrack 
+    // object's text track's text track list of cues, then throw a NotFoundError exception.
+    // 3. Remove cue from the method's TextTrack object's text track's text track list of cues.
+    if (!m_cues->remove(cue)) {
+        ec = INVALID_STATE_ERR;
+        return;
+    }
+
+    cue->setTrack(0);
+    if (m_client)
+        m_client->textTrackRemoveCue(this, cue);
 }
 

trunk/Source/WebCore/html/TextTrack.h

@@ -94 +94 @@
 
     void addCue(PassRefPtr<TextTrackCue>, ExceptionCode&);
-    void removeCue(PassRefPtr<TextTrackCue>, ExceptionCode&);
-    
-    void newCuesLoaded();
-    void fetchNewestCues(Vector<TextTrackCue*>&);
+    void removeCue(TextTrackCue*, ExceptionCode&);
     
     virtual void fireCueChangeEvent();

trunk/Source/WebCore/html/TextTrackCue.cpp

@@ -48 +48 @@
     , m_endTime(end)
     , m_content(content)
-    , m_pauseOnExit(pauseOnExit)
     , m_writingDirection(Horizontal)
-    , m_snapToLines(true)
     , m_linePosition(-1)
     , m_textPosition(50)
     , m_cueSize(100)
     , m_cueAlignment(Middle)
+    , m_scriptExecutionContext(context)
     , m_isActive(false)
-    , m_scriptExecutionContext(context)
+    , m_pauseOnExit(pauseOnExit)
+    , m_snapToLines(true)
 {
     parseSettings(settings);
@@ -67 +67 @@
 TextTrack* TextTrackCue::track() const
 {
-    return m_track;
-}
-
-void TextTrackCue::setTrack(TextTrack* track)
+    return m_track.get();
+}
+
+void TextTrackCue::setTrack(PassRefPtr<TextTrack>track)
 {
     m_track = track;

trunk/Source/WebCore/html/TextTrackCue.h

@@ -58 +58 @@
 
     TextTrack* track() const;
-    void setTrack(TextTrack*);
+    void setTrack(PassRefPtr<TextTrack>);
 
     String id() const;
@@ -101 +101 @@
     virtual void derefEventTarget() { deref(); }
     
-    TextTrack* m_track;
-    
     String m_id;
     double m_startTime;
     double m_endTime;
     String m_content;
-    bool m_pauseOnExit;
     Direction m_writingDirection;
-    bool m_snapToLines;
     int m_linePosition;
     int m_textPosition;
@@ -115 +111 @@
     Alignment m_cueAlignment;
     RefPtr<DocumentFragment> m_documentFragment;
+    RefPtr<TextTrack> m_track;
+
+    EventTargetData m_eventTargetData;
+    ScriptExecutionContext* m_scriptExecutionContext;
 
     bool m_isActive;
-    
-    EventTargetData m_eventTargetData;
-    ScriptExecutionContext* m_scriptExecutionContext;
+    bool m_pauseOnExit;
+    bool m_snapToLines;
 };
 

trunk/Source/WebCore/html/TextTrackCueList.cpp

@@ -71 +71 @@
 }
 
-void TextTrackCueList::add(const Vector<RefPtr<TextTrackCue> >& newCues)
+bool TextTrackCueList::add(PassRefPtr<TextTrackCue> cue)
 {
-    for (size_t i = 0; i < newCues.size(); ++i)
-        add(newCues[i]);
+    return add(cue, 0, m_list.size());
 }
 
-void TextTrackCueList::add(PassRefPtr<TextTrackCue> cue)
-{
-    // WebVTT cue timings
-    // 1. The time represented by this WebVTT timestamp must be greater than or equal
-    // to the start time offsets of all previous cues in the file.
-    // http://www.whatwg.org/specs/web-apps/current-work/#webvtt-cue-timings
-    // Note: because this requirement is specific to WebVTT, we may want to check first
-    // whether the cues in this list came from a WebVTT file.
-    if (!m_list.isEmpty() && cue->startTime() < m_list.last()->startTime())
-        return;
-    add(cue, 0, m_list.size());
-}
-
-void TextTrackCueList::add(PassRefPtr<TextTrackCue> cue, size_t start, size_t end)
+bool TextTrackCueList::add(PassRefPtr<TextTrackCue> prpCue, size_t start, size_t end)
 {
     ASSERT(start <= m_list.size());
@@ -97 +83 @@
     // Maintain text track cue order:
     // http://www.whatwg.org/specs/web-apps/current-work/#text-track-cue-order
-    RefPtr<TextTrackCue> newCue = cue;
+    RefPtr<TextTrackCue> cue = prpCue;
     if (start == end) {
-       m_list.insert(start, newCue);
-       return;
+        if (!m_list.isEmpty() && (m_list[start - 1].get() == cue.get()))
+            return false;
+
+       m_list.insert(start, cue);
+       return true;
     }
 
     size_t index = (start + end) / 2;
-    if (newCue->startTime() < m_list[index]->startTime() || (newCue->startTime() == m_list[index]->startTime() && newCue->endTime() > m_list[index]->endTime()))
-        add(newCue.release(), start, index);
-    else
-        add(newCue.release(), index + 1, end);
+    if (cue->startTime() < m_list[index]->startTime() || (cue->startTime() == m_list[index]->startTime() && cue->endTime() > m_list[index]->endTime()))
+        return add(cue.release(), start, index);
+
+    return add(cue.release(), index + 1, end);
 }
 
-void TextTrackCueList::remove(TextTrackCue* cue)
+bool TextTrackCueList::remove(TextTrackCue* cue)
 {
     size_t index = m_list.find(cue);
     if (index == notFound)
-        return;
+        return false;
+
     cue->setIsActive(false);
     m_list.remove(index);
+    return true;
 }
 

trunk/Source/WebCore/html/TextTrackCueList.h

@@ -50 +50 @@
     TextTrackCueList* activeCues();
 
-    void add(PassRefPtr<TextTrackCue>);
-    void add(const Vector<RefPtr<TextTrackCue> >&);
-    void remove(TextTrackCue*);
+    bool add(PassRefPtr<TextTrackCue>);
+    bool remove(TextTrackCue*);
     bool contains(TextTrackCue*) const;
 
 private:
     TextTrackCueList();
-    void add(PassRefPtr<TextTrackCue>, size_t, size_t);
+    bool add(PassRefPtr<TextTrackCue>, size_t, size_t);
     void clear();
     

trunk/Source/WebCore/loader/TextTrackLoader.cpp

@@ -146 +146 @@
     processNewCueData(resource);
 
-    m_state = resource->errorOccurred() ? Failed : Finished;
+    if (m_state != Failed)
+        m_state = resource->errorOccurred() ? Failed : Finished;
 
     if (!m_cueLoadTimer.isActive())