Use TextTrackKind enum in HTMLMediaElement.addTextTrack IDL signature

Source/core/html/HTMLMediaElement.cpp

 /*
  * Copyright (C) 2007, 2008, 2009, 2010, 2011, 2012, 2013 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 2728 matching lines @@
     }
 
     m_audioTracks->removeAll();
     m_videoTracks->removeAll();
 
     m_audioTracksTimer.stop();
 }
 
 PassRefPtrWillBeRawPtr<TextTrack> HTMLMediaElement::addTextTrack(const AtomicString& kind, const AtomicString& label, const AtomicString& language, ExceptionState& exceptionState)
 {
-    // 4.8.10.12.4 Text track API
-    // The addTextTrack(kind, label, language) method of media elements, when invoked, must run the following steps:
+    // https://html.spec.whatwg.org/multipage/embedded-content.html#dom-media-addtexttrack
 
-    // 1. If kind is not one of the following strings, then throw a SyntaxError exception and abort these steps
-    if (!TextTrack::isValidKindKeyword(kind)) {
-        exceptionState.throwDOMException(SyntaxError, "The 'kind' provided ('" + kind + "') is invalid.");
-        return nullptr;
-    }
+    // 4.8.14.12.5 Text track API

[philipj_slow, 2015/01/19 14:26:29]

Given the direct link this isn't very useful, and is guaranteed to be stale
soon.

[fs, 2015/01/19 14:55:06]

Dropped.

+    // The addTextTrack(kind, label, language) method of media elements, when
+    // invoked, must run the following steps:
 
-    // 2. If the label argument was omitted, let label be the empty string.
-    // 3. If the language argument was omitted, let language be the empty string.
-    // 4. Create a new TextTrack object.
+    // 1. Create a new TextTrack object.
+    // 2. 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, ..., and its text track list of cues to
+    //    an empty list.
+    RefPtrWillBeRawPtr<TextTrack> textTrack = TextTrack::create(kind, label, language);
+    //    ..., its text track readiness state to the text track loaded state, ...
+    textTrack->setReadinessState(TextTrack::Loaded);
 
-    // 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...
-    RefPtrWillBeRawPtr<TextTrack> textTrack = TextTrack::create(kind, label, language);
-
-    // Note, due to side effects when changing track parameters, we have to
-    // first append the track to the text track list.
-
-    // 6. Add the new text track to the media element's list of text tracks.
+    // 3. Add the new text track to the media element's list of text tracks.
+    // 4. Queue a task to fire a trusted event with the name addtrack, that
+    //    does not bubble and is not cancelable, and that uses the TrackEvent
+    //    interface, with the track attribute initialised to the new text
+    //    track's TextTrack object, at the media element's textTracks
+    //    attribute's TextTrackList object.
     addTextTrack(textTrack.get());
 
-    // ... its text track readiness state to the text track loaded state ...
-    textTrack->setReadinessState(TextTrack::Loaded);
+    // Note: Due to side effects when changing track parameters, we have to
+    // first append the track to the text track list.
+    // FIXME: Since setMode() will cause a 'change' event to be queued on the
+    // same task source as the 'addtrack' event (see above), meaning the order

[philipj_slow, 2015/01/19 14:26:29]

s/meaning//

[fs, 2015/01/19 14:55:06]

Done.

+    // is wrong. (If the 'change' event should be fired at all in this case...)

[philipj_slow, 2015/01/19 14:26:29]

The spec says "Whenever a text track that is in a media element's list of text
tracks has its text track mode change value" so the change event should not be
fired. I think I filed or commented on a spec bug about this once. Perhaps
fixing the order in this function to match the spec would fix the problem, but
no reason to do it in this CL unless you think it fits.

[fs, 2015/01/19 14:55:06]

Yes, my interpretation was that 'change' should not be fired here. The code-flow
is a bit elaborate here though, so I opted to leave the issue alone for now.

I updated the comment (final parenthesis) to state that 'change' should not be
fired at all.

 
-    // ... its text track mode to the text track hidden mode, and its text track list of cues to an empty list ...
+    // ..., its text track mode to the text track hidden mode, ...
     textTrack->setMode(TextTrack::hiddenKeyword());
 
+    // 5. Return the new TextTrack object.
     return textTrack.release();
 }
 
 TextTrackList* HTMLMediaElement::textTracks()
 {
     if (!m_textTracks)
         m_textTracks = TextTrackList::create(this);
 
     return m_textTracks.get();
 }
@@ skipping 1318 matching lines @@
 
 #if ENABLE(WEB_AUDIO)
 void HTMLMediaElement::clearWeakMembers(Visitor* visitor)
 {
     if (!visitor->isAlive(m_audioSourceNode) && audioSourceProvider())
         audioSourceProvider()->setClient(nullptr);
 }
 #endif
 
 }