Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(1971)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: webrtc/p2p/base/p2ptransportchannel_unittest.cc

Issue 1648813004: Remove candidates when doing continual gathering (Closed) Base URL: https://chromium.googlesource.com/external/webrtc@master
Patch Set: Added tests, address comments, and merge with head Created 4 years, 9 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/p2p/base/p2ptransportchannel.cc ('k') | webrtc/p2p/base/transport.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/p2p/base/p2ptransportchannel_unittest.cc
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index c54f054fd844a9167efaa5e9c8a2742b4f44a172..a33a266c2e0ac3e1646f6afce1ca25c0e2cfa202 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -98,9 +98,7 @@ static const char* kIcePwd[4] = {"TESTICEPWD00000000000000",
static const uint64_t kTiebreaker1 = 11111;
static const uint64_t kTiebreaker2 = 22222;
-enum {
- MSG_CANDIDATE
-};
+enum { MSG_CANDIDATE, MSG_CANDIDATE_REMOVALS };
pthatcher1 2016/03/09 18:00:53 Can you call these MSG_ADD_CANDIDATES and MSG_REMO
honghaiz3 2016/03/10 02:34:16 Done.
static cricket::IceConfig CreateIceConfig(int receiving_timeout,
bool gather_continually,
@@ -216,12 +214,14 @@ class P2PTransportChannelTestBase : public testing::Test,
rtc::scoped_ptr<cricket::P2PTransportChannel> ch_;
};
- struct CandidateData : public rtc::MessageData {
- CandidateData(cricket::TransportChannel* ch, const cricket::Candidate& c)
- : channel(ch), candidate(c) {
- }
+ struct CandidatesData : public rtc::MessageData {
+ CandidatesData(cricket::TransportChannel* ch, const cricket::Candidate& c)
+ : channel(ch), candidates(1, c) {}
+ CandidatesData(cricket::TransportChannel* ch,
+ const std::vector<cricket::Candidate>& cc)
+ : channel(ch), candidates(cc) {}
cricket::TransportChannel* channel;
- cricket::Candidate candidate;
+ cricket::Candidates candidates;
};
struct Endpoint {
@@ -262,7 +262,7 @@ class P2PTransportChannelTestBase : public testing::Test,
uint64_t tiebreaker_;
bool role_conflict_;
bool save_candidates_;
- std::vector<CandidateData*> saved_candidates_;
+ std::vector<CandidatesData*> saved_candidates_;
};
ChannelData* GetChannelData(cricket::TransportChannel* channel) {
@@ -311,6 +311,8 @@ class P2PTransportChannelTestBase : public testing::Test,
"test content name", component, GetAllocator(endpoint));
channel->SignalCandidateGathered.connect(
this, &P2PTransportChannelTestBase::OnCandidate);
pthatcher1 2016/03/09 18:00:53 Can you rename this OnCandidateGathered?
honghaiz3 2016/03/10 02:34:16 Done.
+ channel->SignalCandidatesRemoved.connect(
+ this, &P2PTransportChannelTestBase::OnCandidatesRemoved);
channel->SignalReadPacket.connect(
this, &P2PTransportChannelTestBase::OnReadPacket);
channel->SignalRoleConflict.connect(
@@ -651,9 +653,9 @@ class P2PTransportChannelTestBase : public testing::Test,
return;
if (GetEndpoint(ch)->save_candidates_) {
- GetEndpoint(ch)->saved_candidates_.push_back(new CandidateData(ch, c));
+ GetEndpoint(ch)->saved_candidates_.push_back(new CandidatesData(ch, c));
} else {
- main_->Post(this, MSG_CANDIDATE, new CandidateData(ch, c));
+ main_->Post(this, MSG_CANDIDATE, new CandidatesData(ch, c));
}
}
@@ -661,24 +663,32 @@ class P2PTransportChannelTestBase : public testing::Test,
GetEndpoint(endpoint)->save_candidates_ = true;
}
+ void OnCandidatesRemoved(cricket::TransportChannelImpl* ch,
+ const std::vector<cricket::Candidate>& candidates) {
+ // Candidate removals are not paused.
+ CandidatesData* candidates_data = new CandidatesData(ch, candidates);
+ main_->Post(this, MSG_CANDIDATE_REMOVALS, candidates_data);
+ }
+
// Tcp candidate verification has to be done when they are generated.
void VerifySavedTcpCandidates(int endpoint, const std::string& tcptype) {
for (auto& data : GetEndpoint(endpoint)->saved_candidates_) {
- EXPECT_EQ(data->candidate.protocol(), cricket::TCP_PROTOCOL_NAME);
- EXPECT_EQ(data->candidate.tcptype(), tcptype);
- if (data->candidate.tcptype() == cricket::TCPTYPE_ACTIVE_STR) {
- EXPECT_EQ(data->candidate.address().port(), cricket::DISCARD_PORT);
- } else if (data->candidate.tcptype() == cricket::TCPTYPE_PASSIVE_STR) {
- EXPECT_NE(data->candidate.address().port(), cricket::DISCARD_PORT);
+ cricket::Candidate& candidate = data->candidates[0];
pthatcher1 2016/03/09 18:00:53 Why not just make this a for loop?
honghaiz3 2016/03/10 02:34:16 Done.
+ EXPECT_EQ(candidate.protocol(), cricket::TCP_PROTOCOL_NAME);
+ EXPECT_EQ(candidate.tcptype(), tcptype);
+ if (candidate.tcptype() == cricket::TCPTYPE_ACTIVE_STR) {
+ EXPECT_EQ(candidate.address().port(), cricket::DISCARD_PORT);
+ } else if (candidate.tcptype() == cricket::TCPTYPE_PASSIVE_STR) {
+ EXPECT_NE(candidate.address().port(), cricket::DISCARD_PORT);
} else {
- FAIL() << "Unknown tcptype: " << data->candidate.tcptype();
+ FAIL() << "Unknown tcptype: " << candidate.tcptype();
}
}
}
void ResumeCandidates(int endpoint) {
Endpoint* ed = GetEndpoint(endpoint);
- std::vector<CandidateData*>::iterator it = ed->saved_candidates_.begin();
+ std::vector<CandidatesData*>::iterator it = ed->saved_candidates_.begin();
for (; it != ed->saved_candidates_.end(); ++it) {
main_->Post(this, MSG_CANDIDATE, *it);
}
@@ -689,10 +699,11 @@ class P2PTransportChannelTestBase : public testing::Test,
void OnMessage(rtc::Message* msg) {
switch (msg->message_id) {
case MSG_CANDIDATE: {
- rtc::scoped_ptr<CandidateData> data(
- static_cast<CandidateData*>(msg->pdata));
+ rtc::scoped_ptr<CandidatesData> data(
+ static_cast<CandidatesData*>(msg->pdata));
cricket::P2PTransportChannel* rch = GetRemoteChannel(data->channel);
- cricket::Candidate c = data->candidate;
+ // Candidates are currently added one at a time.
+ cricket::Candidate c = data->candidates[0];
pthatcher1 2016/03/09 18:00:53 Why not make this a for loop?
honghaiz3 2016/03/10 02:34:16 Done.
if (clear_remote_candidates_ufrag_pwd_) {
c.set_username("");
c.set_password("");
@@ -702,6 +713,16 @@ class P2PTransportChannelTestBase : public testing::Test,
rch->AddRemoteCandidate(c);
break;
}
+ case MSG_CANDIDATE_REMOVALS: {
+ rtc::scoped_ptr<CandidatesData> data(
+ static_cast<CandidatesData*>(msg->pdata));
+ cricket::P2PTransportChannel* rch = GetRemoteChannel(data->channel);
+ for (cricket::Candidate& c : data->candidates) {
+ LOG(LS_INFO) << "Removed remote candidate " << c.ToString();
+ rch->RemoveRemoteCandidate(c);
+ }
+ break;
+ }
}
}
void OnReadPacket(cricket::TransportChannel* channel, const char* data,
@@ -1772,9 +1793,10 @@ TEST_F(P2PTransportChannelMultihomedTest, TestGetState) {
ep2_ch1()->GetState(), 1000);
}
-// Tests that when a network interface becomes inactive, the ports associated
-// with that network will be removed from the port list of the channel if
-// and only if Continual Gathering is enabled.
+// Tests that when a network interface becomes inactive, if and only if
+// Continual Gathering is enabled, the ports associated with that network
+// will be removed from the port list of the channel, and the respective
+// remote candidates on the other participant will be removed eventually.
TEST_F(P2PTransportChannelMultihomedTest, TestNetworkBecomesInactive) {
AddAddress(0, kPublicAddrs[0]);
AddAddress(1, kPublicAddrs[1]);
@@ -1793,14 +1815,20 @@ TEST_F(P2PTransportChannelMultihomedTest, TestNetworkBecomesInactive) {
// Endpoint 1 enabled continual gathering; the port will be removed
// when the interface is removed.
RemoveAddress(0, kPublicAddrs[0]);
- EXPECT_EQ(0U, ep1_ch1()->ports().size());
+ EXPECT_TRUE(ep1_ch1()->ports().empty());
+ // The remote candidates will be removed eventually.
+ EXPECT_TRUE_WAIT(ep2_ch1()->remote_candidates().empty(), 1000);
size_t num_ports = ep2_ch1()->ports().size();
EXPECT_LE(1U, num_ports);
+ size_t num_remote_candidates = ep1_ch1()->remote_candidates().size();
// Endpoint 2 did not enable continual gathering; the port will not be removed
- // when the interface is removed.
+ // when the interface is removed and neither the remote candidates on the
+ // other participant.
RemoveAddress(1, kPublicAddrs[1]);
+ rtc::Thread::Current()->ProcessMessages(500);
EXPECT_EQ(num_ports, ep2_ch1()->ports().size());
+ EXPECT_EQ(num_remote_candidates, ep1_ch1()->remote_candidates().size());
}
/*
« no previous file with comments | « webrtc/p2p/base/p2ptransportchannel.cc ('k') | webrtc/p2p/base/transport.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698