Remove SourceBuffer media tracks on detach from media element

third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-avtracks.html

Index: third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-avtracks.html
diff --git a/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-avtracks.html b/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-avtracks.html
index 131e1d05683727ecd9dbf16088ea921b4a7fe022..8ab1aa7a323b1704ad397b6a306129eccf4b4109 100644
--- a/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-avtracks.html
+++ b/third_party/WebKit/LayoutTests/http/tests/media/media-source/mediasource-avtracks.html
@@ -13,11 +13,12 @@
           mediasource_testafterdataloaded(function(test, mediaElement, mediaSource, segmentInfo, sourceBuffer, mediaData)

[wolenetz, 2016/04/27 21:39:54]

nit indent everything 2 more columns between <script></script> here. (4-space
indent)

[servolk, 2016/04/27 23:41:35]

Done.

           {
               var initSegment = MediaSourceUtil.extractSegmentData(mediaData, segmentInfo.init);
-              test.expectEvent(sourceBuffer, 'updateend', 'initSegment append ended.');
               test.expectEvent(sourceBuffer.audioTracks, 'addtrack', 'sourceBuffer.videoTracks addtrack event');

[wolenetz, 2016/04/27 21:39:54]

nit: use either ' or ", not mixed, to contain strings within
<script>...</script>. Let's go with ".

[wolenetz, 2016/04/27 21:39:55]

nit: s/video/audio/ in message.

[servolk, 2016/04/27 23:41:35]

Done.

[servolk, 2016/04/27 23:41:36]

Done.

               test.expectEvent(sourceBuffer.videoTracks, 'addtrack', 'sourceBuffer.videoTracks addtrack event');
               test.expectEvent(mediaElement.audioTracks, 'addtrack', 'mediaElement.videoTracks addtrack event');

[wolenetz, 2016/04/27 21:39:55]

nit ditto here and below as appropriate.

[servolk, 2016/04/27 23:41:35]

Done.

               test.expectEvent(mediaElement.videoTracks, 'addtrack', 'mediaElement.videoTracks addtrack event');
+              test.expectEvent(mediaElement, 'loadedmetadata', 'loadedmetadata done.');
+              test.expectEvent(sourceBuffer, 'updateend', 'initSegment append ended.');
               sourceBuffer.appendBuffer(initSegment);
               test.waitForExpectedEvents(function()
               {
@@ -48,6 +49,60 @@
                 test.done();
               });
           }, "MediaSource media track properties");
