From 6d64e9a4e0d4dcbf741584e0fc77bf8bedc50fc4 Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Tue, 12 Sep 2017 09:44:05 -0700 Subject: [PATCH] Remove JsepSessionDescription's string Initialize method Most clients already use webrtc::CreateSessionDescription which does the same thing and has the benefit of initializing in one step instead of two and freeing the newly-created session description if there was a parse error. Bug: None Change-Id: Ibeafdf7a6dd73eaea696700bc5eb420838371b75 Reviewed-on: https://chromium-review.googlesource.com/662402 Reviewed-by: Taylor Brandstetter Commit-Queue: Steve Anton Cr-Commit-Position: refs/heads/master@{#19808} --- webrtc/api/jsepsessiondescription.h | 3 -- webrtc/pc/jsepsessiondescription.cc | 7 +---- webrtc/pc/jsepsessiondescription_unittest.cc | 3 +- webrtc/pc/webrtcsession_unittest.cc | 33 +++++++++++--------- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/webrtc/api/jsepsessiondescription.h b/webrtc/api/jsepsessiondescription.h index cb234b09ca..69945b411e 100644 --- a/webrtc/api/jsepsessiondescription.h +++ b/webrtc/api/jsepsessiondescription.h @@ -35,9 +35,6 @@ class JsepSessionDescription : public SessionDescriptionInterface { explicit JsepSessionDescription(const std::string& type); virtual ~JsepSessionDescription(); - // |error| may be null. - bool Initialize(const std::string& sdp, SdpParseError* error); - // Takes ownership of |description|. // TODO(deadbeef): Make this use an std::unique_ptr<>, so ownership logic is // more clear. diff --git a/webrtc/pc/jsepsessiondescription.cc b/webrtc/pc/jsepsessiondescription.cc index 2e972ea3b4..bdb36955b0 100644 --- a/webrtc/pc/jsepsessiondescription.cc +++ b/webrtc/pc/jsepsessiondescription.cc @@ -121,7 +121,7 @@ SessionDescriptionInterface* CreateSessionDescription(const std::string& type, } JsepSessionDescription* jsep_desc = new JsepSessionDescription(type); - if (!jsep_desc->Initialize(sdp, error)) { + if (!SdpDeserialize(sdp, jsep_desc, error)) { delete jsep_desc; return NULL; } @@ -148,11 +148,6 @@ bool JsepSessionDescription::Initialize( return true; } -bool JsepSessionDescription::Initialize(const std::string& sdp, - SdpParseError* error) { - return SdpDeserialize(sdp, this, error); -} - bool JsepSessionDescription::AddCandidate( const IceCandidateInterface* candidate) { if (!candidate || candidate->sdp_mline_index() < 0) diff --git a/webrtc/pc/jsepsessiondescription_unittest.cc b/webrtc/pc/jsepsessiondescription_unittest.cc index 4d8218a9c6..62052af105 100644 --- a/webrtc/pc/jsepsessiondescription_unittest.cc +++ b/webrtc/pc/jsepsessiondescription_unittest.cc @@ -13,6 +13,7 @@ #include "webrtc/api/jsepicecandidate.h" #include "webrtc/api/jsepsessiondescription.h" +#include "webrtc/api/webrtcsdp.h" #include "webrtc/p2p/base/candidate.h" #include "webrtc/p2p/base/p2pconstants.h" #include "webrtc/p2p/base/sessiondescription.h" @@ -97,7 +98,7 @@ class JsepSessionDescriptionTest : public testing::Test { SessionDescriptionInterface* DeSerialize(const std::string& sdp) { JsepSessionDescription* desc(new JsepSessionDescription("dummy")); - EXPECT_TRUE(desc->Initialize(sdp, NULL)); + EXPECT_TRUE(webrtc::SdpDeserialize(sdp, desc, nullptr)); return desc; } diff --git a/webrtc/pc/webrtcsession_unittest.cc b/webrtc/pc/webrtcsession_unittest.cc index 20883270b2..862196557b 100644 --- a/webrtc/pc/webrtcsession_unittest.cc +++ b/webrtc/pc/webrtcsession_unittest.cc @@ -1218,8 +1218,9 @@ class WebRtcSessionTest kSessionVersion, current_desc); } - JsepSessionDescription* CreateRemoteOfferWithSctpPort( - const char* sctp_stream_name, int new_port, + SessionDescriptionInterface* CreateRemoteOfferWithSctpPort( + const char* sctp_stream_name, + int new_port, cricket::MediaSessionOptions options) { options.data_channel_type = cricket::DCT_SCTP; GetOptionsForRemoteOffer(&options); @@ -1227,8 +1228,9 @@ class WebRtcSessionTest } // Takes ownership of offer_basis (and deletes it). - JsepSessionDescription* ChangeSDPSctpPort( - int new_port, webrtc::SessionDescriptionInterface *offer_basis) { + SessionDescriptionInterface* ChangeSDPSctpPort( + int new_port, + webrtc::SessionDescriptionInterface* offer_basis) { // Stringify the input SDP, swap the 5000 for 'new_port' and create a new // SessionDescription from the mutated string. const char* default_port_str = "5000"; @@ -1239,10 +1241,9 @@ class WebRtcSessionTest rtc::replace_substrs(default_port_str, strlen(default_port_str), new_port_str, strlen(new_port_str), &offer_str); - JsepSessionDescription* offer = new JsepSessionDescription( - offer_basis->type()); + SessionDescriptionInterface* offer = + CreateSessionDescription(offer_basis->type(), offer_str, nullptr); delete offer_basis; - offer->Initialize(offer_str, NULL); return offer; } @@ -3683,14 +3684,16 @@ TEST_F(WebRtcSessionTest, TestDisabledRtcpMuxWithBundleEnabled) { rtc::replace_substrs(rtcp_mux.c_str(), rtcp_mux.length(), xrtcp_mux.c_str(), xrtcp_mux.length(), &offer_str); - JsepSessionDescription* local_offer = - new JsepSessionDescription(JsepSessionDescription::kOffer); - EXPECT_TRUE((local_offer)->Initialize(offer_str, NULL)); + SessionDescriptionInterface* local_offer = CreateSessionDescription( + SessionDescriptionInterface::kOffer, offer_str, nullptr); + ASSERT_TRUE(local_offer); SetLocalDescriptionOfferExpectError(kBundleWithoutRtcpMux, local_offer); - JsepSessionDescription* remote_offer = - new JsepSessionDescription(JsepSessionDescription::kOffer); - EXPECT_TRUE((remote_offer)->Initialize(offer_str, NULL)); + + SessionDescriptionInterface* remote_offer = CreateSessionDescription( + SessionDescriptionInterface::kOffer, offer_str, nullptr); + ASSERT_TRUE(remote_offer); SetRemoteDescriptionOfferExpectError(kBundleWithoutRtcpMux, remote_offer); + // Trying unmodified SDP. SetLocalDescriptionWithoutError(offer); } @@ -4189,8 +4192,8 @@ TEST_P(WebRtcSessionTest, TestSctpDataChannelSendPortParsing) { // let the session description get parsed. That'll get the proper codecs // into the stream. cricket::MediaSessionOptions options; - JsepSessionDescription* offer = CreateRemoteOfferWithSctpPort( - "stream1", new_send_port, options); + SessionDescriptionInterface* offer = + CreateRemoteOfferWithSctpPort("stream1", new_send_port, options); // SetRemoteDescription will take the ownership of the offer. SetRemoteDescriptionWithoutError(offer);