zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revert "Remove thread hops from events provided by JsepTransportController."

Browse Source 
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}
This commit is contained in:
Guido Urdaneta
2021-02-09 12:26:13 +00:00
committed by Commit Bot
parent 82ce7e5515
commit 6e4fcac313
8 changed files with 269 additions and 263 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
pc/jsep_transport_controller_unittest.cc
View File

@ -74,6 +74,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
void CreateJsepTransportController(
JsepTransportController::Config config,
rtc::Thread* signaling_thread = rtc::Thread::Current(),
rtc::Thread* network_thread = rtc::Thread::Current(),
cricket::PortAllocator* port_allocator = nullptr) {
config.transport_observer = this;
@ -83,10 +84,9 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
config.dtls_transport_factory = fake_dtls_transport_factory_.get();
config.on_dtls_handshake_error_ = [](rtc::SSLHandshakeError s) {};
transport_controller_ = std::make_unique<JsepTransportController>(
network_thread, port_allocator, nullptr /* async_resolver_factory */,
config);
network_thread->Invoke<void>(RTC_FROM_HERE,
[&] { ConnectTransportControllerSignals(); });
signaling_thread, network_thread, port_allocator,
nullptr /* async_resolver_factory */, config);
ConnectTransportControllerSignals();
}
void ConnectTransportControllerSignals() {
@ -276,14 +276,18 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
protected:
void OnConnectionState(cricket::IceConnectionState state) {
ice_signaled_on_thread_ = rtc::Thread::Current();
if (!signaling_thread_->IsCurrent()) {
signaled_on_non_signaling_thread_ = true;
}
connection_state_ = state;
++connection_state_signal_count_;
}
void OnStandardizedIceConnectionState(
PeerConnectionInterface::IceConnectionState state) {
ice_signaled_on_thread_ = rtc::Thread::Current();
if (!signaling_thread_->IsCurrent()) {
signaled_on_non_signaling_thread_ = true;
}
ice_connection_state_ = state;
++ice_connection_state_signal_count_;
}
@ -292,20 +296,26 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
PeerConnectionInterface::PeerConnectionState state) {
RTC_LOG(LS_INFO) << "OnCombinedConnectionState: "
<< static_cast<int>(state);
ice_signaled_on_thread_ = rtc::Thread::Current();
if (!signaling_thread_->IsCurrent()) {
signaled_on_non_signaling_thread_ = true;
}
combined_connection_state_ = state;
++combined_connection_state_signal_count_;
}
void OnGatheringState(cricket::IceGatheringState state) {
ice_signaled_on_thread_ = rtc::Thread::Current();
if (!signaling_thread_->IsCurrent()) {
signaled_on_non_signaling_thread_ = true;
}
gathering_state_ = state;
++gathering_state_signal_count_;
}
void OnCandidatesGathered(const std::string& transport_name,
const Candidates& candidates) {
ice_signaled_on_thread_ = rtc::Thread::Current();
if (!signaling_thread_->IsCurrent()) {
signaled_on_non_signaling_thread_ = true;
}
candidates_[transport_name].insert(candidates_[transport_name].end(),
candidates.begin(), candidates.end());
++candidates_signal_count_;
@ -350,7 +360,7 @@ class JsepTransportControllerTest : public JsepTransportController::Observer,
std::unique_ptr<FakeIceTransportFactory> fake_ice_transport_factory_;
std::unique_ptr<FakeDtlsTransportFactory> fake_dtls_transport_factory_;
rtc::Thread* const signaling_thread_ = nullptr;
rtc::Thread* ice_signaled_on_thread_ = nullptr;
bool signaled_on_non_signaling_thread_ = false;
// Used to verify the SignalRtpTransportChanged/SignalDtlsTransportChanged are
// signaled correctly.
std::map<std::string, RtpTransportInternal*> changed_rtp_transport_by_mid_;
@ -873,12 +883,11 @@ TEST_F(JsepTransportControllerTest, SignalCandidatesGathered) {
EXPECT_EQ(1u, candidates_[kAudioMid1].size());
}
TEST_F(JsepTransportControllerTest, IceSignalingOccursOnNetworkThread) {
TEST_F(JsepTransportControllerTest, IceSignalingOccursOnSignalingThread) {
network_thread_ = rtc::Thread::CreateWithSocketServer();
network_thread_->Start();
EXPECT_EQ(ice_signaled_on_thread_, nullptr);
CreateJsepTransportController(JsepTransportController::Config(),
network_thread_.get(),
signaling_thread_, network_thread_.get(),
/*port_allocator=*/nullptr);
CreateLocalDescriptionAndCompleteConnectionOnNetworkThread();
@ -894,7 +903,7 @@ TEST_F(JsepTransportControllerTest, IceSignalingOccursOnNetworkThread) {
EXPECT_EQ_WAIT(1u, candidates_[kVideoMid1].size(), kTimeout);
EXPECT_EQ(2, candidates_signal_count_);
EXPECT_EQ(ice_signaled_on_thread_, network_thread_.get());
EXPECT_TRUE(!signaled_on_non_signaling_thread_);
network_thread_->Invoke<void>(RTC_FROM_HERE,
[&] { transport_controller_.reset(); });