A few cleanup things for the port classes to clarify test code.
Remove FlushRequestsForTest
Rename test constant
Remove HasPendingRequestForTest
Bug: webrtc:13892
Change-Id: I78e13d229742c40743465b5fb57480c24d7417c1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/258722
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37466}
diff --git a/p2p/base/stun_port.h b/p2p/base/stun_port.h
index 83a90fa..7a3239e 100644
--- a/p2p/base/stun_port.h
+++ b/p2p/base/stun_port.h
@@ -114,10 +114,6 @@
void set_stun_keepalive_lifetime(int lifetime) {
stun_keepalive_lifetime_ = lifetime;
}
- // Returns true if there is a pending request with type `msg_type`.
- bool HasPendingRequestForTest(int msg_type) {
- return request_manager_.HasRequestForTest(msg_type);
- }
StunRequestManager& request_manager() { return request_manager_; }
diff --git a/p2p/base/stun_port_unittest.cc b/p2p/base/stun_port_unittest.cc
index fa51ed6..d9c6500 100644
--- a/p2p/base/stun_port_unittest.cc
+++ b/p2p/base/stun_port_unittest.cc
@@ -64,6 +64,10 @@
bool done() const { return done_; }
bool error() const { return error_; }
+ bool HasPendingRequest(int msg_type) {
+ return stun_port_->request_manager().HasRequestForTest(msg_type);
+ }
+
void SetNetworkType(rtc::AdapterType adapter_type) {
network_.set_type(adapter_type);
}
@@ -392,9 +396,8 @@
CreateStunPort(kStunAddr1);
PrepareAddress();
EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock);
- EXPECT_TRUE_SIMULATED_WAIT(
- !port()->HasPendingRequestForTest(cricket::STUN_BINDING_REQUEST), 2000,
- fake_clock);
+ EXPECT_TRUE_SIMULATED_WAIT(!HasPendingRequest(cricket::STUN_BINDING_REQUEST),
+ 2000, fake_clock);
}
// Test that by default, the STUN binding requests will last for a long time.
@@ -403,9 +406,8 @@
CreateStunPort(kStunAddr1);
PrepareAddress();
EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock);
- EXPECT_TRUE_SIMULATED_WAIT(
- port()->HasPendingRequestForTest(cricket::STUN_BINDING_REQUEST), 1000,
- fake_clock);
+ EXPECT_TRUE_SIMULATED_WAIT(HasPendingRequest(cricket::STUN_BINDING_REQUEST),
+ 1000, fake_clock);
}
class MockAsyncPacketSocket : public rtc::AsyncPacketSocket {
diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc
index b4bce73..3fe02af 100644
--- a/p2p/base/stun_request.cc
+++ b/p2p/base/stun_request.cc
@@ -65,7 +65,7 @@
void StunRequestManager::FlushForTest(int msg_type) {
RTC_DCHECK_RUN_ON(thread_);
for (const auto& [unused, request] : requests_) {
- if (msg_type == kAllRequests || msg_type == request->type()) {
+ if (msg_type == kAllRequestsForTest || msg_type == request->type()) {
// Calling `Send` implies starting the send operation which may be posted
// on a timer and be repeated on a timer until timeout. To make sure that
// a call to `Send` doesn't conflict with a previously started `Send`
@@ -80,8 +80,9 @@
bool StunRequestManager::HasRequestForTest(int msg_type) {
RTC_DCHECK_RUN_ON(thread_);
+ RTC_DCHECK_NE(msg_type, kAllRequestsForTest);
for (const auto& [unused, request] : requests_) {
- if (msg_type == kAllRequests || msg_type == request->type()) {
+ if (msg_type == request->type()) {
return true;
}
}
diff --git a/p2p/base/stun_request.h b/p2p/base/stun_request.h
index cc1e1bf..6e83be38 100644
--- a/p2p/base/stun_request.h
+++ b/p2p/base/stun_request.h
@@ -28,7 +28,7 @@
class StunRequest;
-const int kAllRequests = 0;
+const int kAllRequestsForTest = 0;
// Total max timeouts: 39.75 seconds
// For years, this was 9.5 seconds, but for networks that experience moments of
@@ -48,9 +48,9 @@
void Send(StunRequest* request);
void SendDelayed(StunRequest* request, int delay);
- // If `msg_type` is kAllRequests, sends all pending requests right away.
- // Otherwise, sends those that have a matching type right away.
- // Only for testing.
+ // If `msg_type` is kAllRequestsForTest, sends all pending requests right
+ // away. Otherwise, sends those that have a matching type right away. Only for
+ // testing.
// TODO(tommi): Remove this method and update tests that use it to simulate
// production code.
void FlushForTest(int msg_type);
diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h
index 8a77b9f..2e61d36 100644
--- a/p2p/base/turn_port.h
+++ b/p2p/base/turn_port.h
@@ -191,10 +191,6 @@
sigslot::signal3<TurnPort*, const rtc::SocketAddress&, int>
SignalCreatePermissionResult;
- void FlushRequestsForTest(int msg_type) {
- request_manager_.FlushForTest(msg_type);
- }
-
bool HasRequests() { return !request_manager_.empty(); }
void set_credentials(const RelayCredentials& credentials) {
credentials_ = credentials;
diff --git a/p2p/base/turn_port_unittest.cc b/p2p/base/turn_port_unittest.cc
index ff529cb..ef9b1d4 100644
--- a/p2p/base/turn_port_unittest.cc
+++ b/p2p/base/turn_port_unittest.cc
@@ -1244,10 +1244,10 @@
// This sends out the first RefreshRequest with correct credentials.
// When this succeeds, it will schedule a new RefreshRequest with the bad
// credential.
- turn_port_->FlushRequestsForTest(TURN_REFRESH_REQUEST);
+ turn_port_->request_manager().FlushForTest(TURN_REFRESH_REQUEST);
EXPECT_TRUE_SIMULATED_WAIT(turn_refresh_success_, kSimulatedRtt, fake_clock_);
// Flush it again, it will receive a bad response.
- turn_port_->FlushRequestsForTest(TURN_REFRESH_REQUEST);
+ turn_port_->request_manager().FlushForTest(TURN_REFRESH_REQUEST);
EXPECT_TRUE_SIMULATED_WAIT(!turn_refresh_success_, kSimulatedRtt,
fake_clock_);
EXPECT_FALSE(turn_port_->connected());
@@ -1461,11 +1461,11 @@
// another request with bad_ufrag and bad_pwd.
RelayCredentials bad_credentials("bad_user", "bad_pwd");
turn_port_->set_credentials(bad_credentials);
- turn_port_->FlushRequestsForTest(kAllRequests);
+ turn_port_->request_manager().FlushForTest(kAllRequestsForTest);
EXPECT_TRUE_SIMULATED_WAIT(turn_create_permission_success_, kSimulatedRtt,
fake_clock_);
// Flush the requests again; the create-permission-request will fail.
- turn_port_->FlushRequestsForTest(kAllRequests);
+ turn_port_->request_manager().FlushForTest(kAllRequestsForTest);
EXPECT_TRUE_SIMULATED_WAIT(!turn_create_permission_success_, kSimulatedRtt,
fake_clock_);
EXPECT_TRUE(CheckConnectionFailedAndPruned(conn));
