Bug 130704 – Prevent 'removetrack' events from firing when all inband text tracks are removed.

• Status: RESOLVED
• Resolution: FIXED
• Component: Media

[Brent Fulgham]

This patch fixes a bug where the 'removetrack' event was being fired when following the W3C logic to "forget the media element's media-resource-specific track" algorithm.

It is based on the Blink work done in <https://codereview.chromium.org/177243018/>, though it diverges a bit due to the drift in code since the fork.

[Radar WebKit Bug Importer]

<rdar://problem/16414074>

[Brent Fulgham]

Created attachment 227711 [details]
Patch

[Eric Carlson]

Comment on attachment 227711 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=227711&action=review

> Source/WebCore/html/HTMLMediaElement.h:293
> +    void removeTextTrack(TextTrack*, bool scheduleEvent = true);

Is the default value used by any of the call sites?

[Brent Fulgham]

(In reply to comment #3)
> (From update of attachment 227711 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=227711&action=review
> 
> > Source/WebCore/html/HTMLMediaElement.h:293
> > +    void removeTextTrack(TextTrack*, bool scheduleEvent = true);
> 
> Is the default value used by any of the call sites?

Yes, mainly in the MediaSources.cpp file. I wanted to make sure the behavior of these locations was unchanged.

Do we need to suppress the remove track event for audio or video track handling? I don't think the WebVTT spec says anything about those operations.

[Eric Carlson]

(In reply to comment #4)
> (In reply to comment #3)
> > (From update of attachment 227711 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=227711&action=review
> > 
> > > Source/WebCore/html/HTMLMediaElement.h:293
> > > +    void removeTextTrack(TextTrack*, bool scheduleEvent = true);
> > 
> > Is the default value used by any of the call sites?
> 
> Yes, mainly in the MediaSources.cpp file. I wanted to make sure the behavior of these locations was unchanged.
> 
You changed HTMLMediaElement::didRemoveTextTrack to pass true explicitly - removeTextTrack(textTrack.get(), true). We should probably be consistent one way or the other.

> Do we need to suppress the remove track event for audio or video track handling? I don't think the WebVTT spec says anything about those operations.

The WebVTT spec wouldn't, but the HTML spec might. I can look later.

[Brent Fulgham]

(In reply to comment #5)
> You changed HTMLMediaElement::didRemoveTextTrack to pass true explicitly - removeTextTrack(textTrack.get(), true). We should probably be consistent one way or the other.

I reverted that line. Now all uses that expect the event to be triggered are called without the argument. We only pass the argument in case where we want to suppress the event.

[Brent Fulgham]

Committed r166238: <http://trac.webkit.org/changeset/166238>

[Alexey Proskuryakov]

The test is very flaky, filed bug 130971.