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

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

Browse Source 
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}
This commit is contained in:
Tommi
2021-02-10 17:40:08 +00:00
committed by Commit Bot
parent f28d7e89b7
commit c3257d0c77
8 changed files with 263 additions and 269 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
pc/jsep_transport_controller_unittest.cc
View File

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