Deprecate use of cricket::MediaContentDescription::Copy

One should use a std::unique_ptr to the object, as returned
by Clone() instead, not a naked pointer.

Bug: webrtc:10701
Change-Id: I10ab309207f2cb5aec83a6d09336699ed7b26f50
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/169342
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30646}

pc/media_session_unittest.cc

@@ -3371,18 +3371,12 @@ TEST(MediaSessionDescription, CopySessionDescription) {
       std::make_unique<AudioContentDescription>();
   acd->set_codecs(MAKE_VECTOR(kAudioCodecs1));
   acd->AddLegacyStream(1);
-  std::unique_ptr<AudioContentDescription> acd_passed =
-      absl::WrapUnique(acd->Copy());
-  source.AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp,
-                    std::move(acd_passed));
+  source.AddContent(cricket::CN_AUDIO, MediaProtocolType::kRtp, acd->Clone());
   std::unique_ptr<VideoContentDescription> vcd =
       std::make_unique<VideoContentDescription>();
   vcd->set_codecs(MAKE_VECTOR(kVideoCodecs1));
   vcd->AddLegacyStream(2);
-  std::unique_ptr<VideoContentDescription> vcd_passed =
-      absl::WrapUnique(vcd->Copy());
-  source.AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp,
-                    std::move(vcd_passed));
+  source.AddContent(cricket::CN_VIDEO, MediaProtocolType::kRtp, vcd->Clone());
 
   std::unique_ptr<SessionDescription> copy = source.Clone();
   ASSERT_TRUE(copy.get() != NULL);

pc/session_description.h

@@ -87,9 +87,15 @@ class MediaContentDescription {
 
   virtual bool has_codecs() const = 0;
 
-  virtual MediaContentDescription* Copy() const = 0;
-  virtual std::unique_ptr<MediaContentDescription> Clone() const {
-    return absl::WrapUnique(Copy());
+  RTC_DEPRECATED virtual MediaContentDescription* Copy() const {
+    return CloneInternal();
+  }
+  // Copy operator that returns an unique_ptr.
+  // Not a virtual function.
+  // If a type-specific variant of Clone() is desired, override it, or
+  // simply use std::make_unique<typename>(*this) instead of Clone().
+  std::unique_ptr<MediaContentDescription> Clone() const {
+    return absl::WrapUnique(CloneInternal());
   }
 
   // |protocol| is the expected media transport protocol, such as RTP/AVPF,
@@ -280,6 +286,12 @@ class MediaContentDescription {
   std::vector<RidDescription> receive_rids_;
 
   absl::optional<std::string> alt_protocol_;
+
+ private:
+  // Copy function that returns a raw pointer. Caller will assert ownership.
+  // Should only be called by the Clone() function. Must be implemented
+  // by each final subclass.
+  virtual MediaContentDescription* CloneInternal() const = 0;
 };
 
 // TODO(bugs.webrtc.org/8620): Remove this alias once downstream projects have
@@ -337,34 +349,46 @@ class AudioContentDescription : public MediaContentDescriptionImpl<AudioCodec> {
  public:
   AudioContentDescription() {}
 
-  virtual AudioContentDescription* Copy() const {
-    return new AudioContentDescription(*this);
+  RTC_DEPRECATED virtual AudioContentDescription* Copy() const {
+    return CloneInternal();
   }
   virtual MediaType type() const { return MEDIA_TYPE_AUDIO; }
   virtual AudioContentDescription* as_audio() { return this; }
   virtual const AudioContentDescription* as_audio() const { return this; }
+
+ private:
+  virtual AudioContentDescription* CloneInternal() const {
+    return new AudioContentDescription(*this);
+  }
 };
 
 class VideoContentDescription : public MediaContentDescriptionImpl<VideoCodec> {
  public:
-  virtual VideoContentDescription* Copy() const {
-    return new VideoContentDescription(*this);
+  RTC_DEPRECATED virtual VideoContentDescription* Copy() const {
+    return CloneInternal();
   }
   virtual MediaType type() const { return MEDIA_TYPE_VIDEO; }
   virtual VideoContentDescription* as_video() { return this; }
   virtual const VideoContentDescription* as_video() const { return this; }
+
+ private:
+  virtual VideoContentDescription* CloneInternal() const {
+    return new VideoContentDescription(*this);
+  }
 };
 
 class RtpDataContentDescription
     : public MediaContentDescriptionImpl<RtpDataCodec> {
  public:
   RtpDataContentDescription() {}
-  RtpDataContentDescription* Copy() const override {
-    return new RtpDataContentDescription(*this);
-  }
   MediaType type() const override { return MEDIA_TYPE_DATA; }
   RtpDataContentDescription* as_rtp_data() override { return this; }
   const RtpDataContentDescription* as_rtp_data() const override { return this; }
+
+ private:
+  RtpDataContentDescription* CloneInternal() const override {
+    return new RtpDataContentDescription(*this);
+  }
 };
 
 class SctpDataContentDescription : public MediaContentDescription {
@@ -375,9 +399,6 @@ class SctpDataContentDescription : public MediaContentDescription {
         use_sctpmap_(o.use_sctpmap_),
         port_(o.port_),
         max_message_size_(o.max_message_size_) {}
-  SctpDataContentDescription* Copy() const override {
-    return new SctpDataContentDescription(*this);
-  }
   MediaType type() const override { return MEDIA_TYPE_DATA; }
   SctpDataContentDescription* as_sctp() override { return this; }
   const SctpDataContentDescription* as_sctp() const override { return this; }
@@ -398,6 +419,9 @@ class SctpDataContentDescription : public MediaContentDescription {
   }
 
  private:
+  SctpDataContentDescription* CloneInternal() const override {
+    return new SctpDataContentDescription(*this);
+  }
   bool use_sctpmap_ = true;  // Note: "true" is no longer conformant.
   // Defaults should be constants imported from SCTP. Quick hack.
   int port_ = 5000;

test/pc/e2e/sdp/sdp_changer.cc

@@ -204,7 +204,7 @@ LocalAndRemoteSdp SignalingInterceptor::PatchVp8Offer(
     // single simulcast section will be converted. Do it before removing content
     // because otherwise description will be deleted.
     std::unique_ptr<cricket::MediaContentDescription> prototype_media_desc =
-        absl::WrapUnique(simulcast_content->media_description()->Copy());
+        simulcast_content->media_description()->Clone();
 
     // Remove simulcast video section from offer.
     RTC_CHECK(desc->RemoveContentByName(simulcast_content->mid()));