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

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

Issue 2171183002: Remove ports that are not used by any channel after timeout (Closed) Base URL: https://chromium.googlesource.com/external/webrtc@master
Patch Set: fix a comment Created 4 years, 5 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
Index: webrtc/p2p/base/port_unittest.cc
diff --git a/webrtc/p2p/base/port_unittest.cc b/webrtc/p2p/base/port_unittest.cc
index e156e017fe8015db092a6d68265e3b37ceef85b9..f284dd494052bf4b0b73be61e87502c837ec4dd7 100644
--- a/webrtc/p2p/base/port_unittest.cc
+++ b/webrtc/p2p/base/port_unittest.cc
@@ -384,7 +384,7 @@ class PortTest : public testing::Test, public sigslot::has_slots<> {
username_(rtc::CreateRandomString(ICE_UFRAG_LENGTH)),
password_(rtc::CreateRandomString(ICE_PWD_LENGTH)),
role_conflict_(false),
- destroyed_(false) {
+ ports_destroyed_(0) {
network_.AddIP(rtc::IPAddress(INADDR_ANY));
}
@@ -755,10 +755,8 @@ class PortTest : public testing::Test, public sigslot::has_slots<> {
port->SignalDestroyed.connect(this, &PortTest::OnDestroyed);
}
- void OnDestroyed(PortInterface* port) {
- destroyed_ = true;
- }
- bool destroyed() const { return destroyed_; }
+ void OnDestroyed(PortInterface* port) { ++ports_destroyed_; }
+ int ports_destroyed() const { return ports_destroyed_; }
rtc::BasicPacketSocketFactory* nat_socket_factory1() {
return &nat_socket_factory1_;
@@ -785,7 +783,7 @@ class PortTest : public testing::Test, public sigslot::has_slots<> {
std::string username_;
std::string password_;
bool role_conflict_;
- bool destroyed_;
+ int ports_destroyed_;
};
void PortTest::TestConnectivity(const char* name1, Port* port1,
@@ -2567,93 +2565,70 @@ TEST_F(PortTest, TestIceLiteConnectivity) {
ch1.Stop();
}
-// This test case verifies that the CONTROLLING port does not time out.
-TEST_F(PortTest, TestControllingNoTimeout) {
+// This test case verifies that both the controlling port and the controlled
+// port will time out after connectivity is lost, if they are not used.
+TEST_F(PortTest, TestPortTimeoutIfNotUsed) {
+ rtc::ScopedFakeClock clock;
+ int timeout_delay = 100;
UDPPort* port1 = CreateUdpPort(kLocalAddr1);
ConnectToSignalDestroyed(port1);
- port1->set_timeout_delay(10); // milliseconds
- port1->SetIceRole(cricket::ICEROLE_CONTROLLING);
- port1->SetIceTiebreaker(kTiebreaker1);
-
- UDPPort* port2 = CreateUdpPort(kLocalAddr2);
- port2->SetIceRole(cricket::ICEROLE_CONTROLLED);
- port2->SetIceTiebreaker(kTiebreaker2);
-
- // Set up channels and ensure both ports will be deleted.
- TestChannel ch1(port1);
- TestChannel ch2(port2);
-
- // Simulate a connection that succeeds, and then is destroyed.
- StartConnectAndStopChannels(&ch1, &ch2);
-
- // After the connection is destroyed, the port should not be destroyed.
- rtc::Thread::Current()->ProcessMessages(kTimeout);
- EXPECT_FALSE(destroyed());
-}
-
-// This test case verifies that the CONTROLLED port does time out, but only
-// after connectivity is lost.
-TEST_F(PortTest, TestControlledTimeout) {
- UDPPort* port1 = CreateUdpPort(kLocalAddr1);
+ port1->set_timeout_delay(timeout_delay); // milliseconds
port1->SetIceRole(cricket::ICEROLE_CONTROLLING);
port1->SetIceTiebreaker(kTiebreaker1);
UDPPort* port2 = CreateUdpPort(kLocalAddr2);
ConnectToSignalDestroyed(port2);
- port2->set_timeout_delay(10); // milliseconds
+ port2->set_timeout_delay(timeout_delay); // milliseconds
+
port2->SetIceRole(cricket::ICEROLE_CONTROLLED);
port2->SetIceTiebreaker(kTiebreaker2);
- // The connection must not be destroyed before a connection is attempted.
- EXPECT_FALSE(destroyed());
-
- port1->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
- port2->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
-
// Set up channels and ensure both ports will be deleted.
TestChannel ch1(port1);
TestChannel ch2(port2);
// Simulate a connection that succeeds, and then is destroyed.
StartConnectAndStopChannels(&ch1, &ch2);
-
- // The controlled port should be destroyed after 10 milliseconds.
- EXPECT_TRUE_WAIT(destroyed(), kTimeout);
+ // After the connection is destroyed, the port will be destroyed because
+ // none of them is marked as being used.
+ EXPECT_EQ_SIMULATED_WAIT(2, ports_destroyed(), kTimeout, clock);
}
-// This test case verifies that if the role of a port changes from controlled
-// to controlling after all connections fail, the port will not be destroyed.
-TEST_F(PortTest, TestControlledToControllingNotDestroyed) {
+// This test case verifies that neither the controlling port nor the controlled
+// port will time out after connectivity is lost, if it is used. If it is marked
+// as "not used" later, it will be destroyed.
+TEST_F(PortTest, TestPortNotTimeoutUntilNotUsed) {
+ rtc::ScopedFakeClock clock;
+ int timeout_delay = 100;
UDPPort* port1 = CreateUdpPort(kLocalAddr1);
+ ConnectToSignalDestroyed(port1);
+ port1->set_timeout_delay(timeout_delay); // milliseconds
port1->SetIceRole(cricket::ICEROLE_CONTROLLING);
port1->SetIceTiebreaker(kTiebreaker1);
UDPPort* port2 = CreateUdpPort(kLocalAddr2);
ConnectToSignalDestroyed(port2);
- port2->set_timeout_delay(10); // milliseconds
+ port2->set_timeout_delay(timeout_delay); // milliseconds
port2->SetIceRole(cricket::ICEROLE_CONTROLLED);
port2->SetIceTiebreaker(kTiebreaker2);
- // The connection must not be destroyed before a connection is attempted.
- EXPECT_FALSE(destroyed());
-
- port1->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
- port2->set_component(cricket::ICE_CANDIDATE_COMPONENT_DEFAULT);
-
- // Set up channels and ensure both ports will be deleted.
+ // Set up channels and start using the ports.
TestChannel ch1(port1);
TestChannel ch2(port2);
+ port1->StartBeingUsed();
+ port2->StartBeingUsed();
// Simulate a connection that succeeds, and then is destroyed.
StartConnectAndStopChannels(&ch1, &ch2);
- // Switch the role after all connections are destroyed.
- EXPECT_TRUE_WAIT(ch2.conn() == nullptr, kTimeout);
- port1->SetIceRole(cricket::ICEROLE_CONTROLLED);
- port2->SetIceRole(cricket::ICEROLE_CONTROLLING);
-
- // After the connection is destroyed, the port should not be destroyed.
- rtc::Thread::Current()->ProcessMessages(kTimeout);
- EXPECT_FALSE(destroyed());
+ // After the connection is destroyed, the ports will not destroyed because
+ // they are marked as being used.
+ SIMULATED_WAIT(ports_destroyed() > 0, kTimeout, clock);
+ EXPECT_EQ(0, ports_destroyed());
+
+ // They will be destroyed right away if it is marked as not being used.
+ port1->StopBeingUsed();
+ port2->StopBeingUsed();
+ EXPECT_EQ_SIMULATED_WAIT(2, ports_destroyed(), 1, clock);
}
TEST_F(PortTest, TestSupportsProtocol) {

Powered by Google App Engine
This is Rietveld 408576698