Changeset 138270 in webkit


Ignore:
Timestamp:
Dec 20, 2012 10:28:57 AM (11 years ago)
Author:
commit-queue@webkit.org
Message:

onload callback for <track> element attached to <video> does not fire
​https://bugs.webkit.org/show_bug.cgi?id=103258

Patch by Antoine Quint <Antoine Quint> on 2012-12-20
Reviewed by Eric Carlson.

Source/WebCore:

We now correctly implement the track processing model per the latest spec at
​http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#start-the-track-processing-model.

For a <track> to load, three conditions must be met:

  1. it must have a non-empty src
  2. it must have a mode other than "disabled" (the default)
  3. it must have a parent media element

We used to only check if we were able to load upon setting the track's src,
whereas a change of any of those conditions should have done this, which this
patch now correctly implements.

We also correctly implement the load model asynchronously after step 4, per
the spec, hence the split of the code previously entirely contained in
HTMLTrackElement::scheduleLoad() in another method fired on a timer.

Test: media/track/track-element-load-event.html

  • html/HTMLMediaElement.cpp:

(WebCore::HTMLMediaElement::textTrackModeChanged):

  • html/HTMLTrackElement.cpp:

(WebCore::HTMLTrackElement::HTMLTrackElement):
(WebCore::HTMLTrackElement::insertedInto):
(WebCore::HTMLTrackElement::parseAttribute):
(WebCore::HTMLTrackElement::scheduleLoad):
(WebCore):
(WebCore::HTMLTrackElement::loadTimerFired):
(WebCore::HTMLTrackElement::textTrackModeChanged):

  • html/HTMLTrackElement.h:
  • html/track/TextTrack.cpp:

(WebCore::TextTrack::removeAllCues):
(WebCore):

  • html/track/TextTrack.h:

(TextTrack):

LayoutTests:

As a result of fixing this bug, a few new failures were uncovered and TestExpectations
needed to be updated to take this into account. Additionally, some existing tests were
incorrect or outdated and were fixed as well. Finally, a new test was added to thoroughly
test the various conditions required for a <track> element to successfully load.

  • fast/events/constructors/track-event-constructor.html: Update the test to correctly set

a non-disabled mode on the text track such that it may load per the rules enforced with this patch.

  • http/tests/security/text-track-crossorigin.html: Update the test to correctly set a non-disabled

mode on the text track such that it may load per the rules enforced with this patch.

  • media/track/track-add-track-expected.txt: Update the output to match changes made to test.
  • media/track/track-add-track.html: Update the test to correctly set a non-disabled mode on

the text track such that it may load per the rules enforced with this patch. Also, ensure
the .readyState of the text track is correctly assumed to be NONE vs. LOADING as it would
have to wait until the next run loop to be changed to anything but NONE.

  • media/track/track-element-load-event-expected.txt: Added.
  • media/track/track-element-load-event.html: Added.
  • media/track/track-load-from-src-readyState.html: Update the test to correctly set a

non-disabled mode on the text track such that it may load per the rules enforced with
this patch.

  • platform/chromium/TestExpectations: Skip tests that now fail instead of timing out.
  • platform/efl/TestExpectations: Skip tests that now fail instead of timing out.
  • platform/gtk/TestExpectations: Skip tests that now fail instead of timing out.
  • platform/mac/TestExpectations: Skip tests that now fail instead of timing out.
  • platform/qt/TestExpectations: Skip tests that now fail instead of timing out.
  • platform/win/TestExpectations: Skip tests that now fail instead of timing out.
