video-6336: media element leak causes error on chrome 92

CHANGELOG.md

@@ -2,6 +2,21 @@ The Twilio Programmable Video SDKs use [Semantic Versioning](http://www.semver.o
 
 **Version 1.x will End of Life on September 8th, 2021.** Check [this guide](https://www.twilio.com/docs/video/migrating-1x-2x) to plan your migration to the latest 2.x version. Support for the 1.x version ended on December 4th, 2020.
 
+2.15.3 (In Progress)
+====================
+
+Bug Fixes
+---------
+Chrome 92 [started enforcing](https://chromium-review.googlesource.com/c/chromium/src/+/2816118) limit on number of WebMediaPlayers. This blocks creation of further WebMediaPlayers once there are already 75 (desktop) or 40 (mobile) players. Fixed a related bug where the SDK was not cleaning up an internally maintained media elements.
+Please ensure that your application cleans up media elements as well after they are detached.
+```js
+  const elements = track.detach();
+  elements.forEach(el => {
+    el.remove();
+    el.srcObject = null;
+  });
+```
+
 2.15.2 (July 15, 2021)
 ======================
 

lib/media/track/mediatrack.js

@@ -145,8 +145,10 @@ class MediaTrack extends Track {
   _end() {
     this._log.debug('Ended');
     if (this._dummyEl) {
-      this._detachElement(this._dummyEl);
+      this._dummyEl.remove();
+      this._dummyEl.srcObject = null;
       this._dummyEl.oncanplay = null;
+      this._dummyEl = null;
     }
   }
 
@@ -165,7 +167,6 @@ class MediaTrack extends Track {
         : null;
       this._elShims.set(el, shimMediaElement(el, onUnintentionallyPaused));
     }
-
     return el;
   }
 
@@ -266,12 +267,10 @@ class MediaTrack extends Track {
     if (!this._attachments.has(el)) {
       return el;
     }
-
     const mediaStream = el.srcObject;
     if (mediaStream instanceof this._MediaStream) {
       mediaStream.removeTrack(this.processedTrack || this.mediaStreamTrack);
     }
-
     this._attachments.delete(el);
 
     if (this._shouldShimAttachedElements && this._elShims.has(el)) {

test/integration/index.js

@@ -7,6 +7,7 @@ require('./spec/bandwidthprofile/regressions');
 require('./spec/bandwidthprofile/renderhints');
 require('./spec/bandwidthprofile/video');
 require('./spec/connect');
+require('./spec/leaktests');
 require('./spec/logger');
 require('./spec/localparticipant/networkqualitylevel');
 require('./spec/localparticipant/publishunpublishtrack');

test/integration/spec/leaktests.js

@@ -0,0 +1,132 @@
+/* eslint-disable no-undefined */
+'use strict';
+
+const assert = require('assert');
+const defaults = require('../../lib/defaults');
+const { completeRoom, createRoom } = require('../../lib/rest');
+const { audio: createLocalAudioTrack, video: createLocalVideoTrack } = require('../../../lib/createlocaltrack');
+const connect = require('../../../lib/connect');
+const getToken = require('../../lib/token');
+const { isChrome } = require('../../lib/guessbrowser');
+
+const {
+  smallVideoConstraints,
+  randomName,
+  participantsConnected,
+  tracksSubscribed,
+  tracksUnpublished,
+  waitForSometime,
+  waitFor,
+  trackStarted
+} = require('../../lib/util');
+
+
+function getTracksOfKind(participant, kind) {
+  return [...participant.tracks.values()].filter(remoteTrack => remoteTrack.kind === kind).map(({ track }) => track);
+}
+
+// eslint-disable-next-line no-warning-comments
+// TODO(mpatwardhan): Fix VIDEO-6356 and enable this test for p2p rooms.
+(isChrome && defaults.topology !== 'peer-to-peer' ? describe : describe.skip)('VIDEO-6336: media element leak detection', function() {
+  // eslint-disable-next-line no-invalid-this
+  this.timeout(5 * 60 * 1000);
+
+  it('tracks continue to start after 50 connect disconnects ', async () => {
+    const localAudioTrack = await createLocalAudioTrack({ fake: true });
+    const localVideoTrack = await createLocalVideoTrack(smallVideoConstraints);
+    const roomName = await createRoom(randomName(), defaults.topology);
+    const connectOptions = Object.assign({
+      name: roomName,
+      tracks: [localAudioTrack, localVideoTrack]
+    }, defaults);
+
+    const aliceRoom = await waitFor(connect(getToken('Alice'), connectOptions), 'Alice to connect to room');
+    async function joinRoomAndEnsureTracksStarted(i) {
+      // eslint-disable-next-line no-console
+      console.log(`${i}] connecting to room ${aliceRoom.sid}`);
+      const bobRoom = await waitFor(connect(getToken('Bob'), connectOptions), `${i}] Bob to join room: ${aliceRoom.sid}`);
+
+      // wait for Bob to see alice connected.
+      await waitFor(participantsConnected(bobRoom, 1), `${i}] Bob to see Alice connected: ${aliceRoom.sid}`);
+
+      const aliceRemote = bobRoom.participants.get(aliceRoom.localParticipant.sid);
+      await waitFor(tracksSubscribed(aliceRemote, 2), `${i}] Bob to see Alice's track: ${aliceRoom.sid}`);
+
+      const remoteVideoTracks = getTracksOfKind(aliceRemote, 'video');
+      const remoteAudioTracks = getTracksOfKind(aliceRemote, 'audio');
+      assert.strictEqual(remoteVideoTracks.length, 1);
+      assert.strictEqual(remoteAudioTracks.length, 1);
+
+      await waitFor(trackStarted(remoteVideoTracks[0]), `${i}] Bob to see Alice's video track started: ${aliceRoom.sid}`);
+      await waitFor(trackStarted(remoteAudioTracks[0]), `${i}] Bob to see Alice's audio track started: ${aliceRoom.sid}`);
+      bobRoom.disconnect();
+      await waitForSometime(1000);
+    }
+
+    // Alice joins room first.
+    for (let i = 0; i < 50; i++) {
+      // eslint-disable-next-line no-await-in-loop
+      await joinRoomAndEnsureTracksStarted(i);
+    }
+
+    aliceRoom.disconnect();
+    await completeRoom(aliceRoom.sid);
+  });
+
+  it('tracks continue to start after 50 publish/unpublish ', async () => {
+    const localAudioTrack = await createLocalAudioTrack({ fake: true });
+    const localVideoTrack = await createLocalVideoTrack(smallVideoConstraints);
+    const roomName = await createRoom(randomName(), defaults.topology);
+    const connectOptions = Object.assign({
+      name: roomName,
+      tracks: []
+    }, defaults);
+
+    const aliceRoom = await waitFor(connect(getToken('Alice'), connectOptions), 'Alice to connect to room');
+    const bobRoom = await waitFor(connect(getToken('Bob'), connectOptions), `Bob to join room: ${aliceRoom.sid}`);
+    // wait for Bob to see alice connected.
+    await waitFor(participantsConnected(bobRoom, 1), `Bob to see Alice connected: ${aliceRoom.sid}`);
+
+    const aliceRemote = bobRoom.participants.get(aliceRoom.localParticipant.sid);
+
+    async function publishAndVerifyStarted(i) {
+
+      // eslint-disable-next-line no-console
+      console.log(`${i}] Alice publishing tracks`);
+      // alice publishes two tracks.
+      const audioPublication = await waitFor(aliceRoom.localParticipant.publishTrack(localAudioTrack), `Alice to publish a audio track: ${aliceRoom.sid}`);
+      const videoPublication = await waitFor(aliceRoom.localParticipant.publishTrack(localVideoTrack), `Alice to publish a video track: ${aliceRoom.sid}`);
+
+      await waitFor(tracksSubscribed(aliceRemote, 2), `${i}] Bob to see Alice's track: ${aliceRoom.sid}`);
+
+      const remoteVideoTracks = getTracksOfKind(aliceRemote, 'video');
+      const remoteAudioTracks = getTracksOfKind(aliceRemote, 'audio');
+      assert.strictEqual(remoteVideoTracks.length, 1);
+      assert.strictEqual(remoteAudioTracks.length, 1);
+
+      // bob sees tracks published by alice started.
+      await waitFor(trackStarted(remoteVideoTracks[0]), `${i}] Bob to see Alice's video track started: ${aliceRoom.sid}`);
+      await waitFor(trackStarted(remoteAudioTracks[0]), `${i}] Bob to see Alice's audio track started: ${aliceRoom.sid}`);
+
+      const bobSeesUnpublished =  tracksUnpublished(aliceRemote, 2);
+
+      // alice un-publishes tracks.
+      audioPublication.unpublish();
+      videoPublication.unpublish();
+
+      // bob sees tracks unpublished.
+      await waitFor(bobSeesUnpublished, `${i}] Bob to see tracks unpublished: ${aliceRoom.sid} `);
+
+      await waitForSometime(1000);
+    }
+
+    for (let i = 0; i < 50; i++) {
+      // eslint-disable-next-line no-await-in-loop
+      await publishAndVerifyStarted(i);
+    }
+
+    aliceRoom.disconnect();
+    await completeRoom(aliceRoom.sid);
+  });
+
+});

test/lib/document.js

@@ -22,6 +22,10 @@ class HTMLElement {
     return this;
   }
 
+  remove() {
+
+  }
+
   play() {
     return Promise.resolve();
   }

test/unit/spec/media/track/localmediatrack.js

@@ -117,7 +117,7 @@ const { defer } = require('../../../../../lib/util');
     describe('#_setMediaStreamTrack', () => {
       let dummyElement;
       beforeEach(() => {
-        dummyElement = { oncanplay: 'bar' };
+        dummyElement = { oncanplay: 'bar', remove: sinon.spy() };
         document.createElement = sinon.spy(() => {
           return dummyElement;
         });
@@ -340,7 +340,8 @@ const { defer } = require('../../../../../lib/util');
       const dummyElement = {
         oncanplay: null,
         videoWidth: 320,
-        videoHeight: 240
+        videoHeight: 240,
+        remove: sinon.spy()
       };
 
       before(() => {

test/unit/spec/media/track/mediatrack.js

@@ -56,7 +56,7 @@ describe('MediaTrack', () => {
       track._detachElement = sinon.spy();
       track._attachments.delete = sinon.spy();
 
-      dummyElement = { oncanplay: 'bar' };
+      dummyElement = { oncanplay: 'bar', remove: sinon.spy() };
       track._createElement = sinon.spy(() => {
         return dummyElement;
       });