Migrate away from rtc::MessageHandler in p2p unittests
Bug: webrtc:9702, webrtc:11318
Change-Id: Ifde789af67f9761fc4a88b398d250bd83eba94de
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/271287
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37796}
diff --git a/p2p/base/p2p_transport_channel_unittest.cc b/p2p/base/p2p_transport_channel_unittest.cc
index 28576f4..9a860d2 100644
--- a/p2p/base/p2p_transport_channel_unittest.cc
+++ b/p2p/base/p2p_transport_channel_unittest.cc
@@ -62,6 +62,8 @@
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 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 @@
// 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 @@
std::unique_ptr<P2PTransportChannel> ch_;
};
- struct CandidatesData : public rtc::MessageData {
- CandidatesData(IceTransportInternal* ch, const Candidate& c)
- : channel(ch), candidates(1, c) {}
- CandidatesData(IceTransportInternal* ch, const std::vector<Candidate>& cc)
- : channel(ch), candidates(cc) {}
+ struct CandidateData {
IceTransportInternal* channel;
- Candidates candidates;
+ Candidate candidate;
};
struct Endpoint : public sigslot::has_slots<> {
@@ -406,7 +401,7 @@
uint64_t tiebreaker_;
bool role_conflict_;
bool save_candidates_;
- std::vector<std::unique_ptr<CandidatesData>> saved_candidates_;
+ std::vector<CandidateData> saved_candidates_;
bool ready_to_send_ = false;
std::map<IceRegatheringReason, int> ice_regathering_counter_;
};
@@ -487,7 +482,7 @@
}
void DestroyChannels() {
- main_.Clear(this);
+ safety_->SetNotAlive();
ep1_.cd1_.ch_.reset();
ep2_.cd1_.ch_.reset();
ep1_.cd2_.ch_.reset();
@@ -822,10 +817,10 @@
if (GetEndpoint(ch)->save_candidates_) {
GetEndpoint(ch)->saved_candidates_.push_back(
- std::unique_ptr<CandidatesData>(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 @@
void OnCandidatesRemoved(IceTransportInternal* ch,
const std::vector<Candidate>& 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<CandidateData> 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<CandidatesData> data(
- static_cast<CandidatesData*>(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<CandidatesData> data(
- static_cast<CandidatesData*>(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 @@
std::unique_ptr<rtc::NATSocketServer> nss_;
std::unique_ptr<rtc::FirewallSocketServer> ss_;
rtc::AutoSocketServerThread main_;
+ rtc::scoped_refptr<PendingTaskSafetyFlag> safety_ =
+ PendingTaskSafetyFlag::Create();
std::unique_ptr<TestStunServer> stun_server_;
TestTurnServer turn_server_;
rtc::SocksProxyServer socks_server1_;
@@ -5197,9 +5184,7 @@
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 @@
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 @@
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 @@
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 @@
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 @@
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 ef9b1d4..efb88f5 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 @@
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 @@
// 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 @@
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 @@
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());