Location:
trunk
Files:
2 added
18 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r138268 r138270  
     12012-12-20  Antoine Quint  <graouts@apple.com>
     2
     3        onload callback for <track> element attached to <video> does not fire
     4        https://bugs.webkit.org/show_bug.cgi?id=103258
     5
     6        Reviewed by Eric Carlson.
     7
     8        As a result of fixing this bug, a few new failures were uncovered and TestExpectations
     9        needed to be updated to take this into account. Additionally, some existing tests were
     10        incorrect or outdated and were fixed as well. Finally, a new test was added to thoroughly
     11        test the various conditions required for a <track> element to successfully load.
     12
     13        * fast/events/constructors/track-event-constructor.html: Update the test to correctly set
     14        a non-disabled mode on the text track such that it may load per the rules enforced with this patch.
     15        * http/tests/security/text-track-crossorigin.html: Update the test to correctly set a non-disabled
     16        mode on the text track such that it may load per the rules enforced with this patch.
     17        * media/track/track-add-track-expected.txt: Update the output to match changes made to test.
     18        * media/track/track-add-track.html: Update the test to correctly set a non-disabled mode on
     19        the text track such that it may load per the rules enforced with this patch. Also, ensure
     20        the .readyState of the text track is correctly assumed to be NONE vs. LOADING as it would
     21        have to wait until the next run loop to be changed to anything but NONE.
     22        * media/track/track-element-load-event-expected.txt: Added.
     23        * media/track/track-element-load-event.html: Added.
     24        * media/track/track-load-from-src-readyState.html: Update the test to correctly set a
     25        non-disabled mode on the text track such that it may load per the rules enforced with
     26        this patch.
     27        * platform/chromium/TestExpectations: Skip tests that now fail instead of timing out.
     28        * platform/efl/TestExpectations: Skip tests that now fail instead of timing out.
     29        * platform/gtk/TestExpectations: Skip tests that now fail instead of timing out.
     30        * platform/mac/TestExpectations: Skip tests that now fail instead of timing out.
     31        * platform/qt/TestExpectations: Skip tests that now fail instead of timing out.
     32        * platform/win/TestExpectations: Skip tests that now fail instead of timing out.
     33
    1342012-12-20  Joshua Bell  <jsbell@chromium.org>
    235

  • trunk/LayoutTests/fast/events/constructors/track-event-constructor.html

    r99261 r138270  
    5656        trackElement = document.createElement('track');
    5757        video.appendChild(trackElement);
     58        trackElement.track.mode = "hidden";
    5859        trackElement.addEventListener('load', test);
    5960        trackElement.src='data:text/vtt,WEBVTT FILE \r\r1\r00:00:00.000 --> 00:00:30.500\ronly one caption';

  • trunk/LayoutTests/http/tests/security/text-track-crossorigin.html

    r120174 r138270  
    8080            {
    8181                trackElement = document.querySelectorAll('track')[0];
     82                trackElement.track.mode = "hidden";
    8283                log('Loading <b>without</b> Access-Control-Allow-Origin header, no "crossorigin" attribute on &lt;video&gt;');
    8384                var url = "http://localhost:8000/security/resources/captions-with-access-control-headers.php"

  • trunk/LayoutTests/media/track/track-add-track-expected.txt

    r106361 r138270  
    55RUN(video.appendChild(trackElement))
    66RUN(trackElement.src = 'captions-webvtt/tc004-webvtt-file.vtt')
     7RUN(trackElement.track.mode = 'hidden')
    78EXPECTED (video.textTracks.length == '1') OK
    8 EXPECTED (trackElement.readyState == '1') OK
     9EXPECTED (trackElement.readyState == '0') OK
    910EVENT(addtrack)
    1011EXPECTED (event.target == '[object TextTrackList]') OK

  • trunk/LayoutTests/media/track/track-add-track.html

    r106361 r138270  
    4242
    4343                run("trackElement.src = 'captions-webvtt/tc004-webvtt-file.vtt'");
     44                run("trackElement.track.mode = 'hidden'");
    4445                testExpected("video.textTracks.length", 1);
    45                 testExpected("trackElement.readyState", HTMLTrackElement.LOADING);
     46                testExpected("trackElement.readyState", HTMLTrackElement.NONE);
    4647            }
    4748

  • trunk/LayoutTests/media/track/track-load-from-src-readyState.html

    r126786 r138270  
    2525            track.addEventListener("load", function () { trackLoaded(); }, true);
    2626            track.src = "captions-webvtt/tc004-webvtt-file.vtt";   
    27             track.mode = "hidden";
     27            track.track.mode = "hidden";
    2828
    2929        </script>

  • trunk/LayoutTests/platform/chromium/TestExpectations

    r138260 r138270  
    30143014webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/voice_with_evil_timestamp.html [ Skip ]
    30153015
     3016# After fixing webkit.org/b/103258, these tests fail when they used to simply timeout.
     3017# At any rate, there is no regression, just a different type of failure.
     3018webkit.org/b/105536 media/track/opera/interfaces/TextTrack/addCue.html [ Failure ]
     3019webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/endTime.html [ Failure ]
     3020webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/startTime.html [ Failure ]
     3021
    30163022webkit.org/b/72271 [ SnowLeopard Debug ] fast/dom/node-iterator-reference-node-moved-crash.html [ Crash Pass ]
    30173023

  • trunk/LayoutTests/platform/efl/TestExpectations

    r138251 r138270  
    790790webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/cue_font_size_transition.html [ Skip ]
    791791webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/voice_with_evil_timestamp.html [ Skip ]
     792
     793# After fixing webkit.org/b/103258, these tests fail when they used to simply timeout.
     794# At any rate, there is no regression, just a different type of failure.
     795webkit.org/b/105536 media/track/opera/interfaces/TextTrack/addCue.html [ Failure ]
     796webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/endTime.html [ Failure ]
     797webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/startTime.html [ Failure ]
    792798
    793799webkit.org/b/100636 [ Debug ] jquery/traversing.html [ Failure Crash Pass ]

  • trunk/LayoutTests/platform/gtk/TestExpectations

    r138250 r138270  
    494494webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/voice_with_evil_timestamp.html [ Skip ]
    495495
     496# After fixing webkit.org/b/103258, these tests fail when they used to simply timeout.
     497# At any rate, there is no regression, just a different type of failure.
     498webkit.org/b/105536 media/track/opera/interfaces/TextTrack/addCue.html [ Failure ]
     499webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/endTime.html [ Failure ]
     500webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/startTime.html [ Failure ]
     501
    496502# No support for exposing in-band text tracks
    497503webkit.org/b/103771 media/track/track-in-band.html [ Failure ]

  • trunk/LayoutTests/platform/mac/TestExpectations

    r138238 r138270  
    474474media/track/track-webvtt-tc028-unsupported-markup.html
    475475
    476 # Opera-submitted tests to W3C for <track>, a lot of failures still.
    477476# Opera-submitted tests to W3C for <track>, a lot of failures still.
    478477webkit.org/b/103926 media/track/opera/idl/media-idl-tests.html [ Skip ]
    … …  
    509508webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/voice_with_evil_timestamp.html [ Skip ]
    510509
     510# After fixing webkit.org/b/103258, these tests fail when they used to simply timeout.
     511# At any rate, there is no regression, just a different type of failure.
     512webkit.org/b/105536 media/track/opera/interfaces/TextTrack/addCue.html [ Failure ]
     513webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/endTime.html [ Failure ]
     514webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/startTime.html [ Failure ]
     515
    511516# Tests for MediaSource API. Feature is not yet functional.
    512517# https://bugs.webkit.org/show_bug.cgi?id=64731

  • trunk/LayoutTests/platform/qt/TestExpectations

    r138258 r138270  
    927927webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/cue_font_size_transition.html [ Skip ]
    928928webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/voice_with_evil_timestamp.html [ Skip ]
     929
     930# After fixing webkit.org/b/103258, these tests fail when they used to simply timeout.
     931# At any rate, there is no regression, just a different type of failure.
     932webkit.org/b/105536 media/track/opera/interfaces/TextTrack/addCue.html [ Failure ]
     933webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/endTime.html [ Failure ]
     934webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/startTime.html [ Failure ]
    929935
    930936# https://bugs.webkit.org/show_bug.cgi?id=38376

  • trunk/LayoutTests/platform/win/TestExpectations

    r138017 r138270  
    15851585webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/cue_font_size_transition.html [ Skip ]
    15861586webkit.org/b/103926 media/track/opera/track/webvtt/rendering/adhoc/voice_with_evil_timestamp.html [ Skip ]
     1587
     1588# After fixing webkit.org/b/103258, these tests fail when they used to simply timeout.
     1589# At any rate, there is no regression, just a different type of failure.
     1590webkit.org/b/105536 media/track/opera/interfaces/TextTrack/addCue.html [ Failure ]
     1591webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/endTime.html [ Failure ]
     1592webkit.org/b/105536 media/track/opera/interfaces/TextTrackCue/startTime.html [ Failure ]
    15871593
    15881594# Tests for MediaSource API. Feature is not yet functional.

  • trunk/Source/WebCore/ChangeLog

    r138268 r138270  
     12012-12-20  Antoine Quint  <graouts@apple.com>
     2
     3        onload callback for <track> element attached to <video> does not fire
     4        https://bugs.webkit.org/show_bug.cgi?id=103258
     5
     6        Reviewed by Eric Carlson.
     7
     8        We now correctly implement the track processing model per the latest spec at
     9        http://www.whatwg.org/specs/web-apps/current-work/multipage/the-video-element.html#start-the-track-processing-model.
     10       
     11        For a <track> to load, three conditions must be met:
     12       
     13        1. it must have a non-empty src
     14        2. it must have a mode other than "disabled" (the default)
     15        3. it must have a parent media element
     16       
     17        We used to only check if we were able to load upon setting the track's src,
     18        whereas a change of any of those conditions should have done this, which this
     19        patch now correctly implements.
     20       
     21        We also correctly implement the load model asynchronously after step 4, per
     22        the spec, hence the split of the code previously entirely contained in
     23        HTMLTrackElement::scheduleLoad() in another method fired on a timer.
     24
     25        Test: media/track/track-element-load-event.html
     26
     27        * html/HTMLMediaElement.cpp:
     28        (WebCore::HTMLMediaElement::textTrackModeChanged):
     29        * html/HTMLTrackElement.cpp:
     30        (WebCore::HTMLTrackElement::HTMLTrackElement):
     31        (WebCore::HTMLTrackElement::insertedInto):
     32        (WebCore::HTMLTrackElement::parseAttribute):
     33        (WebCore::HTMLTrackElement::scheduleLoad):
     34        (WebCore):
     35        (WebCore::HTMLTrackElement::loadTimerFired):
     36        (WebCore::HTMLTrackElement::textTrackModeChanged):
     37        * html/HTMLTrackElement.h:
     38        * html/track/TextTrack.cpp:
     39        (WebCore::TextTrack::removeAllCues):
     40        (WebCore):
     41        * html/track/TextTrack.h:
     42        (TextTrack):
     43
    1442012-12-20  Joshua Bell  <jsbell@chromium.org>
    245

  • trunk/Source/WebCore/html/HTMLMediaElement.cpp

    r138224 r138270  
    13521352                if (trackElement->readyState() == HTMLTrackElement::LOADED)
    13531353                    textTrackAddCues(track, track->cues());
    1354                 else if (trackElement->readyState() == HTMLTrackElement::NONE)
    1355                     trackElement->scheduleLoad();
    13561354
    13571355                // If this is the first added track, create the list of text tracks.

  • trunk/Source/WebCore/html/HTMLTrackElement.cpp

    r137378 r138270  
    5757inline HTMLTrackElement::HTMLTrackElement(const QualifiedName& tagName, Document* document)
    5858    : HTMLElement(tagName, document)
     59    , m_loadTimer(this, &HTMLTrackElement::loadTimerFired)
    5960{
    6061    LOG(Media, "HTMLTrackElement::HTMLTrackElement - %p", this);
    … …  
    7576Node::InsertionNotificationRequest HTMLTrackElement::insertedInto(ContainerNode* insertionPoint)
    7677{
     78    // Since we've moved to a new parent, we may now be able to load.
     79    scheduleLoad();
     80
    7781    HTMLElement::insertedInto(insertionPoint);
    7882    HTMLMediaElement* parent = mediaElement();
    … …  
    9397    if (RuntimeEnabledFeatures::webkitVideoTrackEnabled()) {
    9498        if (name == srcAttr) {
    95             if (!value.isEmpty() && mediaElement())
     99            if (!value.isEmpty())
    96100                scheduleLoad();
    97             // 4.8.10.12.3 Sourcing out-of-band text tracks
    98             // As the kind, label, and srclang attributes are set, changed, or removed, the text track must update accordingly...
     101            else if (m_track)
     102                m_track->removeAllCues();
     103
     104        // 4.8.10.12.3 Sourcing out-of-band text tracks
     105        // As the kind, label, and srclang attributes are set, changed, or removed, the text track must update accordingly...
    99106        } else if (name == kindAttr)
    100107            track()->setKind(value.lower());
    … …  
    189196void HTMLTrackElement::scheduleLoad()
    190197{
     198    // 1. If another occurrence of this algorithm is already running for this text track and its track element,
     199    // abort these steps, letting that other algorithm take care of this element.
     200    if (m_loadTimer.isActive())
     201        return;
     202
    191203    if (!RuntimeEnabledFeatures::webkitVideoTrackEnabled())
    192204        return;
    193205
     206    // 2. If the text track's text track mode is not set to one of hidden or showing, abort these steps.
     207    if (ensureTrack()->mode() != TextTrack::hiddenKeyword() && ensureTrack()->mode() != TextTrack::showingKeyword())
     208        return;
     209
     210    // 3. If the text track's track element does not have a media element as a parent, abort these steps.
    194211    if (!mediaElement())
    195212        return;
    196213
     214    // 4. Run the remainder of these steps asynchronously, allowing whatever caused these steps to run to continue.
     215    m_loadTimer.startOneShot(0);
     216}
     217
     218void HTMLTrackElement::loadTimerFired(Timer<HTMLTrackElement>*)
     219{
    197220    if (!fastHasAttribute(srcAttr))
    198221        return;
    199222
    200     // 4.8.10.12.3 Sourcing out-of-band text tracks
    201 
    202     // 1. Set the text track readiness state to loading.
     223    // 6. Set the text track readiness state to loading.
    203224    setReadyState(HTMLTrackElement::LOADING);
    204225
     226    // 7. Let URL be the track URL of the track element.
    205227    KURL url = getNonEmptyURLAttribute(srcAttr);
     228
     229    // 8. If the track element's parent is a media element then let CORS mode be the state of the parent media
     230    // element's crossorigin content attribute. Otherwise, let CORS mode be No CORS.
    206231    if (!canLoadUrl(url)) {
    207232        didCompleteLoad(ensureTrack(), HTMLTrackElement::Failure);
    … …  
    303328void HTMLTrackElement::textTrackModeChanged(TextTrack* track)
    304329{
     330    // Since we've moved to a new parent, we may now be able to load.
     331    if (readyState() == HTMLTrackElement::NONE)
     332        scheduleLoad();
     333
    305334    if (HTMLMediaElement* parent = mediaElement())
    306335        return parent->textTrackModeChanged(track);

  • trunk/Source/WebCore/html/HTMLTrackElement.h

    r135202 r138270  
    8080    virtual bool isURLAttribute(const Attribute&) const OVERRIDE;
    8181
     82    void loadTimerFired(Timer<HTMLTrackElement>*);
     83
    8284#if ENABLE(MICRODATA)
    8385    virtual String itemValueText() const OVERRIDE;
    … …  
    99101
    100102    RefPtr<LoadableTextTrack> m_track;
     103    Timer<HTMLTrackElement> m_loadTimer;
    101104};
    102105

  • trunk/Source/WebCore/html/track/TextTrack.cpp

    r136843 r138270  
    202202}
    203203
     204void TextTrack::removeAllCues()
     205{
     206    if (!m_cues)
     207        return;
     208
     209    if (m_client)
     210        m_client->textTrackRemoveCues(this, m_cues.get());
     211   
     212    for (size_t i = 0; i < m_cues->length(); ++i)
     213        m_cues->item(i)->setTrack(0);
     214   
     215    m_cues = 0;
     216}
     217
    204218TextTrackCueList* TextTrack::activeCues() const
    205219{

  • trunk/Source/WebCore/html/track/TextTrack.h

    r136528 r138270  
    124124    virtual void setIsDefault(bool) { }
    125125
     126    void removeAllCues();
     127
    126128protected:
    127129    TextTrack(ScriptExecutionContext*, TextTrackClient*, const AtomicString& kind, const AtomicString& label, const AtomicString& language, TextTrackType);
Note: See TracChangeset for help on using the changeset viewer.