diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc index 28576f41fc..9a860d25dc 100644 --- a/p2p/base/p2p_transport_channel_unittest.cc +++ b/p2p/base/p2p_transport_channel_unittest.cc @@ -62,6 +62,8 @@ using ::testing::ReturnRef; using ::testing::SaveArg; using ::testing::SetArgPointee; using ::testing::SizeIs; +using ::webrtc::PendingTaskSafetyFlag; +using ::webrtc::SafeTask; // Default timeout for tests in this file. // Should be large enough for slow buildbots to run the tests reliably. @@ -129,8 +131,6 @@ const cricket::IceParameters kIceParams[4] = { const uint64_t kLowTiebreaker = 11111; const uint64_t kHighTiebreaker = 22222; -enum { MSG_ADD_CANDIDATES, MSG_REMOVE_CANDIDATES }; - cricket::IceConfig CreateIceConfig( int receiving_timeout, cricket::ContinualGatheringPolicy continual_gathering_policy, @@ -261,7 +261,6 @@ namespace cricket { // Note that this class is a base class for use by other tests, who will provide // specialized test behavior. class P2PTransportChannelTestBase : public ::testing::Test, - public rtc::MessageHandlerAutoCleanup, public sigslot::has_slots<> { public: P2PTransportChannelTestBase() @@ -349,13 +348,9 @@ class P2PTransportChannelTestBase : public ::testing::Test, std::unique_ptr ch_; }; - struct CandidatesData : public rtc::MessageData { - CandidatesData(IceTransportInternal* ch, const Candidate& c) - : channel(ch), candidates(1, c) {} - CandidatesData(IceTransportInternal* ch, const std::vector& cc) - : channel(ch), candidates(cc) {} + struct CandidateData { IceTransportInternal* channel; - Candidates candidates; + Candidate candidate; }; struct Endpoint : public sigslot::has_slots<> { @@ -406,7 +401,7 @@ class P2PTransportChannelTestBase : public ::testing::Test, uint64_t tiebreaker_; bool role_conflict_; bool save_candidates_; - std::vector> saved_candidates_; + std::vector saved_candidates_; bool ready_to_send_ = false; std::map ice_regathering_counter_; }; @@ -487,7 +482,7 @@ class P2PTransportChannelTestBase : public ::testing::Test, } void DestroyChannels() { - main_.Clear(this); + safety_->SetNotAlive(); ep1_.cd1_.ch_.reset(); ep2_.cd1_.ch_.reset(); ep1_.cd2_.ch_.reset(); @@ -822,10 +817,10 @@ class P2PTransportChannelTestBase : public ::testing::Test, if (GetEndpoint(ch)->save_candidates_) { GetEndpoint(ch)->saved_candidates_.push_back( - std::unique_ptr(new CandidatesData(ch, c))); + {.channel = ch, .candidate = c}); } else { - main_.Post(RTC_FROM_HERE, this, MSG_ADD_CANDIDATES, - new CandidatesData(ch, c)); + main_.PostTask(SafeTask( + safety_, [this, ch, c = c]() mutable { AddCandidate(ch, c); })); } } @@ -851,70 +846,60 @@ class P2PTransportChannelTestBase : public ::testing::Test, void OnCandidatesRemoved(IceTransportInternal* ch, const std::vector& candidates) { // Candidate removals are not paused. - CandidatesData* candidates_data = new CandidatesData(ch, candidates); - main_.Post(RTC_FROM_HERE, this, MSG_REMOVE_CANDIDATES, candidates_data); + main_.PostTask(SafeTask(safety_, [this, ch, candidates]() mutable { + P2PTransportChannel* rch = GetRemoteChannel(ch); + if (rch == nullptr) { + return; + } + for (const Candidate& c : candidates) { + RTC_LOG(LS_INFO) << "Removed remote candidate " << c.ToString(); + rch->RemoveRemoteCandidate(c); + } + })); } // Tcp candidate verification has to be done when they are generated. void VerifySavedTcpCandidates(int endpoint, absl::string_view tcptype) { for (auto& data : GetEndpoint(endpoint)->saved_candidates_) { - for (auto& candidate : data->candidates) { - EXPECT_EQ(candidate.protocol(), TCP_PROTOCOL_NAME); - EXPECT_EQ(candidate.tcptype(), tcptype); - if (candidate.tcptype() == TCPTYPE_ACTIVE_STR) { - EXPECT_EQ(candidate.address().port(), DISCARD_PORT); - } else if (candidate.tcptype() == TCPTYPE_PASSIVE_STR) { - EXPECT_NE(candidate.address().port(), DISCARD_PORT); - } else { - FAIL() << "Unknown tcptype: " << candidate.tcptype(); - } + EXPECT_EQ(data.candidate.protocol(), TCP_PROTOCOL_NAME); + EXPECT_EQ(data.candidate.tcptype(), tcptype); + if (data.candidate.tcptype() == TCPTYPE_ACTIVE_STR) { + EXPECT_EQ(data.candidate.address().port(), DISCARD_PORT); + } else if (data.candidate.tcptype() == TCPTYPE_PASSIVE_STR) { + EXPECT_NE(data.candidate.address().port(), DISCARD_PORT); + } else { + FAIL() << "Unknown tcptype: " << data.candidate.tcptype(); } } } void ResumeCandidates(int endpoint) { Endpoint* ed = GetEndpoint(endpoint); - for (auto& candidate : ed->saved_candidates_) { - main_.Post(RTC_FROM_HERE, this, MSG_ADD_CANDIDATES, candidate.release()); + std::vector candidates = std::move(ed->saved_candidates_); + if (!candidates.empty()) { + main_.PostTask(SafeTask( + safety_, [this, candidates = std::move(candidates)]() mutable { + for (CandidateData& data : candidates) { + AddCandidate(data.channel, data.candidate); + } + })); } ed->saved_candidates_.clear(); ed->save_candidates_ = false; } - void OnMessage(rtc::Message* msg) { - switch (msg->message_id) { - case MSG_ADD_CANDIDATES: { - std::unique_ptr data( - static_cast(msg->pdata)); - P2PTransportChannel* rch = GetRemoteChannel(data->channel); - if (!rch) { - return; - } - for (auto& c : data->candidates) { - if (remote_ice_parameter_source_ != FROM_CANDIDATE) { - c.set_username(""); - c.set_password(""); - } - RTC_LOG(LS_INFO) << "Candidate(" << data->channel->component() << "->" - << rch->component() << "): " << c.ToString(); - rch->AddRemoteCandidate(c); - } - break; - } - case MSG_REMOVE_CANDIDATES: { - std::unique_ptr data( - static_cast(msg->pdata)); - P2PTransportChannel* rch = GetRemoteChannel(data->channel); - if (!rch) { - return; - } - for (Candidate& c : data->candidates) { - RTC_LOG(LS_INFO) << "Removed remote candidate " << c.ToString(); - rch->RemoveRemoteCandidate(c); - } - break; - } + void AddCandidate(IceTransportInternal* channel, Candidate& candidate) { + P2PTransportChannel* rch = GetRemoteChannel(channel); + if (rch == nullptr) { + return; } + if (remote_ice_parameter_source_ != FROM_CANDIDATE) { + candidate.set_username(""); + candidate.set_password(""); + } + RTC_LOG(LS_INFO) << "Candidate(" << channel->component() << "->" + << rch->component() << "): " << candidate.ToString(); + rch->AddRemoteCandidate(candidate); } void OnReadPacket(rtc::PacketTransportInternal* transport, @@ -1010,6 +995,8 @@ class P2PTransportChannelTestBase : public ::testing::Test, std::unique_ptr nss_; std::unique_ptr ss_; rtc::AutoSocketServerThread main_; + rtc::scoped_refptr safety_ = + PendingTaskSafetyFlag::Create(); std::unique_ptr stun_server_; TestTurnServer turn_server_; rtc::SocksProxyServer socks_server1_; @@ -5197,9 +5184,7 @@ TEST_F(P2PTransportChannelTest, PauseCandidates(0); PauseCandidates(1); ASSERT_EQ_WAIT(1u, GetEndpoint(0)->saved_candidates_.size(), kMediumTimeout); - ASSERT_EQ(1u, GetEndpoint(0)->saved_candidates_[0]->candidates.size()); - const auto& local_candidate = - GetEndpoint(0)->saved_candidates_[0]->candidates[0]; + const auto& local_candidate = GetEndpoint(0)->saved_candidates_[0].candidate; // The IP address of ep1's host candidate should be obfuscated. EXPECT_TRUE(local_candidate.address().IsUnresolvedIP()); // This is the underlying private IP address of the same candidate at ep1. @@ -5257,9 +5242,7 @@ TEST_F(P2PTransportChannelTest, PauseCandidates(1); ASSERT_EQ_WAIT(1u, GetEndpoint(0)->saved_candidates_.size(), kMediumTimeout); - ASSERT_EQ(1u, GetEndpoint(0)->saved_candidates_[0]->candidates.size()); - const auto& local_candidate = - GetEndpoint(0)->saved_candidates_[0]->candidates[0]; + const auto& local_candidate = GetEndpoint(0)->saved_candidates_[0].candidate; // The IP address of ep1's host candidate should be obfuscated. ASSERT_TRUE(local_candidate.address().IsUnresolvedIP()); // This is the underlying private IP address of the same candidate at ep1. @@ -5316,9 +5299,8 @@ TEST_F(P2PTransportChannelTest, CanConnectWithHostCandidateWithMdnsName) { PauseCandidates(0); PauseCandidates(1); ASSERT_EQ_WAIT(1u, GetEndpoint(0)->saved_candidates_.size(), kMediumTimeout); - ASSERT_EQ(1u, GetEndpoint(0)->saved_candidates_[0]->candidates.size()); const auto& local_candidate_ep1 = - GetEndpoint(0)->saved_candidates_[0]->candidates[0]; + GetEndpoint(0)->saved_candidates_[0].candidate; // The IP address of ep1's host candidate should be obfuscated. EXPECT_TRUE(local_candidate_ep1.address().IsUnresolvedIP()); // This is the underlying private IP address of the same candidate at ep1, @@ -5373,8 +5355,7 @@ TEST_F(P2PTransportChannelTest, ASSERT_EQ_WAIT(1u, GetEndpoint(1)->saved_candidates_.size(), kMediumTimeout); for (const auto& candidates_data : GetEndpoint(0)->saved_candidates_) { - ASSERT_EQ(1u, candidates_data->candidates.size()); - const auto& local_candidate_ep1 = candidates_data->candidates[0]; + const auto& local_candidate_ep1 = candidates_data.candidate; if (local_candidate_ep1.type() == LOCAL_PORT_TYPE) { // This is the underlying private IP address of the same candidate at ep1, // and let the mock resolver of ep2 receive the correct resolution. @@ -5546,8 +5527,7 @@ TEST_F(P2PTransportChannelTest, PauseCandidates(1); ASSERT_EQ_WAIT(1u, GetEndpoint(0)->saved_candidates_.size(), kMediumTimeout); const auto& candidates_data = GetEndpoint(0)->saved_candidates_[0]; - ASSERT_EQ(1u, candidates_data->candidates.size()); - const auto& local_candidate_ep1 = candidates_data->candidates[0]; + const auto& local_candidate_ep1 = candidates_data.candidate; ASSERT_TRUE(local_candidate_ep1.type() == LOCAL_PORT_TYPE); // This is the underlying private IP address of the same candidate at ep1, // and let the mock resolver of ep2 receive the correct resolution. @@ -6419,9 +6399,7 @@ TEST_F(P2PTransportChannelTest, TestIceNoOldCandidatesAfterIceRestart) { kDefaultTimeout, clock); for (const auto& cd : GetEndpoint(0)->saved_candidates_) { - for (const auto& c : cd->candidates) { - EXPECT_EQ(c.username(), kIceUfrag[3]); - } + EXPECT_EQ(cd.candidate.username(), kIceUfrag[3]); } DestroyChannels(); diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc index ef9b1d4cd7..efb88f5058 100644 --- a/p2p/base/turn_port_unittest.cc +++ b/p2p/base/turn_port_unittest.cc @@ -35,8 +35,6 @@ #include "rtc_base/checks.h" #include "rtc_base/fake_clock.h" #include "rtc_base/gunit.h" -#include "rtc_base/location.h" -#include "rtc_base/message_handler.h" #include "rtc_base/net_helper.h" #include "rtc_base/socket.h" #include "rtc_base/socket_address.h" @@ -118,8 +116,6 @@ static const cricket::ProtocolAddress kTurnPortHostnameProtoAddr( kTurnInvalidAddr, cricket::PROTO_UDP); -static const unsigned int MSG_TESTFINISH = 0; - #if defined(WEBRTC_LINUX) && !defined(WEBRTC_ANDROID) static int GetFDCount() { struct dirent* dp; @@ -175,9 +171,7 @@ class TestConnectionWrapper : public sigslot::has_slots<> { // Note: This test uses a fake clock with a simulated network round trip // (between local port and TURN server) of kSimulatedRtt. -class TurnPortTest : public ::testing::Test, - public sigslot::has_slots<>, - public rtc::MessageHandlerAutoCleanup { +class TurnPortTest : public ::testing::Test, public sigslot::has_slots<> { public: TurnPortTest() : ss_(new TurnPortTestVirtualSocketServer()), @@ -198,12 +192,6 @@ class TurnPortTest : public ::testing::Test, fake_clock_.AdvanceTime(webrtc::TimeDelta::Seconds(1)); } - virtual void OnMessage(rtc::Message* msg) { - RTC_CHECK(msg->message_id == MSG_TESTFINISH); - if (msg->message_id == MSG_TESTFINISH) - test_finish_ = true; - } - void OnTurnPortComplete(Port* port) { turn_ready_ = true; } void OnTurnPortError(Port* port) { turn_error_ = true; } void OnCandidateError(Port* port, @@ -1674,7 +1662,7 @@ TEST_F(TurnPortTest, TestResolverShutdown) { ASSERT_TRUE_WAIT(turn_error_, kResolverTimeout); EXPECT_TRUE(turn_port_->Candidates().empty()); turn_port_.reset(); - rtc::Thread::Current()->Post(RTC_FROM_HERE, this, MSG_TESTFINISH); + rtc::Thread::Current()->PostTask([this] { test_finish_ = true; }); // Waiting for above message to be processed. ASSERT_TRUE_SIMULATED_WAIT(test_finish_, 1, fake_clock_); EXPECT_EQ(last_fd_count, GetFDCount());