Update the header extensions capabilities with mid, rid and rrid

Video and audio senders are missing mid, rid and rrid extensions in
their GetCapabilities call.

Bug: chromium:1007894
Change-Id: Ie9edba28ae32fda5e501913cac694f43bfb185ac
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/156560
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29493}
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 74647a8..294e3be 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -507,6 +507,12 @@
       webrtc::RtpExtension(webrtc::RtpExtension::kFrameMarkingUri, id++));
   capabilities.header_extensions.push_back(
       webrtc::RtpExtension(webrtc::RtpExtension::kColorSpaceUri, id++));
+  capabilities.header_extensions.push_back(
+      webrtc::RtpExtension(webrtc::RtpExtension::kMidUri, id++));
+  capabilities.header_extensions.push_back(
+      webrtc::RtpExtension(webrtc::RtpExtension::kRidUri, id++));
+  capabilities.header_extensions.push_back(
+      webrtc::RtpExtension(webrtc::RtpExtension::kRepairedRidUri, id++));
   if (webrtc::field_trial::IsEnabled("WebRTC-GenericDescriptorAdvertised")) {
     capabilities.header_extensions.push_back(webrtc::RtpExtension(
         webrtc::RtpExtension::kGenericFrameDescriptorUri00, id++));
diff --git a/media/engine/webrtc_voice_engine.cc b/media/engine/webrtc_voice_engine.cc
index a3b27a5..4c3bc84 100644
--- a/media/engine/webrtc_voice_engine.cc
+++ b/media/engine/webrtc_voice_engine.cc
@@ -531,6 +531,12 @@
       webrtc::RtpExtension(webrtc::RtpExtension::kAbsSendTimeUri, id++));
   capabilities.header_extensions.push_back(webrtc::RtpExtension(
       webrtc::RtpExtension::kTransportSequenceNumberUri, id++));
+  capabilities.header_extensions.push_back(
+      webrtc::RtpExtension(webrtc::RtpExtension::kMidUri, id++));
+  capabilities.header_extensions.push_back(
+      webrtc::RtpExtension(webrtc::RtpExtension::kRidUri, id++));
+  capabilities.header_extensions.push_back(
+      webrtc::RtpExtension(webrtc::RtpExtension::kRepairedRidUri, id++));
   return capabilities;
 }
 
diff --git a/pc/media_session.cc b/pc/media_session.cc
index dd5a814..873f27d 100644
--- a/pc/media_session.cc
+++ b/pc/media_session.cc
@@ -1357,30 +1357,25 @@
   ComputeAudioCodecsIntersectionAndUnion();
 }
 
