commit d7180cccc48efdec6241e3b21449046e62ebb91a
author Steve Anton <steveanton@webrtc.org> Thu Feb 07 18:44:53 2019
committer Commit Bot <commit-bot@chromium.org> Thu Feb 07 20:22:38 2019
tree 30e1ee803c72908f5e1f2ceed55e393f80698929
parent ce470aab518f067a67aa03aaab1fc61a45afa0ec

Also check the pending remote description when generating MIDs for legacy remote offers

Bug: webrtc:10296
Change-Id: Ia10299177175e57d3f494281310d6c91bed9ebdb
Reviewed-on: https://webrtc-review.googlesource.com/c/121860
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Amit Hilbuch <amithi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#26591}

pc/peer_connection.cc

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index f811234..f28e7c0 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -2311,13 +2311,16 @@
 }
 
 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 @@
       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 @@
           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 << ".";

pc/peer_connection_jsep_unittest.cc

diff --git a/pc/peer_connection_jsep_unittest.cc b/pc/peer_connection_jsep_unittest.cc
index 86d26fb..b2fe86b 100644
--- a/pc/peer_connection_jsep_unittest.cc
+++ b/pc/peer_connection_jsep_unittest.cc
@@ -1694,6 +1694,24 @@
   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();