Convert TransportDescriptionFactory to return unique_ptrs

Bug: None
Change-Id: I887f38c680c813d4a1ce9a06387638b47779eb48
Reviewed-on: https://webrtc-review.googlesource.com/c/113825
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25973}

p2p/base/transportdescriptionfactory.cc

@@ -14,6 +14,7 @@
 #include <memory>
 #include <string>
 
+#include "absl/memory/memory.h"
 #include "p2p/base/transportdescription.h"
 #include "rtc_base/logging.h"
 #include "rtc_base/sslfingerprint.h"
@@ -25,11 +26,11 @@ TransportDescriptionFactory::TransportDescriptionFactory()
 
 TransportDescriptionFactory::~TransportDescriptionFactory() = default;
 
-TransportDescription* TransportDescriptionFactory::CreateOffer(
+std::unique_ptr<TransportDescription> TransportDescriptionFactory::CreateOffer(
     const TransportOptions& options,
     const TransportDescription* current_description,
     IceCredentialsIterator* ice_credentials) const {
-  std::unique_ptr<TransportDescription> desc(new TransportDescription());
+  auto desc = absl::make_unique<TransportDescription>();
 
   // Generate the ICE credentials if we don't already have them.
   if (!current_description || options.ice_restart) {
@@ -54,10 +55,10 @@ TransportDescription* TransportDescriptionFactory::CreateOffer(
     }
   }
 
-  return desc.release();
+  return desc;
 }
 
-TransportDescription* TransportDescriptionFactory::CreateAnswer(
+std::unique_ptr<TransportDescription> TransportDescriptionFactory::CreateAnswer(
     const TransportDescription* offer,
     const TransportOptions& options,
     bool require_transport_attributes,
@@ -70,7 +71,7 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer(
     return NULL;
   }
 
-  std::unique_ptr<TransportDescription> desc(new TransportDescription());
+  auto desc = absl::make_unique<TransportDescription>();
   // Generate the ICE credentials if we don't already have them or ice is
   // being restarted.
   if (!current_description || options.ice_restart) {
@@ -107,7 +108,7 @@ TransportDescription* TransportDescriptionFactory::CreateAnswer(
     return NULL;
   }
 
-  return desc.release();
+  return desc;
 }
 
 bool TransportDescriptionFactory::SetSecurityInfo(TransportDescription* desc,

p2p/base/transportdescriptionfactory.h

@@ -11,6 +11,8 @@
 #ifndef P2P_BASE_TRANSPORTDESCRIPTIONFACTORY_H_
 #define P2P_BASE_TRANSPORTDESCRIPTIONFACTORY_H_
 
+#include <memory>
+
 #include "p2p/base/icecredentialsiterator.h"
 #include "p2p/base/transportdescription.h"
 #include "rtc_base/rtccertificate.h"
@@ -53,7 +55,7 @@ class TransportDescriptionFactory {
   }
 
   // Creates a transport description suitable for use in an offer.
-  TransportDescription* CreateOffer(
+  std::unique_ptr<TransportDescription> CreateOffer(
       const TransportOptions& options,
       const TransportDescription* current_description,
       IceCredentialsIterator* ice_credentials) const;
@@ -64,7 +66,7 @@ class TransportDescriptionFactory {
   // sdp-mux-attributes, and null will be returned otherwise. It's expected
   // that this will be set to false for an m= section that's in a BUNDLE group
   // but isn't the first m= section in the group.
-  TransportDescription* CreateAnswer(
+  std::unique_ptr<TransportDescription> CreateAnswer(
       const TransportDescription* offer,
       const TransportOptions& options,
       bool require_transport_attributes,

p2p/base/transportdescriptionfactory_unittest.cc

@@ -70,22 +70,22 @@ class TransportDescriptionFactoryTest : public testing::Test {
     SetDtls(dtls);
     cricket::TransportOptions options;
     // The initial offer / answer exchange.
-    std::unique_ptr<TransportDescription> offer(
-        f1_.CreateOffer(options, NULL, &ice_credentials_));
-    std::unique_ptr<TransportDescription> answer(
-        f2_.CreateAnswer(offer.get(), options, true, NULL, &ice_credentials_));
+    std::unique_ptr<TransportDescription> offer =
+        f1_.CreateOffer(options, NULL, &ice_credentials_);
+    std::unique_ptr<TransportDescription> answer =
+        f2_.CreateAnswer(offer.get(), options, true, NULL, &ice_credentials_);
 
     // Create an updated offer where we restart ice.
     options.ice_restart = true;
-    std::unique_ptr<TransportDescription> restart_offer(
-        f1_.CreateOffer(options, offer.get(), &ice_credentials_));
+    std::unique_ptr<TransportDescription> restart_offer =
+        f1_.CreateOffer(options, offer.get(), &ice_credentials_);
 
     VerifyUfragAndPasswordChanged(dtls, offer.get(), restart_offer.get());
 
     // Create a new answer. The transport ufrag and password is changed since
     // |options.ice_restart == true|
-    std::unique_ptr<TransportDescription> restart_answer(f2_.CreateAnswer(
-        restart_offer.get(), options, true, answer.get(), &ice_credentials_));
+    std::unique_ptr<TransportDescription> restart_answer = f2_.CreateAnswer(
+        restart_offer.get(), options, true, answer.get(), &ice_credentials_);
     ASSERT_TRUE(restart_answer.get() != NULL);
 
     VerifyUfragAndPasswordChanged(dtls, answer.get(), restart_answer.get());
@@ -114,21 +114,21 @@ class TransportDescriptionFactoryTest : public testing::Test {
 
     cricket::TransportOptions options;
     // The initial offer / answer exchange.
-    std::unique_ptr<TransportDescription> offer(
-        f1_.CreateOffer(options, nullptr, &ice_credentials_));
-    std::unique_ptr<TransportDescription> answer(f2_.CreateAnswer(
-        offer.get(), options, true, nullptr, &ice_credentials_));
+    std::unique_ptr<TransportDescription> offer =
+        f1_.CreateOffer(options, nullptr, &ice_credentials_);
+    std::unique_ptr<TransportDescription> answer = f2_.CreateAnswer(
+        offer.get(), options, true, nullptr, &ice_credentials_);
     VerifyRenomination(offer.get(), false);
     VerifyRenomination(answer.get(), false);
 
     options.enable_ice_renomination = true;
-    std::unique_ptr<TransportDescription> renomination_offer(
-        f1_.CreateOffer(options, offer.get(), &ice_credentials_));
+    std::unique_ptr<TransportDescription> renomination_offer =
+        f1_.CreateOffer(options, offer.get(), &ice_credentials_);
     VerifyRenomination(renomination_offer.get(), true);
 
-    std::unique_ptr<TransportDescription> renomination_answer(
+    std::unique_ptr<TransportDescription> renomination_answer =
         f2_.CreateAnswer(renomination_offer.get(), options, true, answer.get(),
-                         &ice_credentials_));
+                         &ice_credentials_);
     VerifyRenomination(renomination_answer.get(), true);
   }
 
@@ -162,8 +162,8 @@ class TransportDescriptionFactoryTest : public testing::Test {
 };
 
 TEST_F(TransportDescriptionFactoryTest, TestOfferDefault) {
-  std::unique_ptr<TransportDescription> desc(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", "");
 }
 
@@ -173,20 +173,20 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferDtls) {
   std::string digest_alg;
   ASSERT_TRUE(
       cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg));
-  std::unique_ptr<TransportDescription> desc(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", digest_alg);
   // Ensure it also works with SEC_REQUIRED.
   f1_.set_secure(cricket::SEC_REQUIRED);
-  desc.reset(f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  desc = f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", digest_alg);
 }
 
 // Test generating an offer with DTLS fails with no identity.
 TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsWithNoIdentity) {
   f1_.set_secure(cricket::SEC_ENABLED);
-  std::unique_ptr<TransportDescription> desc(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   ASSERT_TRUE(desc.get() == NULL);
 }
 
@@ -198,37 +198,36 @@ TEST_F(TransportDescriptionFactoryTest, TestOfferDtlsReofferDtls) {
   std::string digest_alg;
   ASSERT_TRUE(
       cert1_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg));
-  std::unique_ptr<TransportDescription> old_desc(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> old_desc =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   ASSERT_TRUE(old_desc.get() != NULL);
-  std::unique_ptr<TransportDescription> desc(
-      f1_.CreateOffer(TransportOptions(), old_desc.get(), &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc =
+      f1_.CreateOffer(TransportOptions(), old_desc.get(), &ice_credentials_);
   CheckDesc(desc.get(), "", old_desc->ice_ufrag, old_desc->ice_pwd, digest_alg);
 }
 
 TEST_F(TransportDescriptionFactoryTest, TestAnswerDefault) {
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   ASSERT_TRUE(offer.get() != NULL);
-  std::unique_ptr<TransportDescription> desc(f2_.CreateAnswer(
-      offer.get(), TransportOptions(), true, NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc = f2_.CreateAnswer(
+      offer.get(), TransportOptions(), true, NULL, &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", "");
-  desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL,
-                              &ice_credentials_));
+  desc = f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL,
+                          &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", "");
 }
 
 // Test that we can update an answer properly; ICE credentials shouldn't change.
 TEST_F(TransportDescriptionFactoryTest, TestReanswer) {
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   ASSERT_TRUE(offer.get() != NULL);
-  std::unique_ptr<TransportDescription> old_desc(f2_.CreateAnswer(
-      offer.get(), TransportOptions(), true, NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> old_desc = f2_.CreateAnswer(
+      offer.get(), TransportOptions(), true, NULL, &ice_credentials_);
   ASSERT_TRUE(old_desc.get() != NULL);
-  std::unique_ptr<TransportDescription> desc(
-      f2_.CreateAnswer(offer.get(), TransportOptions(), true, old_desc.get(),
-                       &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc = f2_.CreateAnswer(
+      offer.get(), TransportOptions(), true, old_desc.get(), &ice_credentials_);
   ASSERT_TRUE(desc.get() != NULL);
   CheckDesc(desc.get(), "", old_desc->ice_ufrag, old_desc->ice_pwd, "");
 }
@@ -237,11 +236,11 @@ TEST_F(TransportDescriptionFactoryTest, TestReanswer) {
 TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToNoDtls) {
   f1_.set_secure(cricket::SEC_ENABLED);
   f1_.set_certificate(cert1_);
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   ASSERT_TRUE(offer.get() != NULL);
-  std::unique_ptr<TransportDescription> desc(f2_.CreateAnswer(
-      offer.get(), TransportOptions(), true, NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc = f2_.CreateAnswer(
+      offer.get(), TransportOptions(), true, NULL, &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", "");
 }
 
@@ -250,15 +249,15 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToNoDtls) {
 TEST_F(TransportDescriptionFactoryTest, TestAnswerNoDtlsToDtls) {
   f2_.set_secure(cricket::SEC_ENABLED);
   f2_.set_certificate(cert2_);
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   ASSERT_TRUE(offer.get() != NULL);
-  std::unique_ptr<TransportDescription> desc(f2_.CreateAnswer(
-      offer.get(), TransportOptions(), true, NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc = f2_.CreateAnswer(
+      offer.get(), TransportOptions(), true, NULL, &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", "");
   f2_.set_secure(cricket::SEC_REQUIRED);
-  desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL,
-                              &ice_credentials_));
+  desc = f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL,
+                          &ice_credentials_);
   ASSERT_TRUE(desc.get() == NULL);
 }
 
@@ -276,15 +275,15 @@ TEST_F(TransportDescriptionFactoryTest, TestAnswerDtlsToDtls) {
   ASSERT_TRUE(
       cert2_->GetSSLCertificate().GetSignatureDigestAlgorithm(&digest_alg2));
 
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(TransportOptions(), NULL, &ice_credentials_);
   ASSERT_TRUE(offer.get() != NULL);
-  std::unique_ptr<TransportDescription> desc(f2_.CreateAnswer(
-      offer.get(), TransportOptions(), true, NULL, &ice_credentials_));
+  std::unique_ptr<TransportDescription> desc = f2_.CreateAnswer(
+      offer.get(), TransportOptions(), true, NULL, &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", digest_alg2);
   f2_.set_secure(cricket::SEC_REQUIRED);
-  desc.reset(f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL,
-                              &ice_credentials_));
+  desc = f2_.CreateAnswer(offer.get(), TransportOptions(), true, NULL,
+                          &ice_credentials_);
   CheckDesc(desc.get(), "", "", "", digest_alg2);
 }
 
@@ -316,11 +315,11 @@ TEST_F(TransportDescriptionFactoryTest, TestIceRenominationWithDtls) {
 // Test that offers and answers have ice-option:trickle.
 TEST_F(TransportDescriptionFactoryTest, AddsTrickleIceOption) {
   cricket::TransportOptions options;
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(options, nullptr, &ice_credentials_));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(options, nullptr, &ice_credentials_);
   EXPECT_TRUE(offer->HasOption("trickle"));
-  std::unique_ptr<TransportDescription> answer(
-      f2_.CreateAnswer(offer.get(), options, true, nullptr, &ice_credentials_));
+  std::unique_ptr<TransportDescription> answer =
+      f2_.CreateAnswer(offer.get(), options, true, nullptr, &ice_credentials_);
   EXPECT_TRUE(answer->HasOption("trickle"));
 }
 
@@ -330,8 +329,8 @@ TEST_F(TransportDescriptionFactoryTest, CreateOfferIceCredentialsIterator) {
       cricket::IceParameters("kalle", "anka", false)};
   cricket::IceCredentialsIterator credentialsIterator(credentials);
   cricket::TransportOptions options;
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(options, nullptr, &credentialsIterator));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(options, nullptr, &credentialsIterator);
   EXPECT_EQ(offer->GetIceParameters().ufrag, credentials[0].ufrag);
   EXPECT_EQ(offer->GetIceParameters().pwd, credentials[0].pwd);
 }
@@ -339,14 +338,14 @@ TEST_F(TransportDescriptionFactoryTest, CreateOfferIceCredentialsIterator) {
 // Test CreateAnswer with IceCredentialsIterator.
 TEST_F(TransportDescriptionFactoryTest, CreateAnswerIceCredentialsIterator) {
   cricket::TransportOptions options;
-  std::unique_ptr<TransportDescription> offer(
-      f1_.CreateOffer(options, nullptr, &ice_credentials_));
+  std::unique_ptr<TransportDescription> offer =
+      f1_.CreateOffer(options, nullptr, &ice_credentials_);
 
   std::vector<cricket::IceParameters> credentials = {
       cricket::IceParameters("kalle", "anka", false)};
   cricket::IceCredentialsIterator credentialsIterator(credentials);
-  std::unique_ptr<TransportDescription> answer(f1_.CreateAnswer(
-      offer.get(), options, false, nullptr, &credentialsIterator));
+  std::unique_ptr<TransportDescription> answer = f1_.CreateAnswer(
+      offer.get(), options, false, nullptr, &credentialsIterator);
   EXPECT_EQ(answer->GetIceParameters().ufrag, credentials[0].ufrag);
   EXPECT_EQ(answer->GetIceParameters().pwd, credentials[0].pwd);
 }

pc/mediasession.cc

@@ -1807,7 +1807,8 @@ bool MediaSessionDescriptionFactory::AddTransportOffer(
   return ret;
 }
 
-TransportDescription* MediaSessionDescriptionFactory::CreateTransportAnswer(
+std::unique_ptr<TransportDescription>
+MediaSessionDescriptionFactory::CreateTransportAnswer(
     const std::string& content_name,
     const SessionDescription* offer_desc,
     const TransportOptions& transport_options,
@@ -2091,10 +2092,10 @@ bool MediaSessionDescriptionFactory::AddAudioContentForAnswer(
   const AudioContentDescription* offer_audio_description =
       offer_content->media_description()->as_audio();
 
-  std::unique_ptr<TransportDescription> audio_transport(CreateTransportAnswer(
+  std::unique_ptr<TransportDescription> audio_transport = CreateTransportAnswer(
       media_description_options.mid, offer_description,
       media_description_options.transport_options, current_description,
-      bundle_transport != nullptr, ice_credentials));
+      bundle_transport != nullptr, ice_credentials);
   if (!audio_transport) {
     return false;
   }
@@ -2188,10 +2189,10 @@ bool MediaSessionDescriptionFactory::AddVideoContentForAnswer(
   const VideoContentDescription* offer_video_description =
       offer_content->media_description()->as_video();
 
-  std::unique_ptr<TransportDescription> video_transport(CreateTransportAnswer(
+  std::unique_ptr<TransportDescription> video_transport = CreateTransportAnswer(
       media_description_options.mid, offer_description,
       media_description_options.transport_options, current_description,
-      bundle_transport != nullptr, ice_credentials));
+      bundle_transport != nullptr, ice_credentials);
   if (!video_transport) {
     return false;
   }
@@ -2273,10 +2274,10 @@ bool MediaSessionDescriptionFactory::AddDataContentForAnswer(
     StreamParamsVec* current_streams,
     SessionDescription* answer,
     IceCredentialsIterator* ice_credentials) const {
-  std::unique_ptr<TransportDescription> data_transport(CreateTransportAnswer(
+  std::unique_ptr<TransportDescription> data_transport = CreateTransportAnswer(
       media_description_options.mid, offer_description,
       media_description_options.transport_options, current_description,
-      bundle_transport != nullptr, ice_credentials));
+      bundle_transport != nullptr, ice_credentials);
   if (!data_transport) {
     return false;
   }

pc/mediasession.h

@@ -15,6 +15,7 @@
 
 #include <algorithm>
 #include <map>
+#include <memory>
 #include <string>
 #include <vector>
 
@@ -197,7 +198,7 @@ class MediaSessionDescriptionFactory {
                          SessionDescription* offer,
                          IceCredentialsIterator* ice_credentials) const;
 
-  TransportDescription* CreateTransportAnswer(
+  std::unique_ptr<TransportDescription> CreateTransportAnswer(
       const std::string& content_name,
       const SessionDescription* offer_desc,
       const TransportOptions& transport_options,