Bug 1232082 - fix removal of remote tracks to update receivers. r=bwc
☠☠ backed out by a8a91621bad6 ☠ ☠
authorJan-Ivar Bruaroey <jib@mozilla.com>
Tue, 05 Jan 2016 19:51:52 -0500
changeset 314780 1c8afc8ea1c1ef380074d7ab53b22be8a1e5d02a
parent 314779 d8ec4008871edf61c0265f109a47ba851c097dd3
child 314781 7fc1687025ecda73045e6b31651652d0c0be61b0
push id5703
push userraliiev@mozilla.com
push dateMon, 07 Mar 2016 14:18:41 +0000
treeherdermozilla-beta@31e373ad5b5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1232082
milestone46.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1232082 - fix removal of remote tracks to update receivers. r=bwc
dom/media/PeerConnection.js
dom/media/tests/mochitest/pc.js
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
--- a/dom/media/PeerConnection.js
+++ b/dom/media/PeerConnection.js
@@ -1361,39 +1361,40 @@ PeerConnectionObserver.prototype = {
   //                 This is more aggressive than failed, and may trigger
   //                 intermittently (and resolve itself without action) on a
   //                 flaky network.
   //
   //   closed        The ICE Agent has shut down and is no longer responding to
   //                 STUN requests.
 
   handleIceConnectionStateChange: function(iceConnectionState) {
-    if (this._dompc.iceConnectionState === 'new') {
+    let pc = this._dompc;
+    if (pc.iceConnectionState === 'new') {
       var checking_histogram = Services.telemetry.getHistogramById("WEBRTC_ICE_CHECKING_RATE");
       if (iceConnectionState === 'checking') {
         checking_histogram.add(true);
       } else if (iceConnectionState === 'failed') {
         checking_histogram.add(false);
       }
-    } else if (this._dompc.iceConnectionState === 'checking') {
-      var success_histogram = Services.telemetry.getHistogramById(this._dompc._isLoop ?
+    } else if (pc.iceConnectionState === 'checking') {
+      var success_histogram = Services.telemetry.getHistogramById(pc._isLoop ?
         "LOOP_ICE_SUCCESS_RATE" : "WEBRTC_ICE_SUCCESS_RATE");
       if (iceConnectionState === 'completed' ||
           iceConnectionState === 'connected') {
         success_histogram.add(true);
       } else if (iceConnectionState === 'failed') {
         success_histogram.add(false);
       }
     }
 
     if (iceConnectionState === 'failed') {
-      this._dompc.logError("ICE failed, see about:webrtc for more details", null, 0);
+      pc.logError("ICE failed, see about:webrtc for more details", null, 0);
     }
 
-    this._dompc.changeIceConnectionState(iceConnectionState);
+    pc.changeIceConnectionState(iceConnectionState);
   },
 
   // This method is responsible for updating iceGatheringState. This
   // state is defined in the WebRTC specification as follows:
   //
   // iceGatheringState:
   // ------------------
   //   new        The object was just created, and no networking has occurred
@@ -1438,62 +1439,61 @@ PeerConnectionObserver.prototype = {
 
       default:
         this._dompc.logWarning("Unhandled state type: " + state, null, 0);
         break;
     }
   },
 
   onGetStatsSuccess: function(dict) {
-    let chromeobj = new RTCStatsReport(this._dompc._win, dict);
-    let webidlobj = this._dompc._win.RTCStatsReport._create(this._dompc._win,
-                                                            chromeobj);
+    let pc = this._dompc;
+    let chromeobj = new RTCStatsReport(pc._win, dict);
+    let webidlobj = pc._win.RTCStatsReport._create(pc._win, chromeobj);
     chromeobj.makeStatsPublic();
-    this._dompc._onGetStatsSuccess(webidlobj);
+    pc._onGetStatsSuccess(webidlobj);
   },
 
   onGetStatsError: function(code, message) {
     this._dompc._onGetStatsFailure(this.newError(message, code));
   },
 
   onAddStream: function(stream) {
     let ev = new this._dompc._win.MediaStreamEvent("addstream",
                                                    { stream: stream });
     this.dispatchEvent(ev);
   },
 
-  onRemoveStream: function(stream, type) {
+  onRemoveStream: function(stream) {
     this.dispatchEvent(new this._dompc._win.MediaStreamEvent("removestream",
                                                              { stream: stream }));
   },
 
   onAddTrack: function(track, streams) {
     let pc = this._dompc;
     let receiver = pc._win.RTCRtpReceiver._create(pc._win,
                                                   new RTCRtpReceiver(this,
                                                                      track));
     pc._receivers.push(receiver);
-    let ev = new this._dompc._win.RTCTrackEvent("track",
-                                                { receiver: receiver,
-                                                  track: track,
-                                                  streams: streams });
+    let ev = new pc._win.RTCTrackEvent("track",
+                                       { receiver: receiver,
+                                         track: track,
+                                         streams: streams });
     this.dispatchEvent(ev);
 
     // Fire legacy event as well for a little bit.
-    ev = new this._dompc._win.MediaStreamTrackEvent("addtrack", { track: track });
+    ev = new pc._win.MediaStreamTrackEvent("addtrack", { track: track });
     this.dispatchEvent(ev);
   },
 
-  onRemoveTrack: function(track, type) {
-    let i = this._dompc._receivers.findIndex(receiver => receiver.track == track);
+  onRemoveTrack: function(track) {
+    let pc = this._dompc;
+    let i = pc._receivers.findIndex(receiver => receiver.track == track);
     if (i >= 0) {
-      this._receivers.splice(i, 1);
+      pc._receivers.splice(i, 1);
     }
-    this.dispatchEvent(new this._dompc._win.MediaStreamTrackEvent("removetrack",
-                                                                  { track: track }));
   },
 
   onReplaceTrackSuccess: function() {
     var pc = this._dompc;
     pc._onReplaceTrackSender.track = pc._onReplaceTrackWithTrack;
     pc._onReplaceTrackWithTrack = null;
     pc._onReplaceTrackSender = null;
     pc._onReplaceTrackSuccess();
--- a/dom/media/tests/mochitest/pc.js
+++ b/dom/media/tests/mochitest/pc.js
@@ -1388,39 +1388,33 @@ PeerConnectionWrapper.prototype = {
    * @returns {Promise}
    *        A promise that resolves when media is flowing.
    */
   waitForRtpFlow(track) {
     var hasFlow = stats => {
       var rtpStatsKey = Object.keys(stats)
         .find(key => !stats[key].isRemote && stats[key].type.endsWith("boundrtp"));
       ok(rtpStatsKey, "Should have RTP stats for track " + track.id);
+      if (!rtpStatsKey) {
+        return false;
+      }
       var rtp = stats[rtpStatsKey];
       var nrPackets = rtp[rtp.type == "outboundrtp" ? "packetsSent"
                                                     : "packetsReceived"];
       info("Track " + track.id + " has " + nrPackets + " " +
            rtp.type + " RTP packets.");
       return nrPackets > 0;
     };
 
-    return new Promise(resolve => {
-      info("Checking RTP packet flow for track " + track.id);
+    info("Checking RTP packet flow for track " + track.id);
 
-      var waitForFlow = () => {
-        this._pc.getStats(track).then(stats => {
-          if (hasFlow(stats)) {
-            ok(true, "RTP flowing for track " + track.id);
-            resolve();
-          } else {
-            wait(200).then(waitForFlow);
-          }
-        });
-      };
-      waitForFlow();
-    });
+    var retry = () => this._pc.getStats(track)
+      .then(stats => hasFlow(stats)? ok(true, "RTP flowing for track " + track.id) :
+                                     wait(200).then(retry));
+    return retry();
   },
 
   /**
    * Wait for presence of video flow on all media elements and rtp flow on
    * all sending and receiving track involved in this test.
    *
    * @returns {Promise}
    *        A promise that resolves when media flows for all elements and tracks
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -1863,29 +1863,41 @@ PeerConnectionImpl::SetRemoteDescription
       }
 #endif
     }
 
     std::vector<RefPtr<JsepTrack>> removedTracks =
       mJsepSession->GetRemoteTracksRemoved();
 
     for (auto i = removedTracks.begin(); i != removedTracks.end(); ++i) {
-      RefPtr<RemoteSourceStreamInfo> info =
-        mMedia->GetRemoteStreamById((*i)->GetStreamId());
+      const std::string& streamId = (*i)->GetStreamId();
+      const std::string& trackId = (*i)->GetTrackId();
+
+      RefPtr<RemoteSourceStreamInfo> info = mMedia->GetRemoteStreamById(streamId);
       if (!info) {
         MOZ_ASSERT(false, "A stream/track was removed that wasn't in PCMedia. "
                           "This is a bug.");
         continue;
       }
 
-      mMedia->RemoveRemoteTrack((*i)->GetStreamId(), (*i)->GetTrackId());
+      mMedia->RemoveRemoteTrack(streamId, trackId);
+
+      DOMMediaStream* stream = info->GetMediaStream();
+      nsTArray<RefPtr<MediaStreamTrack>> tracks;
+      stream->GetTracks(tracks);
+      for (auto& track : tracks) {
+        if (PeerConnectionImpl::GetTrackId(*track) == trackId) {
+          pco->OnRemoveTrack(*track, jrv);
+          break;
+        }
+      }
 
       // We might be holding the last ref, but that's ok.
       if (!info->GetTrackCount()) {
-        pco->OnRemoveStream(*info->GetMediaStream(), jrv);
+        pco->OnRemoveStream(*stream, jrv);
       }
     }
 
     pco->OnSetRemoteDescriptionSuccess(jrv);
 #if !defined(MOZILLA_EXTERNAL_LINKAGE)
     startCallTelem();
 #endif
   }