Remove a sigslot from webrtc_session_description_factory

callback are know at construction time and only need some synchronization at destruction time. In this case such synchronization can be done with cheaper/simpler WeakPtr concept.

Asynchronous call to SetCertificate is no longer needed thanks to
previous removal of sigslot in
https://webrtc-review.googlesource.com/c/src/+/192362

Bug: webrtc:11943
Change-Id: Icadbcb4f83be9ed4b8f53a72beaef8573f2c9356
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/272402
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37868}
This commit is contained in:
Danil Chapovalov 2022-08-19 14:03:54 +02:00 committed by WebRTC LUCI CQ
parent 5cb3a90870
commit b22f0c2238
3 changed files with 49 additions and 74 deletions

View file

@ -1396,7 +1396,7 @@ rtc_source_set("webrtc_session_description_factory") {
"../rtc_base:rtc_base",
"../rtc_base:stringutils",
"../rtc_base:threading",
"../rtc_base/third_party/sigslot:sigslot",
"../rtc_base:weak_ptr",
]
absl_deps = [
"//third_party/abseil-cpp/absl/algorithm:container",

View file

@ -71,7 +71,6 @@ static bool ValidMediaSessionOptions(
enum {
MSG_CREATE_SESSIONDESCRIPTION_SUCCESS,
MSG_CREATE_SESSIONDESCRIPTION_FAILED,
MSG_USE_CONSTRUCTOR_CERTIFICATE
};
struct CreateSessionDescriptionMsg : public rtc::MessageData {
@ -86,14 +85,28 @@ struct CreateSessionDescriptionMsg : public rtc::MessageData {
};
} // namespace
void WebRtcCertificateGeneratorCallback::OnFailure() {
SignalRequestFailed();
}
class WebRtcSessionDescriptionFactory::WebRtcCertificateGeneratorCallback
: public rtc::RTCCertificateGeneratorCallback {
public:
explicit WebRtcCertificateGeneratorCallback(
rtc::WeakPtr<WebRtcSessionDescriptionFactory> ptr)
: weak_ptr_(std::move(ptr)) {}
// `rtc::RTCCertificateGeneratorCallback` overrides.
void OnSuccess(
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) override {
if (weak_ptr_) {
weak_ptr_->SetCertificate(certificate);
}
}
void OnFailure() override {
if (weak_ptr_) {
weak_ptr_->OnCertificateRequestFailed();
}
}
void WebRtcCertificateGeneratorCallback::OnSuccess(
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) {
SignalCertificateReady(certificate);
}
private:
rtc::WeakPtr<WebRtcSessionDescriptionFactory> weak_ptr_;
};
// static
void WebRtcSessionDescriptionFactory::CopyCandidatesFromSessionDescription(
@ -167,21 +180,14 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory(
certificate_request_state_ = CERTIFICATE_WAITING;
RTC_LOG(LS_VERBOSE) << "DTLS-SRTP enabled; has certificate parameter.";
// We already have a certificate but we wait to do `SetIdentity`; if we do
// it in the constructor then the caller has not had a chance to connect to
// `SignalCertificateReady`.
signaling_thread_->Post(
RTC_FROM_HERE, this, MSG_USE_CONSTRUCTOR_CERTIFICATE,
new rtc::ScopedRefMessageData<rtc::RTCCertificate>(certificate.get()));
RTC_LOG(LS_INFO) << "Using certificate supplied to the constructor.";
SetCertificate(certificate);
} else {
// Generate certificate.
certificate_request_state_ = CERTIFICATE_WAITING;
auto callback = rtc::make_ref_counted<WebRtcCertificateGeneratorCallback>();
callback->SignalRequestFailed.connect(
this, &WebRtcSessionDescriptionFactory::OnCertificateRequestFailed);
callback->SignalCertificateReady.connect(
this, &WebRtcSessionDescriptionFactory::SetCertificate);
auto callback = rtc::make_ref_counted<WebRtcCertificateGeneratorCallback>(
weak_factory_.GetWeakPtr());
rtc::KeyParams key_params = rtc::KeyParams();
RTC_LOG(LS_VERBOSE)
@ -206,17 +212,7 @@ WebRtcSessionDescriptionFactory::~WebRtcSessionDescriptionFactory() {
rtc::MessageList list;
signaling_thread_->Clear(this, rtc::MQID_ANY, &list);
for (auto& msg : list) {
if (msg.message_id != MSG_USE_CONSTRUCTOR_CERTIFICATE) {
OnMessage(&msg);
} else {
// Skip MSG_USE_CONSTRUCTOR_CERTIFICATE because we don't want to trigger
// SetIdentity-related callbacks in the destructor. This can be a problem
// when WebRtcSession listens to the callback but it was the WebRtcSession
// destructor that caused WebRtcSessionDescriptionFactory's destruction.
// The callback is then ignored, leaking memory allocated by OnMessage for
// MSG_USE_CONSTRUCTOR_CERTIFICATE.
delete msg.pdata;
}
OnMessage(&msg);
}
}
@ -317,15 +313,6 @@ void WebRtcSessionDescriptionFactory::OnMessage(rtc::Message* msg) {
delete param;
break;
}
case MSG_USE_CONSTRUCTOR_CERTIFICATE: {
rtc::ScopedRefMessageData<rtc::RTCCertificate>* param =
static_cast<rtc::ScopedRefMessageData<rtc::RTCCertificate>*>(
msg->pdata);
RTC_LOG(LS_INFO) << "Using certificate supplied to the constructor.";
SetCertificate(param->data());
delete param;
break;
}
default:
RTC_DCHECK_NOTREACHED();
break;

View file

@ -28,50 +28,18 @@
#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"
#include "rtc_base/weak_ptr.h"
namespace webrtc {
// DTLS certificate request callback class.
class WebRtcCertificateGeneratorCallback
: public rtc::RTCCertificateGeneratorCallback {
public:
// `rtc::RTCCertificateGeneratorCallback` overrides.
void OnSuccess(
const rtc::scoped_refptr<rtc::RTCCertificate>& certificate) override;
void OnFailure() override;
sigslot::signal0<> SignalRequestFailed;
sigslot::signal1<const rtc::scoped_refptr<rtc::RTCCertificate>&>
SignalCertificateReady;
};
struct CreateSessionDescriptionRequest {
enum Type {
kOffer,
kAnswer,
};
CreateSessionDescriptionRequest(Type type,
CreateSessionDescriptionObserver* observer,
const cricket::MediaSessionOptions& options)
: type(type), observer(observer), options(options) {}
Type type;
rtc::scoped_refptr<CreateSessionDescriptionObserver> observer;
cricket::MediaSessionOptions options;
};
// This class is used to create offer/answer session description. Certificates
// for WebRtcSession/DTLS are either supplied at construction or generated
// asynchronously. It queues the create offer/answer request until the
// certificate generation has completed, i.e. when OnCertificateRequestFailed or
// OnCertificateReady is called.
class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
public sigslot::has_slots<> {
class WebRtcSessionDescriptionFactory : public rtc::MessageHandler {
public:
// Can specify either a `cert_generator` or `certificate` to enable DTLS. If
// a certificate generator is given, starts generating the certificate
@ -130,6 +98,25 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
CERTIFICATE_FAILED,
};
// DTLS certificate request callback class.
class WebRtcCertificateGeneratorCallback;
struct CreateSessionDescriptionRequest {
enum Type {
kOffer,
kAnswer,
};
CreateSessionDescriptionRequest(Type type,
CreateSessionDescriptionObserver* observer,
const cricket::MediaSessionOptions& options)
: type(type), observer(observer), options(options) {}
Type type;
rtc::scoped_refptr<CreateSessionDescriptionObserver> observer;
cricket::MediaSessionOptions options;
};
// MessageHandler implementation.
virtual void OnMessage(rtc::Message* msg);
@ -161,6 +148,7 @@ class WebRtcSessionDescriptionFactory : public rtc::MessageHandler,
std::function<void(const rtc::scoped_refptr<rtc::RTCCertificate>&)>
on_certificate_ready_;
rtc::WeakPtrFactory<WebRtcSessionDescriptionFactory> weak_factory_{this};
};
} // namespace webrtc