-static void AddUnifiedPlanExtensions(RtpHeaderExtensions* extensions) {
+static void RemoveUnifiedPlanExtensions(RtpHeaderExtensions* extensions) {
   RTC_DCHECK(extensions);
 
-  rtc::UniqueNumberGenerator<int> unique_id_generator;
-  unique_id_generator.AddKnownId(0);  // The first valid RTP extension ID is 1.
-  for (const webrtc::RtpExtension& extension : *extensions) {
-    const bool collision_free = unique_id_generator.AddKnownId(extension.id);
-    RTC_DCHECK(collision_free);
-  }
-
-  // Unified Plan also offers the MID and RID header extensions.
-  extensions->push_back(webrtc::RtpExtension(webrtc::RtpExtension::kMidUri,
-                                             unique_id_generator()));
-  extensions->push_back(webrtc::RtpExtension(webrtc::RtpExtension::kRidUri,
-                                             unique_id_generator()));
-  extensions->push_back(webrtc::RtpExtension(
-      webrtc::RtpExtension::kRepairedRidUri, unique_id_generator()));
+  extensions->erase(
+      std::remove_if(extensions->begin(), extensions->end(),
+                     [](auto extension) {
+                       return extension.uri == webrtc::RtpExtension::kMidUri ||
+                              extension.uri == webrtc::RtpExtension::kRidUri ||
+                              extension.uri ==
+                                  webrtc::RtpExtension::kRepairedRidUri;
+                     }),
+      extensions->end());
 }
 
 RtpHeaderExtensions
 MediaSessionDescriptionFactory::audio_rtp_header_extensions() const {
   RtpHeaderExtensions extensions = audio_rtp_extensions_;
-  if (is_unified_plan_) {
-    AddUnifiedPlanExtensions(&extensions);
+  if (!is_unified_plan_) {
+    RemoveUnifiedPlanExtensions(&extensions);
   }
 
   return extensions;
@@ -1389,8 +1384,8 @@
 RtpHeaderExtensions
 MediaSessionDescriptionFactory::video_rtp_header_extensions() const {
   RtpHeaderExtensions extensions = video_rtp_extensions_;
-  if (is_unified_plan_) {
-    AddUnifiedPlanExtensions(&extensions);
+  if (!is_unified_plan_) {
+    RemoveUnifiedPlanExtensions(&extensions);
   }
 
   return extensions;
diff --git a/pc/media_session_unittest.cc b/pc/media_session_unittest.cc
index d2feb1f..a2416c4 100644
--- a/pc/media_session_unittest.cc
+++ b/pc/media_session_unittest.cc
@@ -4441,50 +4441,6 @@
   EXPECT_EQ(no_codecs, sf.audio_sendrecv_codecs());
 }
 
-// Checks that the RID extensions are added to the video RTP header extensions.
-// Note: This test somewhat shows that |set_video_rtp_header_extensions()| is
-// not very well defined, as calling set() and immediately get() will yield
-// an object that is not semantically equivalent to the set object.
-TEST_F(MediaSessionDescriptionFactoryTest, VideoHasRidExtensionsInUnifiedPlan) {
-  TransportDescriptionFactory tdf;
-  UniqueRandomIdGenerator ssrc_generator;
-  MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator);
-  sf.set_is_unified_plan(true);
-  cricket::RtpHeaderExtensions extensions;
-  sf.set_video_rtp_header_extensions(extensions);
-  cricket::RtpHeaderExtensions result = sf.video_rtp_header_extensions();
-  // Check to see that RID extensions were added to the extension list
-  EXPECT_GE(result.size(), 2u);
-  EXPECT_THAT(result, Contains(Field("uri", &RtpExtension::uri,
-                                     RtpExtension::kMidUri)));
-  EXPECT_THAT(result, Contains(Field("uri", &RtpExtension::uri,
-                                     RtpExtension::kRidUri)));
-  EXPECT_THAT(result, Contains(Field("uri", &RtpExtension::uri,
-                                     RtpExtension::kRepairedRidUri)));
-}
-
-// Checks that the RID extensions are added to the audio RTP header extensions.
-// Note: This test somewhat shows that |set_audio_rtp_header_extensions()| is
-// not very well defined, as calling set() and immediately get() will yield
-// an object that is not semantically equivalent to the set object.
-TEST_F(MediaSessionDescriptionFactoryTest, AudioHasRidExtensionsInUnifiedPlan) {
-  TransportDescriptionFactory tdf;
-  UniqueRandomIdGenerator ssrc_generator;
-  MediaSessionDescriptionFactory sf(&tdf, &ssrc_generator);
-  sf.set_is_unified_plan(true);
-  cricket::RtpHeaderExtensions extensions;
-  sf.set_audio_rtp_header_extensions(extensions);
-  cricket::RtpHeaderExtensions result = sf.audio_rtp_header_extensions();
-  // Check to see that RID extensions were added to the extension list
-  EXPECT_GE(result.size(), 2u);
-  EXPECT_THAT(result, Contains(Field("uri", &RtpExtension::uri,
-                                     RtpExtension::kMidUri)));
-  EXPECT_THAT(result, Contains(Field("uri", &RtpExtension::uri,
-                                     RtpExtension::kRidUri)));
-  EXPECT_THAT(result, Contains(Field("uri", &RtpExtension::uri,
-                                     RtpExtension::kRepairedRidUri)));
-}
-
 namespace {
 // Compare the two vectors of codecs ignoring the payload type.
 template <class Codec>