Also check the pending remote description when generating MIDs for le…

…gacy remote offers Bug: webrtc:10296 Change-Id: Ia10299177175e57d3f494281310d6c91bed9ebdb Reviewed-on: https://webrtc-review.googlesource.com/c/121860 Commit-Queue: Steve Anton <[email protected]> Reviewed-by: Amit Hilbuch <[email protected]> Cr-Commit-Position: refs/heads/master@{#26591}
basharalbashier · Feb 7, 2019 · d7180cc · d7180cc
1 parent ce470aa
commit d7180cc
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 6 deletions.
diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
@@ -2311,13 +2311,16 @@ static absl::string_view GetDefaultMidForPlanB(cricket::MediaType media_type) {
 }
 
 void PeerConnection::FillInMissingRemoteMids(
-    cricket::SessionDescription* remote_description) {
-  RTC_DCHECK(remote_description);
+    cricket::SessionDescription* new_remote_description) {
+  RTC_DCHECK(new_remote_description);
   const cricket::ContentInfos& local_contents =
       (local_description() ? local_description()->description()->contents()
                            : cricket::ContentInfos());
-  for (size_t i = 0; i < remote_description->contents().size(); ++i) {
-    cricket::ContentInfo& content = remote_description->contents()[i];
+  const cricket::ContentInfos& remote_contents =
+      (remote_description() ? remote_description()->description()->contents()
+                            : cricket::ContentInfos());
+  for (size_t i = 0; i < new_remote_description->contents().size(); ++i) {
+    cricket::ContentInfo& content = new_remote_description->contents()[i];
     if (!content.name.empty()) {
       continue;
     }
@@ -2327,6 +2330,9 @@ void PeerConnection::FillInMissingRemoteMids(
       if (i < local_contents.size()) {
         new_mid = local_contents[i].name;
         source_explanation = "from the matching local media section";
+      } else if (i < remote_contents.size()) {
+        new_mid = remote_contents[i].name;
+        source_explanation = "from the matching previous remote media section";
       } else {
         new_mid = mid_generator_();
         source_explanation = "generated just now";
@@ -2336,9 +2342,9 @@ void PeerConnection::FillInMissingRemoteMids(
           GetDefaultMidForPlanB(content.media_description()->type()));
       source_explanation = "to match pre-existing behavior";
     }
+    RTC_DCHECK(!new_mid.empty());
     content.name = new_mid;
-    remote_description->transport_infos()[i].content_name =
-        std::string(new_mid);
+    new_remote_description->transport_infos()[i].content_name = new_mid;
     RTC_LOG(LS_INFO) << "SetRemoteDescription: Remote media section at i=" << i
                      << " is missing an a=mid line. Filling in the value '"
                      << new_mid << "' " << source_explanation << ".";

diff --git a/pc/peer_connection_jsep_unittest.cc b/pc/peer_connection_jsep_unittest.cc
@@ -1694,6 +1694,24 @@ TEST_F(PeerConnectionJsepTest, LegacyNoMidAudioVideoAnswer) {
   ASSERT_TRUE(caller->SetRemoteDescription(std::move(answer)));
 }
 
+// Test that negotiation works with legacy endpoints which do not support a=mid
+// when setting two remote descriptions without setting a local description in
+// between.
+TEST_F(PeerConnectionJsepTest, LegacyNoMidTwoRemoteOffers) {
+  auto caller = CreatePeerConnection();
+  caller->AddAudioTrack("audio");
+  auto callee = CreatePeerConnection();
+  callee->AddAudioTrack("audio");
+
+  auto offer = caller->CreateOffer();
+  ClearMids(offer.get());
+
+  ASSERT_TRUE(
+      callee->SetRemoteDescription(CloneSessionDescription(offer.get())));
+  ASSERT_TRUE(callee->SetRemoteDescription(std::move(offer)));
+  EXPECT_TRUE(callee->SetLocalDescription(callee->CreateAnswer()));
+}
+
 // Test that SetLocalDescription fails if a=mid lines are missing.
 TEST_F(PeerConnectionJsepTest, SetLocalDescriptionFailsMissingMid) {
   auto caller = CreatePeerConnection();