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}

p2p/base/stun_port.h

@@ -114,10 +114,6 @@ class UDPPort : public Port {
   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_; }
 

p2p/base/stun_port_unittest.cc

@@ -64,6 +64,10 @@ class StunPortTestBase : public ::testing::Test, public sigslot::has_slots<> {
   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 @@ TEST_F(StunPortTest, TestStunBindingRequestShortLifetime) {
   CreateStunPort(kStunAddr1);
   PrepareAddress();
   EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock);
-  EXPECT_TRUE_SIMULATED_WAIT(
+  EXPECT_TRUE_SIMULATED_WAIT(!HasPendingRequest(cricket::STUN_BINDING_REQUEST),
-      !port()->HasPendingRequestForTest(cricket::STUN_BINDING_REQUEST), 2000,
+                             2000, fake_clock);
-      fake_clock);
 }
 
 // Test that by default, the STUN binding requests will last for a long time.
@@ -403,9 +406,8 @@ TEST_F(StunPortTest, TestStunBindingRequestLongLifetime) {
   CreateStunPort(kStunAddr1);
   PrepareAddress();
   EXPECT_TRUE_SIMULATED_WAIT(done(), kTimeoutMs, fake_clock);
-  EXPECT_TRUE_SIMULATED_WAIT(
+  EXPECT_TRUE_SIMULATED_WAIT(HasPendingRequest(cricket::STUN_BINDING_REQUEST),
-      port()->HasPendingRequestForTest(cricket::STUN_BINDING_REQUEST), 1000,
+                             1000, fake_clock);
-      fake_clock);
 }
 
 class MockAsyncPacketSocket : public rtc::AsyncPacketSocket {

p2p/base/stun_request.cc

@@ -65,7 +65,7 @@ void StunRequestManager::SendDelayed(StunRequest* request, int delay) {
 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 @@ void StunRequestManager::FlushForTest(int msg_type) {
 
 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;
     }
   }

p2p/base/stun_request.h

@@ -28,7 +28,7 @@ namespace cricket {
 
 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 @@ class StunRequestManager {
   void Send(StunRequest* request);
   void SendDelayed(StunRequest* request, int delay);
 
-  // If `msg_type` is kAllRequests, sends all pending requests right away.
+  // If `msg_type` is kAllRequestsForTest, sends all pending requests right
-  // Otherwise, sends those that have a matching type right away.
+  // away. Otherwise, sends those that have a matching type right away. Only for
-  // Only for testing.
+  // testing.
   // TODO(tommi): Remove this method and update tests that use it to simulate
   // production code.
   void FlushForTest(int msg_type);

p2p/base/turn_port.h

@@ -191,10 +191,6 @@ class TurnPort : public Port {
   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;

p2p/base/turn_port_unittest.cc

@@ -1244,10 +1244,10 @@ TEST_F(TurnPortTest, TestRefreshRequestGetsErrorResponse) {
   // 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 @@ TEST_F(TurnPortTest, TestRefreshCreatePermissionRequest) {
   // 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));