Duplicate in-band tracks when switching <source> elements

• Component: Media
• Status: RESOLVED FIXED

[Brendan Long]

When we change the source of a HTMLMediaElement like this:

    var newSource = document.createElement("source");
    newSource.src = "some-video.m4v";
    video.replaceChild(newSource, video.firstChild);

In HTMLMediaElement::loadNextSourceChild, we:

    // Recreate the media player for the new url
    createMediaPlayer();

And in HTMLMediaElement::createMediaPlayer():

    m_player = MediaPlayer::create(this);

This will cause any existing MediaPlayer to be destroyed, and because ~MediaPlayer removes the client:

    MediaPlayer::~MediaPlayer()
    {
        m_mediaPlayerClient = 0;
    }

It's impossible for the MediaPlayerPrivate to remove any existing in-band tracks.

One fix is to do this in createMediaPlayer():

    #if ENABLE(VIDEO_TRACK)
        removeAllInbandTracks();
    #endif
        m_player = MediaPlayer::create(this);

For some reason, media-related tests always time out for me, so I'm not able to attach the expected results for this test :\

[Brendan Long]

Created attachment 209786 [details]
Patch

[Brendan Long]

I'm building WebKit on a Mac so I can get the expected test results, it should be done by the time I get back from work.

[Brendan Long]

I tried building this on OSX with:

Tools/Scripts/build-webkit --debug --video-track

Then:

Tools/Scripts/run-webkit-tests --debug LayoutTests/media/track/track-in-band-duplicate-tracks-when-source-changes.html

Which results in a crash in DumpRenderTree, and if I use run-safari, it doesn't see the in-band tracks, and run-launcher doesn't seem to work (can't exec WebKitBuild/Debug: Permission Denied). It's possible my mac is broken somehow :\

[Eric Carlson]

Created attachment 209791 [details]
Updated patch

If the test uses reportExpected() instead of testExpected(), the number of in-band tracks won't be logged and we should be able to use the same test results for all ports that support in-band tracks.

[Brendan Long]

(In reply to comment #4)
> Created an attachment (id=209791) [details]
> Updated patch
> 
> If the test uses reportExpected() instead of testExpected(), the number of in-band tracks won't be logged and we should be able to use the same test results for all ports that support in-band tracks.

Thanks for the suggestion. I might need to use that in the tests on my other patches too.

[Brendan Long]

Created attachment 209799 [details]
Add expected result file

I got the 'expected' file out of DumpRenderTree (which works perfectly for some reason, even though run-webkit-tests doesn't).

[Brendan Long]

Wait, why does EWS say your patch passed, when my original patch got this output?

> Regressions: Unexpected missing results (1)
>   media/track/track-in-band-duplicate-tracks-when-source-changes.html [ Missing ]

[Brendan Long]

(In reply to comment #7)
> Wait, why does EWS say your patch passed, when my original patch got this output?
> 
> > Regressions: Unexpected missing results (1)
> >   media/track/track-in-band-duplicate-tracks-when-source-changes.html [ Missing ]

Nevermind, I think I just didn't wait long enough.

[Build Bot]

Comment on attachment 209791 [details]
Updated patch

Attachment 209791 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/1618043

New failing tests:
media/track/track-in-band-duplicate-tracks-when-source-changes.html

[Build Bot]

Created attachment 209802 [details]
Archive of layout-test-results from webkit-ews-01 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-01  Port: mac-mountainlion  Platform: Mac OS X 10.8.4

[Brendan Long]

The buildbot errors were from the previous patch by the way. The current patch works.

[WebKit Commit Bot]

Comment on attachment 209799 [details]
Add expected result file

Clearing flags on attachment: 209799

Committed r154760: <http://trac.webkit.org/changeset/154760>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

The test added by this patch appears to have never passed on Lion:
http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=media%2Ftrack%2Ftrack-in-band-duplicate-tracks-when-source-changes.html