From 77baeee99e984ae293d8e3a9722ff7263ecc50c0 Mon Sep 17 00:00:00 2001 From: Tomas Gunnarsson Date: Thu, 24 Sep 2020 22:39:21 +0200 Subject: [PATCH] Make MessageHandler be a pure virtual interface. Bug: webrtc:11908 Change-Id: I35d3c4005d970082bff8c5ff24186aab54205c37 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/185340 Reviewed-by: Karl Wiberg Commit-Queue: Tommi Cr-Commit-Position: refs/heads/master@{#32194} --- audio/null_audio_poller.cc | 3 +-- .../client/peer_connection_client.cc | 6 +---- p2p/base/stun_request.cc | 10 ++------ pc/peer_connection.cc | 3 +-- pc/remote_audio_source.cc | 3 +-- pc/webrtc_session_description_factory.cc | 3 +-- rtc_base/message_handler.cc | 9 +------ rtc_base/message_handler.h | 25 ++++++------------- rtc_base/thread.cc | 4 +-- rtc_base/thread.h | 2 +- .../src/jni/pc/peer_connection_factory.cc | 4 +-- sdk/objc/native/src/audio/audio_device_ios.mm | 3 +-- 12 files changed, 20 insertions(+), 55 deletions(-) diff --git a/audio/null_audio_poller.cc b/audio/null_audio_poller.cc index 16d267fb46..22f575d8bb 100644 --- a/audio/null_audio_poller.cc +++ b/audio/null_audio_poller.cc @@ -31,8 +31,7 @@ constexpr size_t kNumSamples = kSamplesPerSecond / 100; // 10ms of samples } // namespace NullAudioPoller::NullAudioPoller(AudioTransport* audio_transport) - : MessageHandler(false), - audio_transport_(audio_transport), + : audio_transport_(audio_transport), reschedule_at_(rtc::TimeMillis() + kPollDelayMs) { RTC_DCHECK(audio_transport); OnMessage(nullptr); // Start the poll loop. diff --git a/examples/peerconnection/client/peer_connection_client.cc b/examples/peerconnection/client/peer_connection_client.cc index a463ceed46..b458927095 100644 --- a/examples/peerconnection/client/peer_connection_client.cc +++ b/examples/peerconnection/client/peer_connection_client.cc @@ -43,11 +43,7 @@ rtc::AsyncSocket* CreateClientSocket(int family) { } // namespace PeerConnectionClient::PeerConnectionClient() - : MessageHandler(false), - callback_(NULL), - resolver_(NULL), - state_(NOT_CONNECTED), - my_id_(-1) {} + : callback_(NULL), resolver_(NULL), state_(NOT_CONNECTED), my_id_(-1) {} PeerConnectionClient::~PeerConnectionClient() { rtc::Thread::Current()->Clear(this); diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc index d6210fc2dc..44376ced95 100644 --- a/p2p/base/stun_request.cc +++ b/p2p/base/stun_request.cc @@ -174,8 +174,7 @@ bool StunRequestManager::CheckResponse(const char* data, size_t size) { } StunRequest::StunRequest() - : rtc::MessageHandler(false), - count_(0), + : count_(0), timeout_(false), manager_(0), msg_(new StunMessage()), @@ -184,12 +183,7 @@ StunRequest::StunRequest() } StunRequest::StunRequest(StunMessage* request) - : rtc::MessageHandler(false), - count_(0), - timeout_(false), - manager_(0), - msg_(request), - tstamp_(0) { + : count_(0), timeout_(false), manager_(0), msg_(request), tstamp_(0) { msg_->SetTransactionID(rtc::CreateRandomString(kStunTransactionIdLength)); } diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 3b5c333a98..c872550d43 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -1026,8 +1026,7 @@ void ExtractSharedMediaSessionOptions( PeerConnection::PeerConnection(PeerConnectionFactory* factory, std::unique_ptr event_log, std::unique_ptr call) - : MessageHandler(false), - factory_(factory), + : factory_(factory), event_log_(std::move(event_log)), event_log_ptr_(event_log_.get()), operations_chain_(rtc::OperationsChain::Create()), diff --git a/pc/remote_audio_source.cc b/pc/remote_audio_source.cc index 3cdceda906..18a4ed25c8 100644 --- a/pc/remote_audio_source.cc +++ b/pc/remote_audio_source.cc @@ -50,8 +50,7 @@ class RemoteAudioSource::AudioDataProxy : public AudioSinkInterface { }; RemoteAudioSource::RemoteAudioSource(rtc::Thread* worker_thread) - : MessageHandler(false), - main_thread_(rtc::Thread::Current()), + : main_thread_(rtc::Thread::Current()), worker_thread_(worker_thread), state_(MediaSourceInterface::kLive) { RTC_DCHECK(main_thread_); diff --git a/pc/webrtc_session_description_factory.cc b/pc/webrtc_session_description_factory.cc index d95174ec44..aaef7fdeb6 100644 --- a/pc/webrtc_session_description_factory.cc +++ b/pc/webrtc_session_description_factory.cc @@ -130,8 +130,7 @@ WebRtcSessionDescriptionFactory::WebRtcSessionDescriptionFactory( std::unique_ptr cert_generator, const rtc::scoped_refptr& certificate, UniqueRandomIdGenerator* ssrc_generator) - : MessageHandler(false), - signaling_thread_(signaling_thread), + : signaling_thread_(signaling_thread), session_desc_factory_(channel_manager, &transport_desc_factory_, ssrc_generator), diff --git a/rtc_base/message_handler.cc b/rtc_base/message_handler.cc index bccbc25bae..e6e973dbd9 100644 --- a/rtc_base/message_handler.cc +++ b/rtc_base/message_handler.cc @@ -14,14 +14,7 @@ namespace rtc { -MessageHandler::MessageHandler(bool auto_cleanup) { - RTC_DCHECK(!auto_cleanup) << "Use MessageHandlerAutoCleanup"; -} - -MessageHandler::~MessageHandler() {} - -MessageHandlerAutoCleanup::MessageHandlerAutoCleanup() - : MessageHandler(false) {} +MessageHandlerAutoCleanup::MessageHandlerAutoCleanup() {} MessageHandlerAutoCleanup::~MessageHandlerAutoCleanup() { // Note that even though this clears currently pending messages for the diff --git a/rtc_base/message_handler.h b/rtc_base/message_handler.h index 9568bc847b..62c8344e1f 100644 --- a/rtc_base/message_handler.h +++ b/rtc_base/message_handler.h @@ -21,29 +21,18 @@ namespace rtc { struct Message; -// MessageQueue/Thread Messages get dispatched to a MessageHandler via the -// |OnMessage()| callback method. -// -// Note: Besides being an interface, the class can perform automatic cleanup -// in the destructor. -// TODO(bugs.webrtc.org/11908): The |auto_cleanup| parameter and associated -// logic is a temporary step while changing the MessageHandler class to be -// a pure virtual interface. The automatic cleanup step involves a number of -// complex operations and as part of this interface, can easily go by unnoticed -// and bundled into situations where it's not needed. +// MessageQueue/Thread Messages get dispatched via the MessageHandler interface. class RTC_EXPORT MessageHandler { public: - virtual ~MessageHandler(); + virtual ~MessageHandler() {} virtual void OnMessage(Message* msg) = 0; - - protected: - // TODO(bugs.webrtc.org/11908): Remove this ctor. - explicit MessageHandler(bool auto_cleanup); - - private: - RTC_DISALLOW_COPY_AND_ASSIGN(MessageHandler); }; +// Warning: Provided for backwards compatibility. +// +// This class performs expensive cleanup in the dtor that will affect all +// instances of Thread (and their pending message queues) and will block the +// current thread as well as all other threads. class RTC_EXPORT MessageHandlerAutoCleanup : public MessageHandler { public: ~MessageHandlerAutoCleanup() override; diff --git a/rtc_base/thread.cc b/rtc_base/thread.cc index fd104009d8..32449020c5 100644 --- a/rtc_base/thread.cc +++ b/rtc_base/thread.cc @@ -74,7 +74,7 @@ const int kSlowDispatchLoggingThreshold = 50; // 50 ms class MessageHandlerWithTask final : public MessageHandler { public: - MessageHandlerWithTask() : MessageHandler(false) {} + MessageHandlerWithTask() {} void OnMessage(Message* msg) override { static_cast(msg->pdata)->Run(); @@ -961,7 +961,7 @@ void Thread::InvokeInternal(const Location& posted_from, class FunctorMessageHandler : public MessageHandler { public: explicit FunctorMessageHandler(rtc::FunctionView functor) - : MessageHandler(false), functor_(functor) {} + : functor_(functor) {} void OnMessage(Message* msg) override { functor_(); } private: diff --git a/rtc_base/thread.h b/rtc_base/thread.h index cd31da4979..24ea356ae6 100644 --- a/rtc_base/thread.h +++ b/rtc_base/thread.h @@ -534,7 +534,7 @@ class RTC_LOCKABLE RTC_EXPORT Thread : public webrtc::TaskQueueBase { private: class QueuedTaskHandler final : public MessageHandler { public: - QueuedTaskHandler() : MessageHandler(false) {} + QueuedTaskHandler() {} void OnMessage(Message* msg) override; }; diff --git a/sdk/android/src/jni/pc/peer_connection_factory.cc b/sdk/android/src/jni/pc/peer_connection_factory.cc index 48791d932a..2392db2403 100644 --- a/sdk/android/src/jni/pc/peer_connection_factory.cc +++ b/sdk/android/src/jni/pc/peer_connection_factory.cc @@ -82,9 +82,7 @@ void PostJavaCallback(JNIEnv* env, JavaAsyncCallback(JNIEnv* env, const JavaRef& j_object, JavaMethodPointer java_method_pointer) - : rtc::MessageHandler(false), - j_object_(env, j_object), - java_method_pointer_(java_method_pointer) {} + : j_object_(env, j_object), java_method_pointer_(java_method_pointer) {} void OnMessage(rtc::Message*) override { java_method_pointer_(AttachCurrentThreadIfNeeded(), j_object_); diff --git a/sdk/objc/native/src/audio/audio_device_ios.mm b/sdk/objc/native/src/audio/audio_device_ios.mm index f130dc2d95..3d953c0331 100644 --- a/sdk/objc/native/src/audio/audio_device_ios.mm +++ b/sdk/objc/native/src/audio/audio_device_ios.mm @@ -101,8 +101,7 @@ static void LogDeviceInfo() { #endif // !defined(NDEBUG) AudioDeviceIOS::AudioDeviceIOS() - : MessageHandler(false), - audio_device_buffer_(nullptr), + : audio_device_buffer_(nullptr), audio_unit_(nullptr), recording_(0), playing_(0),