RTCPeerConnection.addTrack and removeTrack added (behind flag)

chrome/test/data/webrtc/peerconnection_rtp.js

Index: chrome/test/data/webrtc/peerconnection_rtp.js
diff --git a/chrome/test/data/webrtc/peerconnection_rtp.js b/chrome/test/data/webrtc/peerconnection_rtp.js
index 49962de83fd3615a8b986c5eedbabacba18f1566..ba66274a5408683b3544e43d5cd1ac400b482158 100644
--- a/chrome/test/data/webrtc/peerconnection_rtp.js
+++ b/chrome/test/data/webrtc/peerconnection_rtp.js
@@ -45,15 +45,6 @@ function verifyRtpSenders(expectedNumTracks = null) {
     throw failTest('One getSenders() call is not equal to the next.');
   }
 
-  let localTracks = new Set();
-  peerConnection_().getLocalStreams().forEach(function(stream) {
-    stream.getTracks().forEach(function(track) {
-      localTracks.add(track);
-    });
-  });
-  if (peerConnection_().getSenders().length != localTracks.size)
-    throw failTest('The number of senders and tracks are not the same.');
-
   let senders = new Set();
   let senderTracks = new Set();
   peerConnection_().getSenders().forEach(function(sender) {
@@ -67,10 +58,6 @@ function verifyRtpSenders(expectedNumTracks = null) {
   if (senderTracks.size != senders.size)
     throw failTest('senderTracks.size != senders.size');
 
-  if (!setEquals_(senderTracks, localTracks)) {
-    throw failTest('The set of sender tracks is not equal to the set of ' +
-                   'stream tracks.');
-  }
   returnToTest('ok-senders-verified');
 }
 
@@ -93,15 +80,6 @@ function verifyRtpReceivers(expectedNumTracks = null) {
     throw failTest('One getReceivers() call is not equal to the next.');
   }
 
-  let remoteTracks = new Set();
-  peerConnection_().getRemoteStreams().forEach(function(stream) {
-    stream.getTracks().forEach(function(track) {
-      remoteTracks.add(track);
-    });
-  });
-  if (peerConnection_().getReceivers().length != remoteTracks.size)
-    throw failTest('The number of receivers and tracks are not the same.');
-
   let receivers = new Set();
   let receiverTracks = new Set();
   peerConnection_().getReceivers().forEach(function(receiver) {
@@ -117,13 +95,162 @@ function verifyRtpReceivers(expectedNumTracks = null) {
   if (receiverTracks.size != receivers.size)
     throw failTest('receiverTracks.size != receivers.size');
 
-  if (!setEquals_(receiverTracks, remoteTracks)) {
-    throw failTest('The set of receiver tracks is not equal to the set of ' +
-                   'stream tracks.');
-  }
   returnToTest('ok-receivers-verified');
 }
 
+/**
+ * Creates an audio and video track and adds them to the peer connection using
+ * |addTrack|. They are added with or without a stream in accordance with
+ * |streamArgumentType|. The test ensures senders are created and parses the SDP
+ * to determine the resulting stream and track structure is what we expect. In
+ * the 'no-stream' case the test allows the peer connection to use null streams
+ * or to advertise streams that are hidden from us.
+ *
+ * Returns
+ * "ok-<audio stream id> <audio track id> <video stream id> <video track id>" on
+ * success. If no stream is backing up the track, <stream id> is "null".
+ *
+ * @param {string} streamArgumentType Must be one of the following values:
+ * 'no-stream' - The tracks are added without an associated stream.
+ * 'shared-stream' - The tracks are added with the same associated stream.
+ * 'individual-streams' - A stream is created for each track.
+ */
+function createAndAddAudioAndVideoTrack(streamArgumentType) {
+  if (streamArgumentType !== 'no-stream' &&
+      streamArgumentType !== 'shared-stream' &&
+      streamArgumentType !== 'individual-streams')
+    throw failTest('Unsupported streamArgumentType.');
+  getUserMedia({ audio: true, video: true },
+      function(stream) {
+        let initialSenderCount = peerConnection_().getSenders().length;
+        let initialLocalStreams = new Set(peerConnection_().getLocalStreams());
+
+        let audioStream = undefined;
+        if (streamArgumentType !== 'no-stream')
+          audioStream = new MediaStream();
+
+        let audioTrack = stream.getAudioTracks()[0];
+        let audioSender =
+            audioStream ? peerConnection_().addTrack(audioTrack, audioStream)
+                        : peerConnection_().addTrack(audioTrack);
+        if (audioSender.track !== audioTrack)
+          throw failTest('audioSender.track !== audioTrack');

[Taylor_Brandstetter, 2017/07/06 22:45:14]

I don't really like mixing test assertions with helper methods, unless it's
obvious what assertions the helper method will be making.

[hbos_chromium, 2017/07/07 12:07:48]

The bridge between C++ tests and JavaScript is not very nice, I can only return
strings from here. I could return sender.track.id and parse it in C++ to do
EXPECT_EQ(sender_track_id, track_id) but with the hassle of returning things
from javascript it seems a bit silly to return two track IDs that can never be
different unless there is an unlikely bug.

As for the other assertions, I could verify that there is a sender for the track
in a separate function call: EXPECT_TRUE(hasSenderWithTrackId(left_tab_,
track_id)). Just like we now on the remote side do
EXPECT_TRUE(hasReceiverWithTrack(right_tab_, track_id)).

I've reduced but not eliminated the number of assertions. Is that ok, otherwise
what do you propose?

[Taylor_Brandstetter, 2017/07/07 19:44:13]

This seems ok to me.

+
+        peerConnection_().onnegotiationneeded = function() {

[Taylor_Brandstetter, 2017/07/06 22:45:14]

Why does this have to be done in onnegotiationneeded?

[hbos_chromium, 2017/07/07 12:07:48]

It doesn't, I just wanted to verify that it's called. Added a counter separately
and verify that the count is what we expect in a follow-up call in C++ to JS.

+          let videoStream = undefined;
+          if (streamArgumentType === 'shared-stream') {
+            videoStream = audioStream;
+          } else if (streamArgumentType === 'individual-streams') {
+            videoStream = new MediaStream();
+          }
+
+          let videoTrack = stream.getVideoTracks()[0];
+          let videoSender =
+              videoStream ? peerConnection_().addTrack(videoTrack, videoStream)
+                          : peerConnection_().addTrack(videoTrack);
+          if (videoSender.track !== videoTrack)
+            throw failTest('videoSender.track !== videoTrack');
+
+          if (peerConnection_().getSenders().length !==
+                  initialSenderCount + 2 ||
+              !peerConnection_().getSenders().includes(audioSender) ||
+              !peerConnection_().getSenders().includes(videoSender))
+            throw failTest('Unexpected getSenders() result.');
+
+          peerConnection_().onnegotiationneeded = function() {
+            peerConnection_().createOffer().then(function(sessionDecription) {
+              // TODO(hbos): When |getLocalStreams| is the set of streams of all
+              // senders the set will be modified to include |addTrack|-streams.
+              // https://crbug.com/738918
+              if (!setEquals_(initialLocalStreams,
+                              new Set(peerConnection_().getLocalStreams())))
+                throw failTest('The local stream set was modified.');
+              let streamsAndTracks = getSdpStreamsAndTracks(
+                  sessionDecription.sdp);
+              if (!streamsAndTracks.trackIds.has(audioTrack.id) ||
+                  !streamsAndTracks.trackIds.has(videoTrack.id))
+                throw failTest('Track ID are missing from the SDP.');
+              if (streamsAndTracks.streamIds.has(stream.id)) {
+                // We never added this stream.
+                throw failTest(
+                    'The stream we created shouldn\'t be listed in the SDP.');
+              }
+              let audioStreamId = null;
+              let videoStreamId = null;
+              for (let [streamId, trackIds] of
+                   streamsAndTracks.streamTrackMap) {
+                if (trackIds.has(audioTrack.id)) {
+                  if (audioStreamId != null) {
+                    throw failTest(
+                        'The audio track was added to multiple streams.');
+                  }
+                  audioStreamId = streamId;
+                }
+                if (trackIds.has(videoTrack.id)) {
+                  if (videoStreamId != null) {
+                    throw failTest(
+                        'The video track was added to multiple streams.');
+                  }
+                  videoStreamId = streamId;
+                }
+              }
+              if (audioStreamId == null)
+                audioStreamId = 'null';
+              if (videoStreamId == null)
+                videoStreamId = 'null';
+              if (audioStream && audioStream.id !== audioStreamId) {
+                throw failTest('Expected audio stream id ' + audioStream.id +
+                               ', but got ' + audioStreamId);
+              }
+              if (videoStream && videoStream.id !== videoStreamId) {
+                throw failTest('Expected video stream id ' + videoStream.id +
+                               ', but got ' + videoStreamId);
+              }
+              returnToTest('ok-' + audioStreamId + ' ' + audioTrack.id
+                           + ' ' + videoStreamId + ' ' + videoTrack.id);
+            });
+          };
+        };
+      },
+      function(error) {
+        throw failTest('getUserMedia failed: ' + error);
+      });
+}
+
+/**
+ * Calls |removeTrack| with the first sender that has the track with |trackId|
+ * and verifies the SDP is updated accordingly.
+ *
+ * Returns "ok-sender-removed" on success.
+ */
+function removeTrack(trackId) {
+  let sender = null;
+  let otherSenderHasTrack = false;
+  peerConnection_().getSenders().forEach(function(s) {
+    if (s.track && s.track.id == trackId) {
+      if (!sender)
+        sender = s;
+      else
+        otherSenderHasTrack = true;
+    }
+  });
+  if (!sender)
+    throw failTest('There is no sender for track ' + trackId);
+  peerConnection_().removeTrack(sender);
+  if (sender.track)
+    throw failTest('sender.track was not nulled by removeTrack.');
+  peerConnection_().onnegotiationneeded = function() {
+    peerConnection_().createOffer().then(function(sessionDecription) {
+      let streamsAndTracks = getSdpStreamsAndTracks(sessionDecription.sdp);
+      if (!otherSenderHasTrack && streamsAndTracks.trackIds.has(trackId))
+        throw failTest('Removing the track did not remove it from the SDP.');
+      if (otherSenderHasTrack && !streamsAndTracks.trackIds.has(trackId))
+        throw failTest('Track not expected to be removed from the SDP.');
+      returnToTest('ok-sender-removed');
+    });
+  };
+}
+
 // Internals.
 
 /** @private */