Move almost all references from WebRtcSession to PeerConnection
WebRtcSession is being merged into PeerConnection, and to make the
code review easier this is the first step towards achieving that.
Bug: webrtc:8323
Change-Id: I33778e46f20cb14089dff4328947868e207476bd
Reviewed-on: https://webrtc-review.googlesource.com/8760
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20413}diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 5007f33..0420b60 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -487,10 +487,8 @@
stats_collector_ = RTCStatsCollector::Create(this);
// Initialize the WebRtcSession. It creates transport channels etc.
- if (!session_->Initialize(factory_->options(), std::move(cert_generator),
- configuration)) {
- return false;
- }
+ session_->Initialize(factory_->options(), std::move(cert_generator),
+ configuration, this);
// Register PeerConnection as receiver of local ice candidates.
// All the callbacks will be posted to the application from PeerConnection.
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index 99737f9..9163c36 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -209,6 +209,15 @@
virtual bool GetRemoteTrackIdBySsrc(uint32_t ssrc, std::string* track_id) {
return session_->GetRemoteTrackIdBySsrc(ssrc, track_id);
}
+ bool IceRestartPending(const std::string& content_name) const {
+ return session_->IceRestartPending(content_name);
+ }
+ bool NeedsIceRestart(const std::string& content_name) const {
+ return session_->NeedsIceRestart(content_name);
+ }
+ bool GetSslRole(const std::string& content_name, rtc::SSLRole* role) {
+ return session_->GetSslRole(content_name, role);
+ }
// This is needed for stats tests to inject a MockWebRtcSession. Once
// WebRtcSession has been merged in, this will no longer be needed.
diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc
index 36a4d6e..d20f2ed 100644
--- a/pc/rtcstatscollector.cc
+++ b/pc/rtcstatscollector.cc
@@ -22,7 +22,6 @@
#include "p2p/base/p2pconstants.h"
#include "p2p/base/port.h"
#include "pc/peerconnection.h"
-#include "pc/webrtcsession.h"
#include "rtc_base/checks.h"
#include "rtc_base/stringutils.h"
#include "rtc_base/timeutils.h"
@@ -77,7 +76,7 @@
}
std::string RTCTransportStatsIDFromBaseChannel(
- const ProxyTransportMap& proxy_to_transport,
+ const std::map<std::string, std::string>& proxy_to_transport,
const cricket::BaseChannel& base_channel) {
auto proxy_it = proxy_to_transport.find(base_channel.content_name());
if (proxy_it == proxy_to_transport.cend())
diff --git a/pc/statscollector.cc b/pc/statscollector.cc
index eedd228..919ea46 100644
--- a/pc/statscollector.cc
+++ b/pc/statscollector.cc
@@ -50,8 +50,9 @@
typedef TypeForAdd<int64_t> Int64ForAdd;
typedef TypeForAdd<int> IntForAdd;
-StatsReport::Id GetTransportIdFromProxy(const ProxyTransportMap& map,
- const std::string& proxy) {
+StatsReport::Id GetTransportIdFromProxy(
+ const std::map<std::string, std::string>& map,
+ const std::string& proxy) {
RTC_DCHECK(!proxy.empty());
auto found = map.find(proxy);
if (found == map.end()) {
diff --git a/pc/statscollector.h b/pc/statscollector.h
index 568d1ee..5c90066 100644
--- a/pc/statscollector.h
+++ b/pc/statscollector.h
@@ -21,7 +21,6 @@
#include "api/mediastreaminterface.h"
#include "api/peerconnectioninterface.h"
#include "api/statstypes.h"
-#include "pc/webrtcsession.h"
namespace webrtc {
@@ -139,7 +138,7 @@
// Raw pointer to the peer connection the statistics are gathered from.
PeerConnection* const pc_;
double stats_gathering_started_;
- ProxyTransportMap proxy_to_transport_;
+ std::map<std::string, std::string> proxy_to_transport_;
// TODO(tommi): We appear to be holding on to raw pointers to reference
// counted objects? We should be using scoped_refptr here.
diff --git a/pc/webrtcsession.cc b/pc/webrtcsession.cc
index bbe9964..66b515c 100644
--- a/pc/webrtcsession.cc
+++ b/pc/webrtcsession.cc
@@ -536,10 +536,11 @@
LOG(LS_INFO) << "Session: " << id() << " is destroyed.";
}
-bool WebRtcSession::Initialize(
+void WebRtcSession::Initialize(
const PeerConnectionFactoryInterface::Options& options,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
- const PeerConnectionInterface::RTCConfiguration& rtc_configuration) {
+ const PeerConnectionInterface::RTCConfiguration& rtc_configuration,
+ PeerConnection* pc) {
bundle_policy_ = rtc_configuration.bundle_policy;
rtcp_mux_policy_ = rtc_configuration.rtcp_mux_policy;
transport_controller_->SetSslMaxProtocolVersion(options.ssl_max_version);
@@ -589,24 +590,20 @@
audio_options_.audio_jitter_buffer_fast_accelerate = rtc::Optional<bool>(
rtc_configuration.audio_jitter_buffer_fast_accelerate);
+ // Whether the certificate generator/certificate is null or not determines
+ // what WebRtcSessionDescriptionFactory will do, so make sure that we give it
+ // the right instructions by clearing the variables if needed.
if (!dtls_enabled_) {
- // Construct with DTLS disabled.
- webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory(
- signaling_thread(), channel_manager_, this, id(),
- std::unique_ptr<rtc::RTCCertificateGeneratorInterface>()));
- } else {
- // Construct with DTLS enabled.
- if (!certificate) {
- webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory(
- signaling_thread(), channel_manager_, this, id(),
- std::move(cert_generator)));
- } else {
- // Use the already generated certificate.
- webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory(
- signaling_thread(), channel_manager_, this, id(), certificate));
- }
+ cert_generator.reset();
+ certificate = nullptr;
+ } else if (certificate) {
+ // Favor generated certificate over the certificate generator.
+ cert_generator.reset();
}
+ webrtc_session_desc_factory_.reset(new WebRtcSessionDescriptionFactory(
+ signaling_thread(), channel_manager_, pc, id(), std::move(cert_generator),
+ certificate));
webrtc_session_desc_factory_->SignalCertificateReady.connect(
this, &WebRtcSession::OnCertificateReady);
@@ -616,8 +613,6 @@
webrtc_session_desc_factory_->set_enable_encrypted_rtp_header_extensions(
options.crypto_options.enable_encrypted_rtp_header_extensions);
-
- return true;
}
void WebRtcSession::Close() {
diff --git a/pc/webrtcsession.h b/pc/webrtcsession.h
index 37d553a..07202ee 100644
--- a/pc/webrtcsession.h
+++ b/pc/webrtcsession.h
@@ -46,6 +46,7 @@
class IceRestartAnswerLatch;
class JsepIceCandidate;
class MediaStreamSignaling;
+class PeerConnection;
class RtcEventLog;
class WebRtcSessionDescriptionFactory;
@@ -99,14 +100,11 @@
};
// Statistics for all the transports of the session.
-typedef std::map<std::string, cricket::TransportStats> TransportStatsMap;
-typedef std::map<std::string, std::string> ProxyTransportMap;
-
// TODO(pthatcher): Think of a better name for this. We already have
// a TransportStats in transport.h. Perhaps TransportsStats?
struct SessionStats {
- ProxyTransportMap proxy_to_transport;
- TransportStatsMap transport_stats;
+ std::map<std::string, std::string> proxy_to_transport;
+ std::map<std::string, cricket::TransportStats> transport_stats;
};
struct ChannelNamePair {
@@ -172,10 +170,11 @@
// The ID of this session.
const std::string& id() const { return sid_; }
- bool Initialize(
+ void Initialize(
const PeerConnectionFactoryInterface::Options& options,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
- const PeerConnectionInterface::RTCConfiguration& rtc_configuration);
+ const PeerConnectionInterface::RTCConfiguration& rtc_configuration,
+ PeerConnection* pc);
// Deletes the voice, video and data channel and changes the session state
// to STATE_CLOSED.
void Close();
diff --git a/pc/webrtcsessiondescriptionfactory.cc b/pc/webrtcsessiondescriptionfactory.cc
index ea3e6ae..bbab646 100644
--- a/pc/webrtcsessiondescriptionfactory.cc
+++ b/pc/webrtcsessiondescriptionfactory.cc
@@ -15,7 +15,7 @@
#include "api/jsep.h"
#include "api/jsepsessiondescription.h"
#include "api/mediaconstraintsinterface.h"
-#include "pc/webrtcsession.h"
+#include "pc/peerconnection.h"
#include "rtc_base/checks.h"
#include "rtc_base/sslidentity.h"
@@ -118,7 +118,7 @@
WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
rtc::Thread* signaling_thread,
cricket::ChannelManager* channel_manager,
- WebRtcSession* session,
+ PeerConnection* pc,
const std::string& session_id,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate)
@@ -130,10 +130,11 @@
// |kInitSessionVersion|.
session_version_(kInitSessionVersion),
cert_generator_(std::move(cert_generator)),
- session_(session),
+ pc_(pc),
session_id_(session_id),
certificate_request_state_(CERTIFICATE_NOT_NEEDED) {
RTC_DCHECK(signaling_thread_);
+ RTC_DCHECK(!(cert_generator_ && certificate));
bool dtls_enabled = cert_generator_ || certificate;
// SRTP-SDES is disabled if DTLS is on.
SetSdesPolicy(dtls_enabled ? cricket::SEC_DISABLED : cricket::SEC_REQUIRED);
@@ -175,36 +176,6 @@
}
}
-WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
- rtc::Thread* signaling_thread,
- cricket::ChannelManager* channel_manager,
- WebRtcSession* session,
- const std::string& session_id,
- std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator)
- : WebRtcSessionDescriptionFactory(
- signaling_thread,
- channel_manager,
- session,
- session_id,
- std::move(cert_generator),
- nullptr) {
-}
-
-WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
- rtc::Thread* signaling_thread,
- cricket::ChannelManager* channel_manager,
- WebRtcSession* session,
- const std::string& session_id,
- const rtc::scoped_refptr<rtc::RTCCertificate>& certificate)
- : WebRtcSessionDescriptionFactory(signaling_thread,
- channel_manager,
- session,
- session_id,
- nullptr,
- certificate) {
- RTC_DCHECK(certificate);
-}
-
WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() {
RTC_DCHECK(signaling_thread_->IsCurrent());
@@ -270,14 +241,13 @@
PostCreateSessionDescriptionFailed(observer, error);
return;
}
- if (!session_->remote_description()) {
+ if (!pc_->remote_description()) {
error += " can't be called before SetRemoteDescription.";
LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailed(observer, error);
return;
}
- if (session_->remote_description()->type() !=
- JsepSessionDescription::kOffer) {
+ if (pc_->remote_description()->type() != JsepSessionDescription::kOffer) {
error += " failed because remote_description is not an offer.";
LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailed(observer, error);
@@ -344,20 +314,20 @@
void WebRtcSessionDescriptionFactory::InternalCreateOffer(
CreateSessionDescriptionRequest request) {
- if (session_->local_description()) {
+ if (pc_->local_description()) {
// If the needs-ice-restart flag is set as described by JSEP, we should
// generate an offer with a new ufrag/password to trigger an ICE restart.
for (cricket::MediaDescriptionOptions& options :
request.options.media_description_options) {
- if (session_->NeedsIceRestart(options.mid)) {
+ if (pc_->NeedsIceRestart(options.mid)) {
options.transport_options.ice_restart = true;
}
}
}
cricket::SessionDescription* desc(session_desc_factory_.CreateOffer(
- request.options, session_->local_description()
- ? session_->local_description()->description()
+ request.options, pc_->local_description()
+ ? pc_->local_description()->description()
: nullptr));
// RFC 3264
// When issuing an offer that modifies the session,
@@ -378,11 +348,11 @@
"Failed to initialize the offer.");
return;
}
- if (session_->local_description()) {
+ if (pc_->local_description()) {
for (const cricket::MediaDescriptionOptions& options :
request.options.media_description_options) {
if (!options.transport_options.ice_restart) {
- CopyCandidatesFromSessionDescription(session_->local_description(),
+ CopyCandidatesFromSessionDescription(pc_->local_description(),
options.mid, offer);
}
}
@@ -392,18 +362,18 @@
void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
CreateSessionDescriptionRequest request) {
- if (session_->remote_description()) {
+ if (pc_->remote_description()) {
for (cricket::MediaDescriptionOptions& options :
request.options.media_description_options) {
// According to http://tools.ietf.org/html/rfc5245#section-9.2.1.1
// an answer should also contain new ICE ufrag and password if an offer
// has been received with new ufrag and password.
options.transport_options.ice_restart =
- session_->IceRestartPending(options.mid);
+ pc_->IceRestartPending(options.mid);
// We should pass the current SSL role to the transport description
// factory, if there is already an existing ongoing session.
rtc::SSLRole ssl_role;
- if (session_->GetSslRole(options.mid, &ssl_role)) {
+ if (pc_->GetSslRole(options.mid, &ssl_role)) {
options.transport_options.prefer_passive_role =
(rtc::SSL_SERVER == ssl_role);
}
@@ -411,12 +381,11 @@
}
cricket::SessionDescription* desc(session_desc_factory_.CreateAnswer(
- session_->remote_description()
- ? session_->remote_description()->description()
- : nullptr,
- request.options, session_->local_description()
- ? session_->local_description()->description()
- : nullptr));
+ pc_->remote_description() ? pc_->remote_description()->description()
+ : nullptr,
+ request.options,
+ pc_->local_description() ? pc_->local_description()->description()
+ : nullptr));
// RFC 3264
// If the answer is different from the offer in any way (different IP
// addresses, ports, etc.), the origin line MUST be different in the answer.
@@ -434,13 +403,13 @@
"Failed to initialize the answer.");
return;
}
- if (session_->local_description()) {
+ if (pc_->local_description()) {
// Include all local ICE candidates in the SessionDescription unless
// the remote peer has requested an ICE restart.
for (const cricket::MediaDescriptionOptions& options :
request.options.media_description_options) {
if (!options.transport_options.ice_restart) {
- CopyCandidatesFromSessionDescription(session_->local_description(),
+ CopyCandidatesFromSessionDescription(pc_->local_description(),
options.mid, answer);
}
}
diff --git a/pc/webrtcsessiondescriptionfactory.h b/pc/webrtcsessiondescriptionfactory.h
index 231f718..5c3e18e 100644
--- a/pc/webrtcsessiondescriptionfactory.h
+++ b/pc/webrtcsessiondescriptionfactory.h
@@ -29,6 +29,7 @@
namespace webrtc {
class CreateSessionDescriptionObserver;
class MediaConstraintsInterface;
+class PeerConnection;
class SessionDescriptionInterface;
class WebRtcSession;
@@ -74,20 +75,16 @@
class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
public sigslot::has_slots<> {
public:
- // If |certificate_generator| is not null, DTLS is enabled and a default
- // certificate is generated asynchronously; otherwise DTLS is disabled.
+ // Can specify either a |cert_generator| or |certificate| to enable DTLS. If
+ // a certificate generator is given, starts generating the certificate
+ // asynchronously. If a certificate is given, will use that for identifying
+ // over DTLS. If neither is specified, DTLS is disabled.
WebRtcSessionDescriptionFactory(
rtc::Thread* signaling_thread,
cricket::ChannelManager* channel_manager,
- WebRtcSession* session,
+ PeerConnection* pc,
const std::string& session_id,
- std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator);
- // Construct with DTLS enabled using the specified |certificate|.
- WebRtcSessionDescriptionFactory(
- rtc::Thread* signaling_thread,
- cricket::ChannelManager* channel_manager,
- WebRtcSession* session,
- const std::string& session_id,
+ std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
virtual ~WebRtcSessionDescriptionFactory();
@@ -126,16 +123,6 @@
CERTIFICATE_FAILED,
};
- // If |certificate_generator| or |certificate| is not null DTLS is enabled,
- // otherwise DTLS is disabled.
- WebRtcSessionDescriptionFactory(
- rtc::Thread* signaling_thread,
- cricket::ChannelManager* channel_manager,
- WebRtcSession* session,
- const std::string& session_id,
- std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
- const rtc::scoped_refptr<rtc::RTCCertificate>& certificate);
-
// MessageHandler implementation.
virtual void OnMessage(rtc::Message* msg);
@@ -161,8 +148,9 @@
cricket::MediaSessionDescriptionFactory session_desc_factory_;
uint64_t session_version_;
const std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator_;
- // TODO(jiayl): remove the dependency on session once bug 2264 is fixed.
- WebRtcSession* const session_;
+ // TODO(jiayl): remove the dependency on peer connection once bug 2264 is
+ // fixed.
+ PeerConnection* const pc_;
const std::string session_id_;
CertificateRequestState certificate_request_state_;