From 60b6c1dfa93f2bb5116f7e599e65017e0867cece Mon Sep 17 00:00:00 2001
From: Steve Anton <steveanton@webrtc.org>
Date: Wed, 13 Jun 2018 11:32:27 -0700
Subject: [PATCH] [Unified Plan] Clear RtpSender "SSRC" when the SDP has no
 send streams

This fixes a crash that occurs with this sequence of events:
1. AddTrack. SetLocalDescription(CreateOffer())
2. RemoveTrack. SetLocalDescription(CreateOffer())
3. AddTrack.

When AddTrack is called again it re-uses the RtpTransceiver/
RtpSender and try to configure the underlying MediaChannel. But the
MediaChannel would DCHECK since the send stream had been destroyed
by the SLD in 2. and would not get created until SLD is called
again.

Bug: webrtc:9401
Change-Id: I4b5572886e17263aaa4ce0408663444d72e09243
Reviewed-on: https://webrtc-review.googlesource.com/83420
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23605}
---
 pc/peerconnection.cc              |  6 ++++++
 pc/peerconnection_rtp_unittest.cc | 36 +++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index a9d415af08..0e0e9a6210 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -2113,6 +2113,12 @@ RTCError PeerConnection::ApplyLocalDescription(
             streams[0].stream_ids());
         transceiver->internal()->sender_internal()->SetSsrc(
             streams[0].first_ssrc());
+      } else {
+        // 0 is a special value meaning "this sender has no associated send
+        // stream". Need to call this so the sender won't attempt to configure
+        // a no longer existing stream and run into DCHECKs in the lower
+        // layers.
+        transceiver->internal()->sender_internal()->SetSsrc(0);
       }
     }
   } else {
diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc
index 586189eb75..2123eec36c 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -1219,6 +1219,42 @@ TEST_F(PeerConnectionRtpTestUnifiedPlan,
   EXPECT_FALSE(caller->observer()->negotiation_needed());
 }
 
+// Test that setting offers that add/remove/add a track repeatedly without
+// setting the appropriate answer in between works.
+// These are regression tests for bugs.webrtc.org/9401
+TEST_F(PeerConnectionRtpTestUnifiedPlan, AddRemoveAddTrackOffersWorksAudio) {
+  auto caller = CreatePeerConnection();
+
+  auto sender1 = caller->AddAudioTrack("audio1");
+  ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
+
+  caller->pc()->RemoveTrack(sender1);
+  ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
+
+  // This will re-use the transceiver created by the first AddTrack.
+  auto sender2 = caller->AddAudioTrack("audio2");
+  ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
+
+  EXPECT_EQ(1u, caller->pc()->GetTransceivers().size());
+  EXPECT_EQ(sender1, sender2);
+}
+TEST_F(PeerConnectionRtpTestUnifiedPlan, AddRemoveAddTrackOffersWorksVideo) {
+  auto caller = CreatePeerConnection();
+
+  auto sender1 = caller->AddVideoTrack("video1");
+  ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
+
+  caller->pc()->RemoveTrack(sender1);
+  ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
+
+  // This will re-use the transceiver created by the first AddTrack.
+  auto sender2 = caller->AddVideoTrack("video2");
+  ASSERT_TRUE(caller->SetLocalDescription(caller->CreateOffer()));
+
+  EXPECT_EQ(1u, caller->pc()->GetTransceivers().size());
+  EXPECT_EQ(sender1, sender2);
+}
+
 // Test that OnRenegotiationNeeded is fired if SetDirection is called on an
 // active RtpTransceiver with a new direction.
 TEST_F(PeerConnectionRtpTestUnifiedPlan,