Break circular dependency on WebRtcSessionDescriptionFactory

After this change, SdpOfferAnswerHandler implements a read-only
interface called SdpStateProvider, which allows enough access
for WebRtcSessionDescriptionFactory to learn what it needs to know.

Bug: webrtc:12060
Change-Id: Ic888b5027b2df5fee407d32b89da66ff044c40de
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/190145
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#32486}
This commit is contained in:
Harald Alvestrand
2020-10-23 13:30:46 +00:00
committed by Commit Bot
parent 6216693363
commit f01bd6c266
6 changed files with 126 additions and 49 deletions

View File

@ -219,6 +219,7 @@ rtc_library("peerconnection") {
":rtp_sender",
":rtp_transceiver",
":rtp_transmission_manager",
":sdp_state_provider",
":stats_collector_interface",
":transceiver_list",
":usage_pattern",
@ -561,6 +562,14 @@ rtc_source_set("jitter_buffer_delay_interface") {
]
}
rtc_source_set("sdp_state_provider") {
sources = [ "sdp_state_provider.h" ]
deps = [
":rtc_pc_base",
"../api:libjingle_peerconnection_api",
]
}
rtc_source_set("jitter_buffer_delay_proxy") {
sources = [ "jitter_buffer_delay_proxy.h" ]
deps = [

View File

@ -15,6 +15,7 @@
#include <map>
#include <queue>
#include <type_traits>
#include <utility>
#include "absl/algorithm/container.h"
#include "absl/strings/string_view.h"
@ -1023,6 +1024,10 @@ const TransceiverList* SdpOfferAnswerHandler::transceivers() const {
JsepTransportController* SdpOfferAnswerHandler::transport_controller() {
return pc_->transport_controller();
}
const JsepTransportController* SdpOfferAnswerHandler::transport_controller()
const {
return pc_->transport_controller();
}
DataChannelController* SdpOfferAnswerHandler::data_channel_controller() {
return pc_->data_channel_controller();
}
@ -2768,6 +2773,16 @@ bool SdpOfferAnswerHandler::IceRestartPending(
pending_ice_restarts_.end();
}
bool SdpOfferAnswerHandler::NeedsIceRestart(
const std::string& content_name) const {
return transport_controller()->NeedsIceRestart(content_name);
}
absl::optional<rtc::SSLRole> SdpOfferAnswerHandler::GetDtlsRole(
const std::string& mid) const {
return transport_controller()->GetDtlsRole(mid);
}
void SdpOfferAnswerHandler::UpdateNegotiationNeeded() {
RTC_DCHECK_RUN_ON(signaling_thread());
if (!IsUnifiedPlan()) {

View File

@ -37,6 +37,7 @@
#include "api/set_remote_description_observer_interface.h"
#include "api/transport/data_channel_transport_interface.h"
#include "api/turn_customizer.h"
#include "api/video/video_bitrate_allocator_factory.h"
#include "media/base/media_channel.h"
#include "media/base/stream_params.h"
#include "p2p/base/port_allocator.h"
@ -56,6 +57,7 @@
#include "pc/rtp_transceiver.h"
#include "pc/rtp_transmission_manager.h"
#include "pc/sctp_transport.h"
#include "pc/sdp_state_provider.h"
#include "pc/session_description.h"
#include "pc/stats_collector.h"
#include "pc/stream_collection.h"
@ -65,7 +67,10 @@
#include "rtc_base/experiments/field_trial_parser.h"
#include "rtc_base/operations_chain.h"
#include "rtc_base/race_checker.h"
#include "rtc_base/rtc_certificate.h"
#include "rtc_base/ssl_stream_adapter.h"
#include "rtc_base/synchronization/sequence_checker.h"
#include "rtc_base/third_party/sigslot/sigslot.h"
#include "rtc_base/thread.h"
#include "rtc_base/thread_annotations.h"
#include "rtc_base/unique_id_generator.h"
@ -73,14 +78,6 @@
namespace webrtc {
class MediaStreamObserver;
class PeerConnection;
class VideoRtpReceiver;
class RtcEventLog;
class RtpTransmissionManager;
class TransceiverList;
class WebRtcSessionDescriptionFactory;
// SdpOfferAnswerHandler is a component
// of the PeerConnection object as defined
// by the PeerConnectionInterface API surface.
@ -88,7 +85,8 @@ class WebRtcSessionDescriptionFactory;
// - Parsing and interpreting SDP.
// - Generating offers and answers based on the current state.
// This class lives on the signaling thread.
class SdpOfferAnswerHandler : public sigslot::has_slots<> {
class SdpOfferAnswerHandler : public SdpStateProvider,
public sigslot::has_slots<> {
public:
explicit SdpOfferAnswerHandler(PeerConnection* pc);
~SdpOfferAnswerHandler();
@ -114,16 +112,22 @@ class SdpOfferAnswerHandler : public sigslot::has_slots<> {
// Called as part of destroying the owning PeerConnection.
void PrepareForShutdown();
PeerConnectionInterface::SignalingState signaling_state() const;
// Implementation of SdpStateProvider
PeerConnectionInterface::SignalingState signaling_state() const override;
const SessionDescriptionInterface* local_description() const;
const SessionDescriptionInterface* remote_description() const;
const SessionDescriptionInterface* current_local_description() const;
const SessionDescriptionInterface* current_remote_description() const;
const SessionDescriptionInterface* pending_local_description() const;
const SessionDescriptionInterface* pending_remote_description() const;
const SessionDescriptionInterface* local_description() const override;
const SessionDescriptionInterface* remote_description() const override;
const SessionDescriptionInterface* current_local_description() const override;
const SessionDescriptionInterface* current_remote_description()
const override;
const SessionDescriptionInterface* pending_local_description() const override;
const SessionDescriptionInterface* pending_remote_description()
const override;
JsepTransportController* transport_controller();
bool NeedsIceRestart(const std::string& content_name) const override;
bool IceRestartPending(const std::string& content_name) const override;
absl::optional<rtc::SSLRole> GetDtlsRole(
const std::string& mid) const override;
void RestartIce();
@ -168,7 +172,6 @@ class SdpOfferAnswerHandler : public sigslot::has_slots<> {
absl::optional<bool> is_caller();
bool HasNewIceCredentials();
bool IceRestartPending(const std::string& content_name) const;
void UpdateNegotiationNeeded();
void SetHavePendingRtpDataChannel() {
RTC_DCHECK_RUN_ON(signaling_thread());
@ -560,6 +563,8 @@ class SdpOfferAnswerHandler : public sigslot::has_slots<> {
const cricket::PortAllocator* port_allocator() const;
RtpTransmissionManager* rtp_manager();
const RtpTransmissionManager* rtp_manager() const;
JsepTransportController* transport_controller();
const JsepTransportController* transport_controller() const;
// ===================================================================
const cricket::AudioOptions& audio_options() { return audio_options_; }
const cricket::VideoOptions& video_options() { return video_options_; }

54
pc/sdp_state_provider.h Normal file
View File

@ -0,0 +1,54 @@
/*
* Copyright 2020 The WebRTC project authors. All Rights Reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/
#ifndef PC_SDP_STATE_PROVIDER_H_
#define PC_SDP_STATE_PROVIDER_H_
#include <string>
#include "api/jsep.h"
#include "api/peer_connection_interface.h"
namespace webrtc {
// This interface provides access to the state of an SDP offer/answer
// negotiation.
//
// All the functions are const, so using this interface serves as
// assurance that the user is not modifying the state.
class SdpStateProvider {
public:
virtual ~SdpStateProvider() {}
virtual PeerConnectionInterface::SignalingState signaling_state() const = 0;
virtual const SessionDescriptionInterface* local_description() const = 0;
virtual const SessionDescriptionInterface* remote_description() const = 0;
virtual const SessionDescriptionInterface* current_local_description()
const = 0;
virtual const SessionDescriptionInterface* current_remote_description()
const = 0;
virtual const SessionDescriptionInterface* pending_local_description()
const = 0;
virtual const SessionDescriptionInterface* pending_remote_description()
const = 0;
// Whether an ICE restart has been asked for. Used in CreateOffer.
virtual bool NeedsIceRestart(const std::string& content_name) const = 0;
// Whether an ICE restart was indicated in the remote offer.
// Used in CreateAnswer.
virtual bool IceRestartPending(const std::string& content_name) const = 0;
virtual absl::optional<rtc::SSLRole> GetDtlsRole(
const std::string& mid) const = 0;
};
} // namespace webrtc
#endif // PC_SDP_STATE_PROVIDER_H_

View File

@ -14,6 +14,7 @@
#include <algorithm>
#include <memory>
#include <string>
#include <type_traits>
#include <utility>
#include <vector>
@ -22,8 +23,7 @@
#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/sdp_state_provider.h"
#include "pc/session_description.h"
#include "rtc_base/checks.h"
#include "rtc_base/location.h"
@ -127,7 +127,7 @@ void WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
rtc::Thread* signaling_thread,
cricket::ChannelManager* channel_manager,
SdpOfferAnswerHandler* sdp_handler,
const SdpStateProvider* sdp_info,
const std::string& session_id,
bool dtls_enabled,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
@ -143,7 +143,7 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
// |kInitSessionVersion|.
session_version_(kInitSessionVersion),
cert_generator_(dtls_enabled ? std::move(cert_generator) : nullptr),
sdp_handler_(sdp_handler),
sdp_info_(sdp_info),
session_id_(session_id),
certificate_request_state_(CERTIFICATE_NOT_NEEDED) {
RTC_DCHECK(signaling_thread_);
@ -255,13 +255,13 @@ void WebRtcSessionDescriptionFactory::CreateAnswer(
PostCreateSessionDescriptionFailed(observer, error);
return;
}
if (!sdp_handler_->remote_description()) {
if (!sdp_info_->remote_description()) {
error += " can't be called before SetRemoteDescription.";
RTC_LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailed(observer, error);
return;
}
if (sdp_handler_->remote_description()->GetType() != SdpType::kOffer) {
if (sdp_info_->remote_description()->GetType() != SdpType::kOffer) {
error += " failed because remote_description is not an offer.";
RTC_LOG(LS_ERROR) << error;
PostCreateSessionDescriptionFailed(observer, error);
@ -328,12 +328,12 @@ void WebRtcSessionDescriptionFactory::OnMessage(rtc::Message* msg) {
void WebRtcSessionDescriptionFactory::InternalCreateOffer(
CreateSessionDescriptionRequest request) {
if (sdp_handler_->local_description()) {
if (sdp_info_->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 (sdp_handler_->transport_controller()->NeedsIceRestart(options.mid)) {
if (sdp_info_->NeedsIceRestart(options.mid)) {
options.transport_options.ice_restart = true;
}
}
@ -341,10 +341,9 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
std::unique_ptr<cricket::SessionDescription> desc =
session_desc_factory_.CreateOffer(
request.options,
sdp_handler_->local_description()
? sdp_handler_->local_description()->description()
: nullptr);
request.options, sdp_info_->local_description()
? sdp_info_->local_description()->description()
: nullptr);
if (!desc) {
PostCreateSessionDescriptionFailed(request.observer,
"Failed to initialize the offer.");
@ -364,11 +363,11 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
auto offer = std::make_unique<JsepSessionDescription>(
SdpType::kOffer, std::move(desc), session_id_,
rtc::ToString(session_version_++));
if (sdp_handler_->local_description()) {
if (sdp_info_->local_description()) {
for (const cricket::MediaDescriptionOptions& options :
request.options.media_description_options) {
if (!options.transport_options.ice_restart) {
CopyCandidatesFromSessionDescription(sdp_handler_->local_description(),
CopyCandidatesFromSessionDescription(sdp_info_->local_description(),
options.mid, offer.get());
}
}
@ -378,18 +377,18 @@ void WebRtcSessionDescriptionFactory::InternalCreateOffer(
void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
CreateSessionDescriptionRequest request) {
if (sdp_handler_->remote_description()) {
if (sdp_info_->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 =
sdp_handler_->IceRestartPending(options.mid);
sdp_info_->IceRestartPending(options.mid);
// We should pass the current DTLS role to the transport description
// factory, if there is already an existing ongoing session.
absl::optional<rtc::SSLRole> dtls_role =
sdp_handler_->transport_controller()->GetDtlsRole(options.mid);
sdp_info_->GetDtlsRole(options.mid);
if (dtls_role) {
options.transport_options.prefer_passive_role =
(rtc::SSL_SERVER == *dtls_role);
@ -399,12 +398,12 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
std::unique_ptr<cricket::SessionDescription> desc =
session_desc_factory_.CreateAnswer(
sdp_handler_->remote_description()
? sdp_handler_->remote_description()->description()
sdp_info_->remote_description()
? sdp_info_->remote_description()->description()
: nullptr,
request.options,
sdp_handler_->local_description()
? sdp_handler_->local_description()->description()
sdp_info_->local_description()
? sdp_info_->local_description()->description()
: nullptr);
if (!desc) {
PostCreateSessionDescriptionFailed(request.observer,
@ -423,13 +422,13 @@ void WebRtcSessionDescriptionFactory::InternalCreateAnswer(
auto answer = std::make_unique<JsepSessionDescription>(
SdpType::kAnswer, std::move(desc), session_id_,
rtc::ToString(session_version_++));
if (sdp_handler_->local_description()) {
if (sdp_info_->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(sdp_handler_->local_description(),
CopyCandidatesFromSessionDescription(sdp_info_->local_description(),
options.mid, answer.get());
}
}

View File

@ -23,7 +23,7 @@
#include "p2p/base/transport_description_factory.h"
#include "pc/channel_manager.h"
#include "pc/media_session.h"
#include "pc/sdp_offer_answer.h"
#include "pc/sdp_state_provider.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/message_handler.h"
#include "rtc_base/rtc_certificate.h"
@ -35,11 +35,6 @@
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,
@ -86,7 +81,7 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
WebRtcSessionDescriptionFactory(
rtc::Thread* signaling_thread,
cricket::ChannelManager* channel_manager,
SdpOfferAnswerHandler* sdp_handler,
const SdpStateProvider* sdp_info,
const std::string& session_id,
bool dtls_enabled,
std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator,
@ -158,7 +153,7 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
cricket::MediaSessionDescriptionFactory session_desc_factory_;
uint64_t session_version_;
const std::unique_ptr<rtc::RTCCertificateGeneratorInterface> cert_generator_;
SdpOfferAnswerHandler* sdp_handler_;
const SdpStateProvider* sdp_info_;
const std::string session_id_;
CertificateRequestState certificate_request_state_;