From bb19276a325a5f9fce4afa245aa14ec2a4b1a41d Mon Sep 17 00:00:00 2001 From: Steve Anton Date: Fri, 31 Aug 2018 11:57:02 -0700 Subject: [PATCH] Use AsyncInvoker in PeerConnection instead of MessageHandler Bug: webrtc:9702 Change-Id: I89d66d1165a096601aed37b8febad60620073899 Reviewed-on: https://webrtc-review.googlesource.com/97180 Reviewed-by: Seth Hampson Commit-Queue: Steve Anton Cr-Commit-Position: refs/heads/master@{#24515} --- pc/peerconnection.cc | 144 ++++++++++++------------------------------- pc/peerconnection.h | 7 +-- 2 files changed, 43 insertions(+), 108 deletions(-) diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 7bc25706f0..a75585cfdf 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -101,43 +101,8 @@ static const char kDefaultVideoSenderId[] = "defaultv0"; // The length of RTCP CNAMEs. static const int kRtcpCnameLength = 16; -enum { - MSG_SET_SESSIONDESCRIPTION_SUCCESS = 0, - MSG_SET_SESSIONDESCRIPTION_FAILED, - MSG_CREATE_SESSIONDESCRIPTION_FAILED, - MSG_GETSTATS, - MSG_FREE_DATACHANNELS, - MSG_REPORT_USAGE_PATTERN, -}; - static const int REPORT_USAGE_PATTERN_DELAY_MS = 60000; -struct SetSessionDescriptionMsg : public rtc::MessageData { - explicit SetSessionDescriptionMsg( - webrtc::SetSessionDescriptionObserver* observer) - : observer(observer) {} - - rtc::scoped_refptr observer; - RTCError error; -}; - -struct CreateSessionDescriptionMsg : public rtc::MessageData { - explicit CreateSessionDescriptionMsg( - webrtc::CreateSessionDescriptionObserver* observer) - : observer(observer) {} - - rtc::scoped_refptr observer; - RTCError error; -}; - -struct GetStatsMsg : public rtc::MessageData { - GetStatsMsg(webrtc::StatsObserver* observer, - webrtc::MediaStreamTrackInterface* track) - : observer(observer), track(track) {} - rtc::scoped_refptr observer; - rtc::scoped_refptr track; -}; - // Check if we can send |new_stream| on a PeerConnection. bool CanAddLocalMediaStream(webrtc::StreamCollectionInterface* current_streams, webrtc::MediaStreamInterface* new_stream) { @@ -1028,8 +993,9 @@ bool PeerConnection::Initialize( } int delay_ms = return_histogram_very_quickly_ ? 0 : REPORT_USAGE_PATTERN_DELAY_MS; - signaling_thread()->PostDelayed(RTC_FROM_HERE, delay_ms, this, - MSG_REPORT_USAGE_PATTERN, nullptr); + async_invoker_.AsyncInvokeDelayed(RTC_FROM_HERE, signaling_thread(), + [this] { ReportUsagePattern(); }, + delay_ms); return true; } @@ -1582,8 +1548,16 @@ bool PeerConnection::GetStats(StatsObserver* observer, << track->id(); return false; } - signaling_thread()->Post(RTC_FROM_HERE, this, MSG_GETSTATS, - new GetStatsMsg(observer, track)); + // Need to capture |observer| and |track| in scoped_refptrs to ensure they + // live long enough. + rtc::scoped_refptr observer_refptr(observer); + rtc::scoped_refptr track_refptr(track); + async_invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread(), + [this, observer_refptr, track_refptr] { + StatsReports reports; + stats_->GetStats(track_refptr, &reports); + observer_refptr->OnComplete(reports); + }); return true; } @@ -1912,9 +1886,9 @@ void PeerConnection::SetLocalDescription( PostSetSessionDescriptionSuccess(observer); - // MaybeStartGathering needs to be called after posting - // MSG_SET_SESSIONDESCRIPTION_SUCCESS, so that we don't signal any candidates - // before signaling that SetLocalDescription completed. + // MaybeStartGathering needs to be called after posting OnSuccess to the + // SetSessionDescriptionObserver so that we don't signal any candidates before + // signaling that SetLocalDescription completed. transport_controller_->MaybeStartGathering(); if (local_description()->GetType() == SdpType::kAnswer) { @@ -3237,51 +3211,6 @@ void PeerConnection::Close() { observer_ = nullptr; } -void PeerConnection::OnMessage(rtc::Message* msg) { - switch (msg->message_id) { - case MSG_SET_SESSIONDESCRIPTION_SUCCESS: { - SetSessionDescriptionMsg* param = - static_cast(msg->pdata); - param->observer->OnSuccess(); - delete param; - break; - } - case MSG_SET_SESSIONDESCRIPTION_FAILED: { - SetSessionDescriptionMsg* param = - static_cast(msg->pdata); - param->observer->OnFailure(std::move(param->error)); - delete param; - break; - } - case MSG_CREATE_SESSIONDESCRIPTION_FAILED: { - CreateSessionDescriptionMsg* param = - static_cast(msg->pdata); - param->observer->OnFailure(std::move(param->error)); - delete param; - break; - } - case MSG_GETSTATS: { - GetStatsMsg* param = static_cast(msg->pdata); - StatsReports reports; - stats_->GetStats(param->track, &reports); - param->observer->OnComplete(reports); - delete param; - break; - } - case MSG_FREE_DATACHANNELS: { - sctp_data_channels_to_free_.clear(); - break; - } - case MSG_REPORT_USAGE_PATTERN: { - ReportUsagePattern(); - break; - } - default: - RTC_NOTREACHED() << "Not implemented"; - break; - } -} - cricket::VoiceMediaChannel* PeerConnection::voice_media_channel() const { RTC_DCHECK(!IsUnifiedPlan()); auto* voice_channel = static_cast( @@ -3554,29 +3483,37 @@ void PeerConnection::OnVideoTrackRemoved(VideoTrackInterface* track, void PeerConnection::PostSetSessionDescriptionSuccess( SetSessionDescriptionObserver* observer) { - SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer); - signaling_thread()->Post(RTC_FROM_HERE, this, - MSG_SET_SESSIONDESCRIPTION_SUCCESS, msg); + async_invoker_.AsyncInvoke( + RTC_FROM_HERE, signaling_thread(), + rtc::Bind(&SetSessionDescriptionObserver::OnSuccess, observer)); } void PeerConnection::PostSetSessionDescriptionFailure( SetSessionDescriptionObserver* observer, - RTCError&& error) { + RTCError error) { RTC_DCHECK(!error.ok()); - SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer); - msg->error = std::move(error); - signaling_thread()->Post(RTC_FROM_HERE, this, - MSG_SET_SESSIONDESCRIPTION_FAILED, msg); + // TODO(steveanton): In C++14 this can be done with a lambda. + struct Functor { + void operator()() { observer->OnFailure(std::move(error)); } + rtc::scoped_refptr observer; + RTCError error; + }; + async_invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread(), + Functor{observer, std::move(error)}); } void PeerConnection::PostCreateSessionDescriptionFailure( CreateSessionDescriptionObserver* observer, RTCError error) { RTC_DCHECK(!error.ok()); - CreateSessionDescriptionMsg* msg = new CreateSessionDescriptionMsg(observer); - msg->error = std::move(error); - signaling_thread()->Post(RTC_FROM_HERE, this, - MSG_CREATE_SESSIONDESCRIPTION_FAILED, msg); + // TODO(steveanton): In C++14 this can be done with a lambda. + struct Functor { + void operator()() { observer->OnFailure(std::move(error)); } + rtc::scoped_refptr observer; + RTCError error; + }; + async_invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread(), + Functor{observer, std::move(error)}); } void PeerConnection::GetOptionsForOffer( @@ -4468,8 +4405,9 @@ void PeerConnection::OnSctpDataChannelClosed(DataChannel* channel) { // we can't free it directly here; we need to free it asynchronously. sctp_data_channels_to_free_.push_back(*it); sctp_data_channels_.erase(it); - signaling_thread()->Post(RTC_FROM_HERE, this, MSG_FREE_DATACHANNELS, - nullptr); + async_invoker_.AsyncInvoke( + RTC_FROM_HERE, signaling_thread(), + [this] { sctp_data_channels_to_free_.clear(); }); return; } } @@ -6233,8 +6171,8 @@ void PeerConnection::ClearStatsCache() { } void PeerConnection::RequestUsagePatternReportForTesting() { - signaling_thread()->Post(RTC_FROM_HERE, this, MSG_REPORT_USAGE_PATTERN, - nullptr); + async_invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread(), + [this] { ReportUsagePattern(); }); } } // namespace webrtc diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 2942709253..bd9ef24569 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -52,7 +52,6 @@ class RtcEventLog; class PeerConnection : public PeerConnectionInternal, public DataChannelProviderInterface, public JsepTransportController::Observer, - public rtc::MessageHandler, public sigslot::has_slots<> { public: enum class UsageEvent : int { @@ -288,9 +287,6 @@ class PeerConnection : public PeerConnectionInternal, uint32_t first_ssrc; }; - // Implements MessageHandler. - void OnMessage(rtc::Message* msg) override; - // Plan B helpers for getting the voice/video media channels for the single // audio/video transceiver, if it exists. cricket::VoiceMediaChannel* voice_media_channel() const; @@ -395,7 +391,7 @@ class PeerConnection : public PeerConnectionInternal, void PostSetSessionDescriptionSuccess( SetSessionDescriptionObserver* observer); void PostSetSessionDescriptionFailure(SetSessionDescriptionObserver* observer, - RTCError&& error); + RTCError error); void PostCreateSessionDescriptionFailure( CreateSessionDescriptionObserver* observer, RTCError error); @@ -1033,6 +1029,7 @@ class PeerConnection : public PeerConnectionInternal, int usage_event_accumulator_ = 0; bool return_histogram_very_quickly_ = false; + rtc::AsyncInvoker async_invoker_; }; } // namespace webrtc