From e99c68dd2124aaf95e921629101964eee544cbc9 Mon Sep 17 00:00:00 2001 From: Lahiru Ginnaliya Gamathige Date: Wed, 30 Sep 2020 14:33:45 -0700 Subject: [PATCH] Replace one use of sigslot with RoboCaller The eventual goal is to replace sigslot entirely, but we need to start small, tread carefully, and evaluate how it works out. Also add a few more RoboCaller unit tests to cover the types we now use with RoboCaller. Change-Id: I9a5814d1668a37546ea484ca88ec9c2be1913d25 Bug: webrtc:11943 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/184660 Commit-Queue: Lahiru Ginnaliya Gamathige Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#32266} --- pc/BUILD.gn | 1 + pc/jsep_transport_controller.cc | 9 ++--- pc/jsep_transport_controller.h | 4 ++- pc/jsep_transport_controller_unittest.cc | 6 ++-- pc/peer_connection.cc | 9 +++-- rtc_base/robo_caller_unittest.cc | 42 ++++++++++++++++++++++++ 6 files changed, 61 insertions(+), 10 deletions(-) diff --git a/pc/BUILD.gn b/pc/BUILD.gn index 5cb362ce76..f59c1d7a88 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -112,6 +112,7 @@ rtc_library("rtc_pc_base") { "../rtc_base", "../rtc_base:checks", "../rtc_base:deprecation", + "../rtc_base:robo_caller", "../rtc_base:rtc_task_queue", "../rtc_base:stringutils", "../rtc_base/synchronization:mutex", diff --git a/pc/jsep_transport_controller.cc b/pc/jsep_transport_controller.cc index 98ad0000d0..4999f2ab04 100644 --- a/pc/jsep_transport_controller.cc +++ b/pc/jsep_transport_controller.cc @@ -1256,10 +1256,11 @@ void JsepTransportController::UpdateAggregateStates_n() { } if (ice_connection_state_ != new_connection_state) { ice_connection_state_ = new_connection_state; - invoker_.AsyncInvoke(RTC_FROM_HERE, signaling_thread_, - [this, new_connection_state] { - SignalIceConnectionState(new_connection_state); - }); + + invoker_.AsyncInvoke( + RTC_FROM_HERE, signaling_thread_, [this, new_connection_state] { + SignalIceConnectionState.Send(new_connection_state); + }); } // Compute the current RTCIceConnectionState as described in diff --git a/pc/jsep_transport_controller.h b/pc/jsep_transport_controller.h index 190f740c7c..f3a92947a5 100644 --- a/pc/jsep_transport_controller.h +++ b/pc/jsep_transport_controller.h @@ -35,6 +35,7 @@ #include "rtc_base/async_invoker.h" #include "rtc_base/constructor_magic.h" #include "rtc_base/ref_counted_object.h" +#include "rtc_base/robo_caller.h" #include "rtc_base/third_party/sigslot/sigslot.h" namespace rtc { @@ -197,10 +198,11 @@ class JsepTransportController : public sigslot::has_slots<> { // Else if all completed => completed, // Else if all connected => connected, // Else => connecting - sigslot::signal1 SignalIceConnectionState; + RoboCaller SignalIceConnectionState; sigslot::signal1 SignalConnectionState; + sigslot::signal1 SignalStandardizedIceConnectionState; diff --git a/pc/jsep_transport_controller_unittest.cc b/pc/jsep_transport_controller_unittest.cc index 7bdba23c2d..40dc23e535 100644 --- a/pc/jsep_transport_controller_unittest.cc +++ b/pc/jsep_transport_controller_unittest.cc @@ -89,8 +89,10 @@ class JsepTransportControllerTest : public JsepTransportController::Observer, } void ConnectTransportControllerSignals() { - transport_controller_->SignalIceConnectionState.connect( - this, &JsepTransportControllerTest::OnConnectionState); + transport_controller_->SignalIceConnectionState.AddReceiver( + [this](cricket::IceConnectionState s) { + JsepTransportControllerTest::OnConnectionState(s); + }); transport_controller_->SignalStandardizedIceConnectionState.connect( this, &JsepTransportControllerTest::OnStandardizedIceConnectionState); transport_controller_->SignalConnectionState.connect( diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc index 64898e207e..2b258cd5a9 100644 --- a/pc/peer_connection.cc +++ b/pc/peer_connection.cc @@ -685,8 +685,6 @@ bool PeerConnection::Initialize( transport_controller_.reset(new JsepTransportController( signaling_thread(), network_thread(), port_allocator_.get(), async_resolver_factory_.get(), config)); - transport_controller_->SignalIceConnectionState.connect( - this, &PeerConnection::OnTransportControllerConnectionState); transport_controller_->SignalStandardizedIceConnectionState.connect( this, &PeerConnection::SetStandardizedIceConnectionState); transport_controller_->SignalConnectionState.connect( @@ -704,6 +702,12 @@ bool PeerConnection::Initialize( transport_controller_->SignalIceCandidatePairChanged.connect( this, &PeerConnection::OnTransportControllerCandidateChanged); + transport_controller_->SignalIceConnectionState.AddReceiver( + [this](cricket::IceConnectionState s) { + RTC_DCHECK_RUN_ON(signaling_thread()); + OnTransportControllerConnectionState(s); + }); + stats_.reset(new StatsCollector(this)); stats_collector_ = RTCStatsCollector::Create(this); @@ -3625,7 +3629,6 @@ PeerConnection::InitializePortAllocator_n( "Disabled")) { port_allocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6); } - if (configuration.disable_ipv6_on_wifi) { port_allocator_flags &= ~(cricket::PORTALLOCATOR_ENABLE_IPV6_ON_WIFI); RTC_LOG(LS_INFO) << "IPv6 candidates on Wi-Fi are disabled."; diff --git a/rtc_base/robo_caller_unittest.cc b/rtc_base/robo_caller_unittest.cc index 99719eaaf3..9330bb68a5 100644 --- a/rtc_base/robo_caller_unittest.cc +++ b/rtc_base/robo_caller_unittest.cc @@ -50,6 +50,32 @@ TEST(RoboCaller, ReferenceTest) { EXPECT_EQ(index, 2); } +enum State { + kNew, + kChecking, +}; + +TEST(RoboCaller, SingleEnumValueTest) { + RoboCaller c; + State s1 = kNew; + int index = 0; + + c.AddReceiver([&index](State s) { index++; }); + c.Send(s1); + + EXPECT_EQ(index, 1); +} + +TEST(RoboCaller, SingleEnumReferenceTest) { + RoboCaller c; + State s = kNew; + + c.AddReceiver([](State& s) { s = kChecking; }); + c.Send(s); + + EXPECT_EQ(s, kChecking); +} + TEST(RoboCaller, ConstReferenceTest) { RoboCaller c; int i = 0; @@ -166,6 +192,22 @@ TEST(RoboCaller, MultipleReceiverSendTest) { EXPECT_EQ(index, 5); } + +class A { + public: + void increment(int& i) const { i++; } +}; + +TEST(RoboCaller, MemberFunctionTest) { + RoboCaller c; + A a; + int index = 1; + + c.AddReceiver([&a](int& i) { a.increment(i); }); + c.Send(index); + + EXPECT_EQ(index, 2); +} // todo(glahiru): Add a test case to catch some error for Karl's first fix // todo(glahiru): Add a test for rtc::Bind // which used the following code in the Send