+
+          mediasource_testafterdataloaded(function(test, mediaElement, mediaSource, segmentInfo, sourceBuffer, mediaData)
+          {
+              var initSegment = MediaSourceUtil.extractSegmentData(mediaData, segmentInfo.init);
+              test.expectEvent(sourceBuffer.audioTracks, 'addtrack', 'sourceBuffer.videoTracks addtrack event');

[wolenetz, 2016/04/27 21:39:55]

nit ditto (audio <-> video) here and below as appropriate.

[servolk, 2016/04/27 23:41:35]

Done.

+              test.expectEvent(sourceBuffer.videoTracks, 'addtrack', 'sourceBuffer.videoTracks addtrack event');
+              test.expectEvent(mediaElement.audioTracks, 'addtrack', 'mediaElement.videoTracks addtrack event');
+              test.expectEvent(mediaElement.videoTracks, 'addtrack', 'mediaElement.videoTracks addtrack event');
+              test.expectEvent(mediaElement, 'loadedmetadata', 'loadedmetadata done.');
+              test.expectEvent(sourceBuffer, 'updateend', 'initSegment append ended.');
+              sourceBuffer.appendBuffer(initSegment);
+
+              var audioTrack;
+              var videoTrack;
+              test.waitForExpectedEvents(function()

[wolenetz, 2016/04/27 21:39:55]

Since these first few lines are common across these two tests. I suggest putting
them in a utility function that takes a callback that is invoked on this first
test.waitForExpectedEvents() in each test. Or even subsume most of the first
test in this utility function, since much of the property checking after that
first waitForExpectedEvents is the same in both tests.

[servolk, 2016/04/27 23:41:35]

Done.

+              {
+                assert_equals(mediaElement.videoTracks.length, 1, "videoTracks.length");

[wolenetz, 2016/04/27 21:39:54]

nit:indentation

[servolk, 2016/04/27 23:41:35]

Done.

+                assert_equals(mediaElement.audioTracks.length, 1, "audioTracks.length");
+                assert_equals(sourceBuffer.videoTracks.length, 1, "videoTracks.length");
+                assert_equals(sourceBuffer.audioTracks.length, 1, "audioTracks.length");
+
+                // The first video track is selected by default.
+                videoTrack = sourceBuffer.videoTracks[0];
+                assert_true(videoTrack.selected, "sourceBuffer.videoTracks[0].selected");
+                assert_equals(videoTrack.sourceBuffer, sourceBuffer, "videoTrack.sourceBuffer");
+                // The first audio track is enabled by default.
+                audioTrack = sourceBuffer.audioTracks[0];
+                assert_true(audioTrack.enabled, "sourceBuffer.audioTracks[0].enabled");
+                assert_equals(audioTrack.sourceBuffer, sourceBuffer, "audioTrack.sourceBuffer");
+
+                test.expectEvent(sourceBuffer.audioTracks, 'removetrack', 'sourceBuffer.videoTracks removetrack event');

[wolenetz, 2016/04/27 21:39:55]

nit ditto (audio <-> video) here and below as appropriate...

[servolk, 2016/04/27 23:41:35]

Done.

+                test.expectEvent(sourceBuffer.videoTracks, 'removetrack', 'sourceBuffer.videoTracks removetrack event');
+                test.expectEvent(mediaElement.audioTracks, 'removetrack', 'mediaElement.videoTracks removetrack event');
+                test.expectEvent(mediaElement.videoTracks, 'removetrack', 'mediaElement.videoTracks removetrack event');
+                // Removing enabled audio track and selected video track should fire 'change' events on mediaElement track lists.
+                test.expectEvent(mediaElement.audioTracks, 'change', 'mediaElement.audioTracks changed.');
+                test.expectEvent(mediaElement.videoTracks, 'change', 'mediaElement.videoTracks changed.');
+                test.expectEvent(mediaSource.sourceBuffers, 'removesourcebuffer', 'mediaSource.sourceBuffers removesourcebuffer event');
+                mediaSource.removeSourceBuffer(sourceBuffer);
+              });
+
+              test.waitForExpectedEvents(function()
+              {
+                assert_equals(mediaSource.sourceBuffers.length, 0, "mediaSource.sourceBuffers.length");

[wolenetz, 2016/04/27 21:39:55]

nit:indentation

[servolk, 2016/04/27 23:41:35]

Done.

+                assert_equals(mediaElement.videoTracks.length, 0, "videoTracks.length");
+                assert_equals(mediaElement.audioTracks.length, 0, "audioTracks.length");
+                assert_equals(sourceBuffer.videoTracks.length, 0, "videoTracks.length");
+                assert_equals(sourceBuffer.audioTracks.length, 0, "audioTracks.length");
+                // Since audio and video tracks have been removed, their .sourceBuffer property should be null now.
+                assert_equals(audioTrack.sourceBuffer, null, "audioTrack.sourceBuffer");
+                assert_equals(videoTrack.sourceBuffer, null, "videoTrack.sourceBuffer");
+                test.done();
+              });
+          }, "MediaSource media tracks must be removed when a SourceBuffer is removed");
         </script>

[wolenetz, 2016/04/27 21:39:55]

Please add a test specifically for this removal also happening on detach. IIUC,
mediaElement.load() should trigger detach().
Please reference the new non-normative note in
http://w3c.github.io/media-source/#mediasource-detach
A more interoperable test (than load() triggering detach) might be to just set
src="". Perhaps testing both would be best, and when this is upstreamed to w3c
web-platform-tests, I'll include only the src="" version, since that's
guaranteed to be spec compliant/interoperable IIUC.

[servolk, 2016/04/27 23:41:36]

Done.

     </body>
 </html>