Make WebRtcSessionDescriptionFactory depend on SdpOfferAnswerHandler

This factory is only used by SdpOfferAnswerHandler, so it should not
need to depend on PeerConnection.

Bug: webrtc:11995
Change-Id: Ib27d9d9fdf440be7db8890bf0e7520d0c67bde22
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189780
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32460}
This commit is contained in:
Harald Alvestrand
2020-10-21 08:59:22 +00:00
committed by Commit Bot
parent 653429c5b3
commit d89ce53daf
4 changed files with 52 additions and 49 deletions

View File

@ -581,21 +581,10 @@ bool PeerConnection::Initialize(
audio_options_.audio_jitter_buffer_enable_rtx_handling =
configuration.audio_jitter_buffer_enable_rtx_handling;
// Whether the certificate generator/certificate is null or not determines
// what PeerConnectionDescriptionFactory will do, so make sure that we give it
// the right instructions by clearing the variables if needed.
if (!dtls_enabled_) {
dependencies.cert_generator.reset();
certificate = nullptr;
} else if (certificate) {
// Favor generated certificate over the certificate generator.
dependencies.cert_generator.reset();
}
auto webrtc_session_desc_factory =
std::make_unique<WebRtcSessionDescriptionFactory>(
signaling_thread(), channel_manager(), this, session_id(),
std::move(dependencies.cert_generator), certificate,
signaling_thread(), channel_manager(), &sdp_handler_, session_id(),
dtls_enabled_, std::move(dependencies.cert_generator), certificate,
&ssrc_generator_);
webrtc_session_desc_factory->SignalCertificateReady.connect(
this, &PeerConnection::OnCertificateReady);

View File

@ -76,6 +76,7 @@ class VideoRtpReceiver;
class RtcEventLog;
class RtpTransmissionManager;
class TransceiverList;
class WebRtcSessionDescriptionFactory;
// SdpOfferAnswerHandler is a component
// of the PeerConnection object as defined
@ -118,6 +119,8 @@ class SdpOfferAnswerHandler {
const SessionDescriptionInterface* pending_local_description() const;
const SessionDescriptionInterface* pending_remote_description() const;
JsepTransportController* transport_controller();
void RestartIce();
// JSEP01
@ -528,7 +531,6 @@ class SdpOfferAnswerHandler {
cricket::ChannelManager* channel_manager() const;
TransceiverList* transceivers();
const TransceiverList* transceivers() const;
JsepTransportController* transport_controller();
DataChannelController* data_channel_controller();
const DataChannelController* data_channel_controller() const;
cricket::PortAllocator* port_allocator();

View File

@ -11,7 +11,7 @@
#include "pc/webrtc_session_description_factory.h"
#include <stddef.h>
#include <algorithm>
#include <memory>
#include <string>
#include <utility>
@ -22,6 +22,8 @@
#include "api/jsep.h"
#include "api/jsep_session_description.h"
#include "api/rtc_error.h"
#include "pc/jsep_transport_controller.h"
#include "pc/sdp_offer_answer.h"
#include "pc/session_description.h"
#include "rtc_base/checks.h"
#include "rtc_base/location.h"
@ -125,8 +127,9 @@ void WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
rtc::Thread* signaling_thread,
cricket::ChannelManager* channel_manager,
PeerConnectionInternal* pc,
SdpOfferAnswerHandler* sdp_handler,
const std::string& session_id,
bool dtls_enabled,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate,
UniqueRandomIdGenerator* ssrc_generator)
@ -139,20 +142,20 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
// to just use a random number as session id and start version from
// |kInitSessionVersion|.
session_version_(kInitSessionVersion),
cert_generator_(std::move(cert_generator)),
pc_(pc),
cert_generator_(dtls_enabled ? std::move(cert_generator) : nullptr),
sdp_handler_(sdp_handler),
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);
if (!dtls_enabled) {
SetSdesPolicy(cricket::SEC_REQUIRED);
RTC_LOG(LS_VERBOSE) << "DTLS-SRTP disabled.";
return;
}
// SRTP-SDES is disabled if DTLS is on.
SetSdesPolicy(cricket::SEC_DISABLED);
if (certificate) {
// Use |certificate|.
certificate_request_state_ = CERTIFICATE_WAITING;
@ -252,13 +255,13 @@ void WebRtcSessionDescriptionFactory::CreateAnswer(
PostCreateSessionDescriptionFailed(observer, error);
return;
}
if (!pc_->remote_description()) {
if (!sdp_handler_->remote_description()) {
error += " can't be called before SetRemoteDescription.";
RTC_LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailed(observer, error);
return;
}
if (pc_->remote_description()->GetType() != SdpType::kOffer) {
if (sdp_handler_->remote_description()->GetType() != SdpType::kOffer) {
error += " failed because remote_description is not an offer.";
RTC_LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailed(observer, error);
@ -325,12 +328,12 @@ void WebRtcSessionDescriptionFactory::OnMessage(rtc::Message* msg) {
void WebRtcSessionDescriptionFactory::InternalCreateOffer(
CreateSessionDescriptionRequest request) {
if (pc_->local_description()) {
if (sdp_handler_->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 (pc_->NeedsIceRestart(options.mid)) {
if (sdp_handler_->transport_controller()->NeedsIceRestart(options.mid)) {
options.transport_options.ice_restart = true;
}
}
@ -338,9 +341,10 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
std::unique_ptr<cricket::SessionDescription> desc =
session_desc_factory_.CreateOffer(
request.options, pc_->local_description()
? pc_->local_description()->description()
: nullptr);
request.options,
sdp_handler_->local_description()
? sdp_handler_->local_description()->description()
: nullptr);
if (!desc) {
PostCreateSessionDescriptionFailed(request.observer,
"Failed to initialize the offer.");
@ -360,11 +364,11 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
auto offer = std::make_unique<JsepSessionDescription>(
SdpType::kOffer, std::move(desc), session_id_,
rtc::ToString(session_version_++));
if (pc_->local_description()) {
if (sdp_handler_->local_description()) {
for (const cricket::MediaDescriptionOptions& options :
request.options.media_description_options) {
if (!options.transport_options.ice_restart) {
CopyCandidatesFromSessionDescription(pc_->local_description(),
CopyCandidatesFromSessionDescription(sdp_handler_->local_description(),
options.mid, offer.get());
}
}
@ -374,31 +378,34 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
CreateSessionDescriptionRequest request) {
if (pc_->remote_description()) {
if (sdp_handler_->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 =
pc_->IceRestartPending(options.mid);
// We should pass the current SSL role to the transport description
sdp_handler_->IceRestartPending(options.mid);
// We should pass the current DTLS role to the transport description
// factory, if there is already an existing ongoing session.
rtc::SSLRole ssl_role;
if (pc_->GetSslRole(options.mid, &ssl_role)) {
absl::optional<rtc::SSLRole> dtls_role =
sdp_handler_->transport_controller()->GetDtlsRole(options.mid);
if (dtls_role) {
options.transport_options.prefer_passive_role =
(rtc::SSL_SERVER == ssl_role);
(rtc::SSL_SERVER == *dtls_role);
}
}
}
std::unique_ptr<cricket::SessionDescription> desc =
session_desc_factory_.CreateAnswer(
pc_->remote_description() ? pc_->remote_description()->description()
: nullptr,
sdp_handler_->remote_description()
? sdp_handler_->remote_description()->description()
: nullptr,
request.options,
pc_->local_description() ? pc_->local_description()->description()
: nullptr);
sdp_handler_->local_description()
? sdp_handler_->local_description()->description()
: nullptr);
if (!desc) {
PostCreateSessionDescriptionFailed(request.observer,
"Failed to initialize the answer.");
@ -416,13 +423,13 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
auto answer = std::make_unique<JsepSessionDescription>(
SdpType::kAnswer, std::move(desc), session_id_,
rtc::ToString(session_version_++));
if (pc_->local_description()) {
if (sdp_handler_->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(pc_->local_description(),
CopyCandidatesFromSessionDescription(sdp_handler_->local_description(),
options.mid, answer.get());
}
}

View File

@ -12,7 +12,6 @@
#define PC_WEBRTC_SESSION_DESCRIPTION_FACTORY_H_
#include <stdint.h>
#include <memory>
#include <queue>
#include <string>
@ -22,18 +21,25 @@
#include "api/scoped_refptr.h"
#include "p2p/base/transport_description.h"
#include "p2p/base/transport_description_factory.h"
#include "pc/channel_manager.h"
#include "pc/media_session.h"
#include "pc/peer_connection_internal.h"
#include "pc/sdp_offer_answer.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/message_handler.h"
#include "rtc_base/rtc_certificate.h"
#include "rtc_base/rtc_certificate_generator.h"
#include "rtc_base/third_party/sigslot/sigslot.h"
#include "rtc_base/thread.h"
#include "rtc_base/thread_message.h"
#include "rtc_base/unique_id_generator.h"
namespace webrtc {
// Forward declaration is necessary because there's a circular dependency
// between this class and SdpOfferAnswerHandler.
// TODO(https://bugs.webrtc.org/12060): Break the dependency.
class SdpOfferAnswerHandler;
// DTLS certificate request callback class.
class WebRtcCertificateGeneratorCallback
: public rtc::RTCCertificateGeneratorCallback,
@ -80,8 +86,9 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
WebRtcSessionDescriptionFactory(
rtc::Thread* signaling_thread,
cricket::ChannelManager* channel_manager,
PeerConnectionInternal* pc,
SdpOfferAnswerHandler* sdp_handler,
const std::string& session_id,
bool dtls_enabled,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate,
rtc::UniqueRandomIdGenerator* ssrc_generator);
@ -151,9 +158,7 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
cricket::MediaSessionDescriptionFactory session_desc_factory_;
uint64_t session_version_;
const std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator_;
// TODO(jiayl): remove the dependency on peer connection once bug 2264 is
// fixed.
PeerConnectionInternal* const pc_;
SdpOfferAnswerHandler* sdp_handler_;
const std::string session_id_;
CertificateRequestState certificate_request_state_;