Reland "Remove thread hops from events provided by JsepTransportController."
This reverts commit 6e4fcac31312f2dda5b60d33874ff0cd62f94321.
Reason for revert: Parent CL issue has been resolved.
Original change's description:
> Revert "Remove thread hops from events provided by JsepTransportController."
>
> This reverts commit f554b3c577f69fa9ffad5c07155898c2d985ac76.
>
> Reason for revert: Parent CL breaks FYI bots.
> See https://webrtc-review.googlesource.com/c/src/+/206466
>
> Original change's description:
> > Remove thread hops from events provided by JsepTransportController.
> >
> > Events associated with Subscribe* methods in JTC had trampolines that
> > would use an async invoker to fire the events on the signaling thread.
> > This was being done for the purposes of PeerConnection but the concept
> > of a signaling thread is otherwise not applicable to JTC and use of
> > JTC from PC is inconsistent across threads (as has been flagged in
> > webrtc:9987).
> >
> > This change makes all CallbackList members only accessible from the
> > network thread and moves the signaling thread related work over to
> > PeerConnection, which makes hops there more visible as well as making
> > that class easier to refactor for thread efficiency.
> >
> > This CL removes the AsyncInvoker from JTC (webrtc:12339)
> >
> > The signaling_thread_ variable is also removed from JTC and more thread
> > checks added to catch errors.
> >
> > Bug: webrtc:12427, webrtc:11988, webrtc:12339
> > Change-Id: Id232aedd00dfd5403b2ba0ca147d3eca7c12c7c5
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206062
> > Commit-Queue: Tommi <tommi@webrtc.org>
> > Reviewed-by: Niels Moller <nisse@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#33195}
>
> TBR=nisse@webrtc.org,tommi@webrtc.org
>
> Change-Id: I6134b71b74a9408854b79d44506d513519e9cf4d
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:12427
> Bug: webrtc:11988
> Bug: webrtc:12339
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206467
> Reviewed-by: Guido Urdaneta <guidou@webrtc.org>
> Commit-Queue: Guido Urdaneta <guidou@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#33203}
TBR=nisse@webrtc.org,tommi@webrtc.org,guidou@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:12427
Bug: webrtc:11988
Bug: webrtc:12339
Change-Id: I4e2e1490e1f9a87ed6ac4d722fd3c442e3059ae0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/206809
Reviewed-by: Tommi <tommi@webrtc.org>
Commit-Queue: Tommi <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33225}
diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h
index 59d66a2..949c9ad 100644
--- a/pc/jsep_transport_controller.h
+++ b/pc/jsep_transport_controller.h
@@ -54,7 +54,6 @@
#include "pc/session_description.h"
#include "pc/srtp_transport.h"
#include "pc/transport_stats.h"
-#include "rtc_base/async_invoker.h"
#include "rtc_base/callback_list.h"
#include "rtc_base/constructor_magic.h"
#include "rtc_base/copy_on_write_buffer.h"
@@ -137,10 +136,11 @@
std::function<void(const rtc::SSLHandshakeError)> on_dtls_handshake_error_;
};
- // The ICE related events are signaled on the |signaling_thread|.
- // All the transport related methods are called on the |network_thread|.
- JsepTransportController(rtc::Thread* signaling_thread,
- rtc::Thread* network_thread,
+ // The ICE related events are fired on the |network_thread|.
+ // All the transport related methods are called on the |network_thread|
+ // and destruction of the JsepTransportController must occur on the
+ // |network_thread|.
+ JsepTransportController(rtc::Thread* network_thread,
cricket::PortAllocator* port_allocator,
AsyncResolverFactory* async_resolver_factory,
Config config);
@@ -227,26 +227,28 @@
// F: void(const std::string&, const std::vector<cricket::Candidate>&)
template <typename F>
void SubscribeIceCandidateGathered(F&& callback) {
- // TODO(bugs.webrtc.org/12427): Post this subscription to the network
- // thread.
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_ice_candidates_gathered_.AddReceiver(std::forward<F>(callback));
}
// F: void(cricket::IceConnectionState)
template <typename F>
void SubscribeIceConnectionState(F&& callback) {
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_ice_connection_state_.AddReceiver(std::forward<F>(callback));
}
// F: void(PeerConnectionInterface::PeerConnectionState)
template <typename F>
void SubscribeConnectionState(F&& callback) {
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_connection_state_.AddReceiver(std::forward<F>(callback));
}
// F: void(PeerConnectionInterface::IceConnectionState)
template <typename F>
void SubscribeStandardizedIceConnectionState(F&& callback) {
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_standardized_ice_connection_state_.AddReceiver(
std::forward<F>(callback));
}
@@ -254,60 +256,65 @@
// F: void(cricket::IceGatheringState)
template <typename F>
void SubscribeIceGatheringState(F&& callback) {
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_ice_gathering_state_.AddReceiver(std::forward<F>(callback));
}
// F: void(const cricket::IceCandidateErrorEvent&)
template <typename F>
void SubscribeIceCandidateError(F&& callback) {
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_ice_candidate_error_.AddReceiver(std::forward<F>(callback));
}
// F: void(const std::vector<cricket::Candidate>&)
template <typename F>
void SubscribeIceCandidatesRemoved(F&& callback) {
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_ice_candidates_removed_.AddReceiver(std::forward<F>(callback));
}
// F: void(const cricket::CandidatePairChangeEvent&)
template <typename F>
void SubscribeIceCandidatePairChanged(F&& callback) {
+ RTC_DCHECK_RUN_ON(network_thread_);
signal_ice_candidate_pair_changed_.AddReceiver(std::forward<F>(callback));
}
private:
- // All of these callbacks are fired on the signaling thread.
+ // All of these callbacks are fired on the network thread.
// If any transport failed => failed,
// Else if all completed => completed,
// Else if all connected => connected,
// Else => connecting
- CallbackList<cricket::IceConnectionState> signal_ice_connection_state_;
+ CallbackList<cricket::IceConnectionState> signal_ice_connection_state_
+ RTC_GUARDED_BY(network_thread_);
CallbackList<PeerConnectionInterface::PeerConnectionState>
- signal_connection_state_;
+ signal_connection_state_ RTC_GUARDED_BY(network_thread_);
CallbackList<PeerConnectionInterface::IceConnectionState>
- signal_standardized_ice_connection_state_;
+ signal_standardized_ice_connection_state_ RTC_GUARDED_BY(network_thread_);
// If all transports done gathering => complete,
// Else if any are gathering => gathering,
// Else => new
- CallbackList<cricket::IceGatheringState> signal_ice_gathering_state_;
+ CallbackList<cricket::IceGatheringState> signal_ice_gathering_state_
+ RTC_GUARDED_BY(network_thread_);
// [mid, candidates]
- // TODO(bugs.webrtc.org/12427): Protect this with network_thread_.
CallbackList<const std::string&, const std::vector<cricket::Candidate>&>
- signal_ice_candidates_gathered_;
+ signal_ice_candidates_gathered_ RTC_GUARDED_BY(network_thread_);
CallbackList<const cricket::IceCandidateErrorEvent&>
- signal_ice_candidate_error_;
+ signal_ice_candidate_error_ RTC_GUARDED_BY(network_thread_);
CallbackList<const std::vector<cricket::Candidate>&>
- signal_ice_candidates_removed_;
+ signal_ice_candidates_removed_ RTC_GUARDED_BY(network_thread_);
CallbackList<const cricket::CandidatePairChangeEvent&>
- signal_ice_candidate_pair_changed_;
+ signal_ice_candidate_pair_changed_ RTC_GUARDED_BY(network_thread_);
RTCError ApplyDescription_n(bool local,
SdpType type,
@@ -452,7 +459,6 @@
void OnDtlsHandshakeError(rtc::SSLHandshakeError error);
- rtc::Thread* const signaling_thread_ = nullptr;
rtc::Thread* const network_thread_ = nullptr;
cricket::PortAllocator* const port_allocator_ = nullptr;
AsyncResolverFactory* const async_resolver_factory_ = nullptr;
@@ -490,7 +496,6 @@
cricket::IceRole ice_role_ = cricket::ICEROLE_CONTROLLING;
uint64_t ice_tiebreaker_ = rtc::CreateRandomId64();
rtc::scoped_refptr<rtc::RTCCertificate> certificate_;
- rtc::AsyncInvoker invoker_;
RTC_DISALLOW_COPY_AND_ASSIGN(JsepTransportController);
};
