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}diff --git a/p2p/base/transportdescriptionfactory.cc b/p2p/base/transportdescriptionfactory.cc
index b03086c..4e4f724 100644
--- a/p2p/base/transportdescriptionfactory.cc
+++ b/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() = 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 @@
}
}
- 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 @@
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 @@
return NULL;
}
- return desc.release();
+ return desc;
}
bool TransportDescriptionFactory::SetSecurityInfo(TransportDescription* desc,
diff --git a/p2p/base/transportdescriptionfactory.h b/p2p/base/transportdescriptionfactory.h
index dc1476a..55e5b9a 100644
--- a/p2p/base/transportdescriptionfactory.h
+++ b/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 @@
}
// 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 @@
// 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,
diff --git a/p2p/base/transportdescriptionfactory_unittest.cc b/p2p/base/transportdescriptionfactory_unittest.cc
index 2eae3ef..e8f623a 100644
--- a/p2p/base/transportdescriptionfactory_unittest.cc
+++ b/p2p/base/transportdescriptionfactory_unittest.cc
@@ -70,22 +70,22 @@
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 @@
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 @@
};
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 @@
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 @@
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, 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, 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 @@
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 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 @@
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 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);
}
diff --git a/pc/mediasession.cc b/pc/mediasession.cc
index 2b7750b..5f6ffd7 100644
--- a/pc/mediasession.cc
+++ b/pc/mediasession.cc
@@ -1807,7 +1807,8 @@
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 @@
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 @@
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 @@
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;
}
diff --git a/pc/mediasession.h b/pc/mediasession.h
index 13ecf70..132e0a6 100644
--- a/pc/mediasession.h
+++ b/pc/mediasession.h
@@ -15,6 +15,7 @@
#include <algorithm>
#include <map>
+#include <memory>
#include <string>
#include <vector>
@@ -197,7 +198,7 @@
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,