Propagate clock into StunRequest
While at it migrate some integers to strong time types
Update Send function signature to use unique_ptr to communicate request
is passed with ownership
Bug: webrtc:42223992
Change-Id: I177dc20ce1d40d5dec9efaf9f37b46ee4f994ea1
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/407080
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45510}
diff --git a/p2p/BUILD.gn b/p2p/BUILD.gn
index 2baa0e2..281a0d0 100644
--- a/p2p/BUILD.gn
+++ b/p2p/BUILD.gn
@@ -758,6 +758,8 @@
"../api:local_network_access_permission",
"../api:packet_socket_factory",
"../api/transport:stun_types",
+ "../api/units:time_delta",
+ "../api/units:timestamp",
"../rtc_base:async_packet_socket",
"../rtc_base:checks",
"../rtc_base:dscp",
@@ -787,10 +789,12 @@
deps = [
"../api:array_view",
"../api:sequence_checker",
+ "../api/environment",
"../api/task_queue",
"../api/task_queue:pending_task_safety_flag",
"../api/transport:stun_types",
"../api/units:time_delta",
+ "../api/units:timestamp",
"../rtc_base:byte_buffer",
"../rtc_base:checks",
"../rtc_base:crypto_random",
@@ -920,6 +924,7 @@
"../rtc_base:logging",
"../rtc_base:net_helper",
"../rtc_base:network",
+ "../rtc_base:platform_thread_types",
"../rtc_base:socket",
"../rtc_base:socket_address",
"../rtc_base:ssl",
diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc
index 5bdf070..1afd41b 100644
--- a/p2p/base/connection.cc
+++ b/p2p/base/connection.cc
@@ -180,7 +180,8 @@
// A ConnectionRequest is a STUN binding used to determine writability.
class Connection::ConnectionRequest : public StunRequest {
public:
- ConnectionRequest(StunRequestManager& manager,
+ ConnectionRequest(const Environment& env,
+ StunRequestManager& manager,
Connection* connection,
std::unique_ptr<IceMessage> message);
void OnResponse(StunMessage* response) override;
@@ -194,10 +195,11 @@
};
Connection::ConnectionRequest::ConnectionRequest(
+ const Environment& env,
StunRequestManager& manager,
Connection* connection,
std::unique_ptr<IceMessage> message)
- : StunRequest(manager, std::move(message)), connection_(connection) {}
+ : StunRequest(env, manager, std::move(message)), connection_(connection) {}
void Connection::ConnectionRequest::OnResponse(StunMessage* response) {
RTC_DCHECK_RUN_ON(connection_->network_thread_);
@@ -1103,19 +1105,20 @@
bool has_delta = delta != nullptr;
auto req = std::make_unique<ConnectionRequest>(
- requests_, this, BuildPingRequest(std::move(delta)));
+ env_, requests_, this, BuildPingRequest(std::move(delta)));
if (!has_delta && ShouldSendGoogPing(req->msg())) {
auto message = std::make_unique<IceMessage>(GOOG_PING_REQUEST, req->id());
message->AddMessageIntegrity32(remote_candidate_.password());
- req.reset(new ConnectionRequest(requests_, this, std::move(message)));
+ req = std::make_unique<ConnectionRequest>(env_, requests_, this,
+ std::move(message));
}
pings_since_last_response_.push_back(SentPing(req->id(), now, nomination));
RTC_LOG(LS_VERBOSE) << ToString()
<< ": Sending STUN ping, id=" << hex_encode(req->id())
<< ", nomination=" << nomination_;
- requests_.Send(req.release());
+ requests_.Send(std::move(req));
state_ = IceCandidatePairState::IN_PROGRESS;
num_pings_sent_++;
}
@@ -1513,7 +1516,7 @@
// connection.
LoggingSeverity sev = !writable() ? LS_INFO : LS_VERBOSE;
- TimeDelta rtt = TimeDelta::Millis(request->Elapsed());
+ TimeDelta rtt = request->Elapsed();
if (RTC_LOG_CHECK_LEVEL_V(sev)) {
std::string pings;
diff --git a/p2p/base/stun_port.cc b/p2p/base/stun_port.cc
index b6b2c53..9903a16 100644
--- a/p2p/base/stun_port.cc
+++ b/p2p/base/stun_port.cc
@@ -26,6 +26,8 @@
#include "api/local_network_access_permission.h"
#include "api/packet_socket_factory.h"
#include "api/transport/stun.h"
+#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
#include "p2p/base/connection.h"
#include "p2p/base/p2p_constants.h"
#include "p2p/base/port.h"
@@ -43,25 +45,27 @@
#include "rtc_base/socket.h"
#include "rtc_base/socket_address.h"
#include "rtc_base/strings/string_builder.h"
-#include "rtc_base/time_utils.h"
#include "system_wrappers/include/metrics.h"
namespace webrtc {
-
+namespace {
// TODO(?): Move these to a common place (used in relayport too)
-const int RETRY_TIMEOUT = 50 * 1000; // 50 seconds
+constexpr TimeDelta kRetryTimeout = TimeDelta::Seconds(50);
// Stop logging errors in UDPPort::SendTo after we have logged
// `kSendErrorLogLimit` messages. Start again after a successful send.
-const int kSendErrorLogLimit = 5;
+constexpr int kSendErrorLogLimit = 5;
+
+} // namespace
// Handles a binding request sent to the STUN server.
class StunBindingRequest : public StunRequest {
public:
StunBindingRequest(UDPPort* port,
const SocketAddress& addr,
- int64_t start_time)
- : StunRequest(port->request_manager(),
+ Timestamp start_time)
+ : StunRequest(port->env(),
+ port->request_manager(),
std::make_unique<StunMessage>(STUN_BINDING_REQUEST)),
port_(port),
server_addr_(addr),
@@ -85,10 +89,10 @@
}
// The keep-alive requests will be stopped after its lifetime has passed.
- if (WithinLifetime(TimeMillis())) {
- port_->request_manager_.SendDelayed(
- new StunBindingRequest(port_, server_addr_, start_time_),
- port_->stun_keepalive_delay());
+ if (WithinLifetime(Connection::AlignTime(env().clock().CurrentTime()))) {
+ port_->request_manager_.Send(std::make_unique<StunBindingRequest>(
+ port_, server_addr_, start_time_),
+ /*delay=*/port_->stun_keepalive_delay());
}
}
@@ -108,11 +112,11 @@
attr ? attr->reason()
: "STUN binding response with no error code attribute.");
- int64_t now = TimeMillis();
- if (WithinLifetime(now) && TimeDiff(now, start_time_) < RETRY_TIMEOUT) {
- port_->request_manager_.SendDelayed(
- new StunBindingRequest(port_, server_addr_, start_time_),
- port_->stun_keepalive_delay());
+ Timestamp now = Connection::AlignTime(env().clock().CurrentTime());
+ if (WithinLifetime(now) && now - start_time_ < kRetryTimeout) {
+ port_->request_manager_.Send(std::make_unique<StunBindingRequest>(
+ port_, server_addr_, start_time_),
+ /*delay=*/port_->stun_keepalive_delay());
}
}
void OnTimeout() override {
@@ -125,17 +129,15 @@
}
private:
- // Returns true if `now` is within the lifetime of the request (a negative
- // lifetime means infinite).
- bool WithinLifetime(int64_t now) const {
- int lifetime = port_->stun_keepalive_lifetime();
- return lifetime < 0 || TimeDiff(now, start_time_) <= lifetime;
+ // Returns true if `now` is within the lifetime of the request.
+ bool WithinLifetime(Timestamp now) const {
+ return now - start_time_ <= port_->stun_keepalive_lifetime();
}
UDPPort* port_;
const SocketAddress server_addr_;
- int64_t start_time_;
+ Timestamp start_time_;
};
UDPPort::AddressResolver::AddressResolver(
@@ -508,8 +510,9 @@
return;
}
- request_manager_.Send(
- new StunBindingRequest(this, stun_addr, TimeMillis()));
+ request_manager_.Send(std::make_unique<StunBindingRequest>(
+ this, stun_addr,
+ Connection::AlignTime(env().clock().CurrentTime())));
});
}
@@ -531,9 +534,10 @@
}
void UDPPort::OnStunBindingRequestSucceeded(
- int rtt_ms,
+ TimeDelta rtt,
const SocketAddress& stun_server_addr,
const SocketAddress& stun_reflected_addr) {
+ int rtt_ms = rtt.ms();
RTC_DCHECK(stats_.stun_binding_responses_received <
stats_.stun_binding_requests_sent);
stats_.stun_binding_responses_received++;
diff --git a/p2p/base/stun_port.h b/p2p/base/stun_port.h
index 1dc4da7..171e232 100644
--- a/p2p/base/stun_port.h
+++ b/p2p/base/stun_port.h
@@ -24,6 +24,7 @@
#include "api/candidate.h"
#include "api/field_trials_view.h"
#include "api/packet_socket_factory.h"
+#include "api/units/time_delta.h"
#include "p2p/base/connection.h"
#include "p2p/base/port.h"
#include "p2p/base/port_interface.h"
@@ -39,10 +40,9 @@
namespace webrtc {
-// Lifetime chosen for STUN ports on low-cost networks.
-static const int INFINITE_LIFETIME = -1;
// Lifetime for STUN ports on high-cost networks: 2 minutes
-static const int HIGH_COST_PORT_KEEPALIVE_LIFETIME = 2 * 60 * 1000;
+inline constexpr TimeDelta kHighCostPortKeepaliveLifetime =
+ TimeDelta::Seconds(2 * 60);
// Communicates using the address on the outside of a NAT.
class RTC_EXPORT UDPPort : public Port {
@@ -105,11 +105,13 @@
void GetStunStats(std::optional<StunStats>* stats) override;
void set_stun_keepalive_delay(const std::optional<int>& delay);
- int stun_keepalive_delay() const { return stun_keepalive_delay_; }
+ TimeDelta stun_keepalive_delay() const {
+ return TimeDelta::Millis(stun_keepalive_delay_);
+ }
// Visible for testing.
- int stun_keepalive_lifetime() const { return stun_keepalive_lifetime_; }
- void set_stun_keepalive_lifetime(int lifetime) {
+ TimeDelta stun_keepalive_lifetime() const { return stun_keepalive_lifetime_; }
+ void set_stun_keepalive_lifetime(TimeDelta lifetime) {
stun_keepalive_lifetime_ = lifetime;
}
@@ -201,7 +203,7 @@
void SendStunBindingRequest(const SocketAddress& stun_addr);
// Below methods handles binding request responses.
- void OnStunBindingRequestSucceeded(int rtt_ms,
+ void OnStunBindingRequestSucceeded(TimeDelta rtt,
const SocketAddress& stun_server_addr,
const SocketAddress& stun_reflected_addr);
void OnStunBindingOrResolveRequestFailed(
@@ -220,11 +222,10 @@
// If this is a low-cost network, it will keep on sending STUN binding
// requests indefinitely to keep the NAT binding alive. Otherwise, stop
- // sending STUN binding requests after HIGH_COST_PORT_KEEPALIVE_LIFETIME.
- int GetStunKeepaliveLifetime() {
- return (network_cost() >= kNetworkCostHigh)
- ? HIGH_COST_PORT_KEEPALIVE_LIFETIME
- : INFINITE_LIFETIME;
+ // sending STUN binding requests after kHighCostPortKeepaliveLifetime.
+ TimeDelta GetStunKeepaliveLifetime() {
+ return (network_cost() >= kNetworkCostHigh) ? kHighCostPortKeepaliveLifetime
+ : TimeDelta::PlusInfinity();
}
ServerAddresses server_addresses_;
@@ -237,7 +238,7 @@
std::unique_ptr<AddressResolver> resolver_;
bool ready_;
int stun_keepalive_delay_;
- int stun_keepalive_lifetime_ = INFINITE_LIFETIME;
+ TimeDelta stun_keepalive_lifetime_ = TimeDelta::PlusInfinity();
DiffServCodePoint dscp_;
StunStats stats_;
diff --git a/p2p/base/stun_port_unittest.cc b/p2p/base/stun_port_unittest.cc
index 6fa8096..99e2e86 100644
--- a/p2p/base/stun_port_unittest.cc
+++ b/p2p/base/stun_port_unittest.cc
@@ -98,9 +98,6 @@
// stun prio = 100 (srflx) << 24 | 40 (IPv6) << 8 | 256 - 1 (component)
constexpr uint32_t kIPv6StunCandidatePriority =
(100 << 24) | (40 << 8) | (256 - 1);
-constexpr int kInfiniteLifetime = -1;
-constexpr int kHighCostPortKeepaliveLifetimeMs = 2 * 60 * 1000;
-
constexpr uint64_t kTiebreakerDefault = 44444;
struct IPAddressTypeTestConfig {
@@ -156,8 +153,7 @@
nat_server_(CreateNatServer(nat_server_address, webrtc::NAT_OPEN_CONE)),
done_(false),
error_(false),
- stun_keepalive_delay_(1),
- stun_keepalive_lifetime_(-1) {
+ stun_keepalive_delay_(1) {
network_ = MakeNetwork(address);
RTC_CHECK(address.family() == nat_server_address.family());
for (const auto& addr : stun_server_addresses) {
@@ -210,10 +206,10 @@
0, 0, stun_servers, std::nullopt);
stun_port_->SetIceTiebreaker(kTiebreakerDefault);
stun_port_->set_stun_keepalive_delay(stun_keepalive_delay_);
- // If `stun_keepalive_lifetime_` is negative, let the stun port
- // choose its lifetime from the network type.
- if (stun_keepalive_lifetime_ >= 0) {
- stun_port_->set_stun_keepalive_lifetime(stun_keepalive_lifetime_);
+ // If `stun_keepalive_lifetime_` is not set, let the stun port choose its
+ // lifetime from the network type.
+ if (stun_keepalive_lifetime_.has_value()) {
+ stun_port_->set_stun_keepalive_lifetime(*stun_keepalive_lifetime_);
}
stun_port_->SignalPortComplete.connect(this,
&StunPortTestBase::OnPortComplete);
@@ -292,7 +288,7 @@
}
void SetKeepaliveDelay(int delay) { stun_keepalive_delay_ = delay; }
- void SetKeepaliveLifetime(int lifetime) {
+ void SetKeepaliveLifetime(TimeDelta lifetime) {
stun_keepalive_lifetime_ = lifetime;
}
@@ -324,7 +320,7 @@
bool done_;
bool error_;
int stun_keepalive_delay_;
- int stun_keepalive_lifetime_;
+ std::optional<TimeDelta> stun_keepalive_lifetime_;
protected:
webrtc::IceCandidateErrorEvent error_event_;
@@ -653,43 +649,41 @@
// type on a STUN port. Also test that it will be updated if the network type
// changes.
TEST_F(StunPortTest, TestStunPortGetStunKeepaliveLifetime) {
- // Lifetime for the default (unknown) network type is `kInfiniteLifetime`.
+ // Lifetime for the default (unknown) network type is infinite.
CreateStunPort(kStunServerAddr1);
- EXPECT_EQ(kInfiniteLifetime, port()->stun_keepalive_lifetime());
- // Lifetime for the cellular network is `kHighCostPortKeepaliveLifetimeMs`
+ EXPECT_EQ(port()->stun_keepalive_lifetime(), TimeDelta::PlusInfinity());
+ // Lifetime for the cellular network is `kHighCostPortKeepaliveLifetime`
SetNetworkType(webrtc::ADAPTER_TYPE_CELLULAR);
- EXPECT_EQ(kHighCostPortKeepaliveLifetimeMs,
- port()->stun_keepalive_lifetime());
+ EXPECT_EQ(port()->stun_keepalive_lifetime(), kHighCostPortKeepaliveLifetime);
- // Lifetime for the wifi network is `kInfiniteLifetime`.
+ // Lifetime for the wifi network is infinite.
SetNetworkType(webrtc::ADAPTER_TYPE_WIFI);
CreateStunPort(kStunServerAddr2);
- EXPECT_EQ(kInfiniteLifetime, port()->stun_keepalive_lifetime());
+ EXPECT_EQ(port()->stun_keepalive_lifetime(), TimeDelta::PlusInfinity());
}
// Test that the stun_keepalive_lifetime is set correctly based on the network
// type on a shared STUN port (UDPPort). Also test that it will be updated
// if the network type changes.
TEST_F(StunPortTest, TestUdpPortGetStunKeepaliveLifetime) {
- // Lifetime for the default (unknown) network type is `kInfiniteLifetime`.
+ // Lifetime for the default (unknown) network type is infinite.
CreateSharedUdpPort(kStunServerAddr1, nullptr);
- EXPECT_EQ(kInfiniteLifetime, port()->stun_keepalive_lifetime());
- // Lifetime for the cellular network is `kHighCostPortKeepaliveLifetimeMs`.
+ EXPECT_EQ(port()->stun_keepalive_lifetime(), TimeDelta::PlusInfinity());
+ // Lifetime for the cellular network is `kHighCostPortKeepaliveLifetime`.
SetNetworkType(webrtc::ADAPTER_TYPE_CELLULAR);
- EXPECT_EQ(kHighCostPortKeepaliveLifetimeMs,
- port()->stun_keepalive_lifetime());
+ EXPECT_EQ(port()->stun_keepalive_lifetime(), kHighCostPortKeepaliveLifetime);
- // Lifetime for the wifi network type is `kInfiniteLifetime`.
+ // Lifetime for the wifi network type is infinite.
SetNetworkType(webrtc::ADAPTER_TYPE_WIFI);
CreateSharedUdpPort(kStunServerAddr2, nullptr);
- EXPECT_EQ(kInfiniteLifetime, port()->stun_keepalive_lifetime());
+ EXPECT_EQ(port()->stun_keepalive_lifetime(), TimeDelta::PlusInfinity());
}
// Test that STUN binding requests will be stopped shortly if the keep-alive
// lifetime is short.
TEST_F(StunPortTest, TestStunBindingRequestShortLifetime) {
SetKeepaliveDelay(101);
- SetKeepaliveLifetime(100);
+ SetKeepaliveLifetime(TimeDelta::Millis(100));
CreateStunPort(kStunServerAddr1);
PrepareAddress();
EXPECT_THAT(
diff --git a/p2p/base/stun_request.cc b/p2p/base/stun_request.cc
index 1a31df5..874da79 100644
--- a/p2p/base/stun_request.cc
+++ b/p2p/base/stun_request.cc
@@ -19,18 +19,18 @@
#include <utility>
#include <vector>
-#include "absl/memory/memory.h"
#include "api/array_view.h"
+#include "api/environment/environment.h"
#include "api/sequence_checker.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "api/task_queue/task_queue_base.h"
#include "api/transport/stun.h"
#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
#include "rtc_base/byte_buffer.h"
#include "rtc_base/checks.h"
#include "rtc_base/logging.h"
#include "rtc_base/string_encode.h"
-#include "rtc_base/time_utils.h" // For TimeMillis
namespace webrtc {
@@ -58,11 +58,8 @@
StunRequestManager::~StunRequestManager() = default;
-void StunRequestManager::Send(StunRequest* request) {
- SendDelayed(request, 0);
-}
-
-void StunRequestManager::SendDelayed(StunRequest* request, int delay) {
+void StunRequestManager::Send(std::unique_ptr<StunRequest> request,
+ TimeDelta delay) {
RTC_DCHECK_RUN_ON(thread_);
RTC_DCHECK_EQ(this, request->manager());
RTC_DCHECK(!request->AuthenticationRequired() ||
@@ -70,9 +67,9 @@
StunMessage::IntegrityStatus::kNotSet)
<< "Sending request w/o integrity!";
auto [iter, was_inserted] =
- requests_.emplace(request->id(), absl::WrapUnique(request));
+ requests_.emplace(request->id(), std::move(request));
RTC_DCHECK(was_inserted);
- request->Send(TimeDelta::Millis(delay));
+ iter->second->Send(delay);
}
void StunRequestManager::FlushForTest(int msg_type) {
@@ -233,20 +230,23 @@
send_packet_(data, size, request);
}
-StunRequest::StunRequest(StunRequestManager& manager)
- : manager_(manager),
- msg_(new StunMessage(STUN_INVALID_MESSAGE_TYPE)),
- tstamp_(0),
+StunRequest::StunRequest(const Environment& env, StunRequestManager& manager)
+ : env_(env),
+ manager_(manager),
+ msg_(std::make_unique<StunMessage>(STUN_INVALID_MESSAGE_TYPE)),
+ tstamp_(Timestamp::Zero()),
count_(0),
timeout_(false) {
RTC_DCHECK_RUN_ON(network_thread());
}
-StunRequest::StunRequest(StunRequestManager& manager,
+StunRequest::StunRequest(const Environment& env,
+ StunRequestManager& manager,
std::unique_ptr<StunMessage> message)
- : manager_(manager),
+ : env_(env),
+ manager_(manager),
msg_(std::move(message)),
- tstamp_(0),
+ tstamp_(Timestamp::Zero()),
count_(0),
timeout_(false) {
RTC_DCHECK_RUN_ON(network_thread());
@@ -264,9 +264,9 @@
return msg_.get();
}
-int StunRequest::Elapsed() const {
+TimeDelta StunRequest::Elapsed() const {
RTC_DCHECK_RUN_ON(network_thread());
- return static_cast<int>(TimeMillis() - tstamp_);
+ return env_.clock().CurrentTime() - tstamp_;
}
void StunRequest::SendInternal() {
@@ -277,7 +277,7 @@
return;
}
- tstamp_ = TimeMillis();
+ tstamp_ = env_.clock().CurrentTime();
ByteBufferWriter buf;
msg_->Write(&buf);
diff --git a/p2p/base/stun_request.h b/p2p/base/stun_request.h
index 8d6a7f5..b7f2025 100644
--- a/p2p/base/stun_request.h
+++ b/p2p/base/stun_request.h
@@ -18,10 +18,12 @@
#include <memory>
#include <string>
+#include "api/environment/environment.h"
#include "api/task_queue/pending_task_safety_flag.h"
#include "api/task_queue/task_queue_base.h"
#include "api/transport/stun.h"
#include "api/units/time_delta.h"
+#include "api/units/timestamp.h"
#include "rtc_base/thread_annotations.h"
namespace webrtc {
@@ -45,8 +47,8 @@
~StunRequestManager();
// Starts sending the given request (perhaps after a delay).
- void Send(StunRequest* request);
- void SendDelayed(StunRequest* request, int delay);
+ void Send(std::unique_ptr<StunRequest> request,
+ TimeDelta delay = TimeDelta::Zero());
// If `msg_type` is kAllRequestsForTest, sends all pending requests right
// away. Otherwise, sends those that have a matching type right away. Only for
@@ -90,8 +92,9 @@
// constructed beforehand or built on demand.
class StunRequest {
public:
- explicit StunRequest(StunRequestManager& manager);
- StunRequest(StunRequestManager& manager,
+ StunRequest(const Environment& env, StunRequestManager& manager);
+ StunRequest(const Environment& env,
+ StunRequestManager& manager,
std::unique_ptr<StunMessage> message);
virtual ~StunRequest();
@@ -112,8 +115,8 @@
// Returns a const pointer to `msg_`.
const StunMessage* msg() const;
- // Time elapsed since last send (in ms)
- int Elapsed() const;
+ // Time elapsed since last send.
+ TimeDelta Elapsed() const;
// Add method to explitly allow requests w/o password.
// - STUN_BINDINGs from StunPort to a stun server
@@ -124,6 +127,8 @@
protected:
friend class StunRequestManager;
+ const Environment& env() { return env_; }
+
// Called by StunRequestManager.
void Send(TimeDelta delay);
@@ -152,9 +157,10 @@
// specified timeout.
void SendDelayed(TimeDelta delay);
+ const Environment env_;
StunRequestManager& manager_;
const std::unique_ptr<StunMessage> msg_;
- int64_t tstamp_ RTC_GUARDED_BY(network_thread());
+ Timestamp tstamp_ RTC_GUARDED_BY(network_thread());
int count_ RTC_GUARDED_BY(network_thread());
bool timeout_ RTC_GUARDED_BY(network_thread());
ScopedTaskSafety task_safety_{
diff --git a/p2p/base/stun_request_unittest.cc b/p2p/base/stun_request_unittest.cc
index ec3bb74..2055a16 100644
--- a/p2p/base/stun_request_unittest.cc
+++ b/p2p/base/stun_request_unittest.cc
@@ -13,8 +13,11 @@
#include <cstddef>
#include <cstdint>
#include <memory>
+#include <string>
+#include <utility>
#include <vector>
+#include "api/environment/environment.h"
#include "api/test/rtc_error_matchers.h"
#include "api/transport/stun.h"
#include "api/units/time_delta.h"
@@ -23,6 +26,7 @@
#include "rtc_base/logging.h"
#include "rtc_base/thread.h"
#include "rtc_base/time_utils.h"
+#include "test/create_test_environment.h"
#include "test/gmock.h"
#include "test/gtest.h"
#include "test/wait_until.h"
@@ -47,10 +51,13 @@
}
} // namespace
+class StunRequestThunker;
+
class StunRequestTest : public ::testing::Test {
public:
StunRequestTest()
- : manager_(Thread::Current(),
+ : env_(CreateTestEnvironment()),
+ manager_(Thread::Current(),
[this](const void* data, size_t size, StunRequest* request) {
OnSendPacket(data, size, request);
}),
@@ -60,6 +67,8 @@
failure_(false),
timeout_(false) {}
+ std::unique_ptr<StunRequestThunker> CreateStunRequest();
+
void OnSendPacket(const void* data, size_t size, StunRequest* req) {
request_count_++;
}
@@ -76,6 +85,7 @@
protected:
AutoThread main_thread_;
+ const Environment env_;
StunRequestManager manager_;
int request_count_;
StunMessage* response_;
@@ -87,8 +97,10 @@
// Forwards results to the test class.
class StunRequestThunker : public StunRequest {
public:
- StunRequestThunker(StunRequestManager& manager, StunRequestTest* test)
- : StunRequest(manager, CreateStunMessage(STUN_BINDING_REQUEST)),
+ StunRequestThunker(const Environment& env,
+ StunRequestManager& manager,
+ StunRequestTest* test)
+ : StunRequest(env, manager, CreateStunMessage(STUN_BINDING_REQUEST)),
test_(test) {
SetAuthenticationRequired(false);
}
@@ -107,12 +119,16 @@
StunRequestTest* test_;
};
+std::unique_ptr<StunRequestThunker> StunRequestTest::CreateStunRequest() {
+ return std::make_unique<StunRequestThunker>(env_, manager_, this);
+}
+
// Test handling of a normal binding response.
TEST_F(StunRequestTest, TestSuccess) {
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_RESPONSE);
- manager_.Send(request);
+ manager_.Send(std::move(request));
EXPECT_TRUE(manager_.CheckResponse(res.get()));
EXPECT_TRUE(response_ == res.get());
@@ -123,10 +139,10 @@
// Test handling of an error binding response.
TEST_F(StunRequestTest, TestError) {
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_ERROR_RESPONSE);
- manager_.Send(request);
+ manager_.Send(std::move(request));
EXPECT_TRUE(manager_.CheckResponse(res.get()));
EXPECT_TRUE(response_ == res.get());
@@ -137,10 +153,10 @@
// Test handling of a binding response with the wrong transaction id.
TEST_F(StunRequestTest, TestUnexpected) {
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res = CreateStunMessage(STUN_BINDING_RESPONSE);
- manager_.Send(request);
+ manager_.Send(std::move(request));
EXPECT_FALSE(manager_.CheckResponse(res.get()));
EXPECT_TRUE(response_ == nullptr);
@@ -152,12 +168,12 @@
// Test that requests are sent at the right times.
TEST_F(StunRequestTest, TestBackoff) {
ScopedFakeClock fake_clock;
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_RESPONSE);
int64_t start = TimeMillis();
- manager_.Send(request);
+ manager_.Send(std::move(request));
for (int i = 0; i < 9; ++i) {
EXPECT_THAT(WaitUntil([&] { return request_count_; }, Ne(i),
{.timeout = TimeDelta::Millis(STUN_TOTAL_TIMEOUT),
@@ -179,11 +195,11 @@
// Test that we timeout properly if no response is received.
TEST_F(StunRequestTest, TestTimeout) {
ScopedFakeClock fake_clock;
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_RESPONSE);
- manager_.Send(request);
+ manager_.Send(std::move(request));
SIMULATED_WAIT(false, STUN_TOTAL_TIMEOUT, fake_clock);
EXPECT_FALSE(manager_.CheckResponse(res.get()));
@@ -196,11 +212,12 @@
// Regression test for specific crash where we receive a response with the
// same id as a request that doesn't have an underlying StunMessage yet.
TEST_F(StunRequestTest, TestNoEmptyRequest) {
- StunRequestThunker* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
+ std::string request_id = request->id();
- manager_.SendDelayed(request, 100);
+ manager_.Send(std::move(request), /*delay=*/TimeDelta::Millis(100));
- StunMessage dummy_req(0, request->id());
+ StunMessage dummy_req(0, request_id);
std::unique_ptr<StunMessage> res =
CreateStunMessage(STUN_BINDING_RESPONSE, &dummy_req);
@@ -216,11 +233,11 @@
// which is not recognized, the transaction should be considered a failure and
// the response should be ignored.
TEST_F(StunRequestTest, TestUnrecognizedComprehensionRequiredAttribute) {
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_ERROR_RESPONSE);
- manager_.Send(request);
+ manager_.Send(std::move(request));
res->AddAttribute(StunAttribute::CreateUInt32(0x7777));
EXPECT_FALSE(manager_.CheckResponse(res.get()));
@@ -243,10 +260,10 @@
};
TEST_F(StunRequestReentranceTest, TestSuccess) {
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_RESPONSE);
- manager_.Send(request);
+ manager_.Send(std::move(request));
EXPECT_TRUE(manager_.CheckResponse(res.get()));
EXPECT_TRUE(response_ == res.get());
@@ -256,10 +273,10 @@
}
TEST_F(StunRequestReentranceTest, TestError) {
- auto* request = new StunRequestThunker(manager_, this);
+ std::unique_ptr<StunRequestThunker> request = CreateStunRequest();
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_ERROR_RESPONSE);
- manager_.Send(request);
+ manager_.Send(std::move(request));
EXPECT_TRUE(manager_.CheckResponse(res.get()));
EXPECT_TRUE(response_ == res.get());
diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
index cff27b0..fe18fa6 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -21,6 +21,7 @@
#include <vector>
#include "absl/algorithm/container.h"
+#include "absl/memory/memory.h"
#include "absl/strings/match.h"
#include "absl/strings/string_view.h"
#include "api/array_view.h"
@@ -51,6 +52,7 @@
#include "rtc_base/network.h"
#include "rtc_base/network/received_packet.h"
#include "rtc_base/network/sent_packet.h"
+#include "rtc_base/platform_thread_types.h"
#include "rtc_base/socket.h"
#include "rtc_base/socket_address.h"
#include "rtc_base/ssl_certificate.h"
@@ -1162,7 +1164,7 @@
}
void TurnPort::SendRequest(StunRequest* req, int delay) {
- request_manager_.SendDelayed(req, delay);
+ request_manager_.Send(absl::WrapUnique(req), TimeDelta::Millis(delay));
}
void TurnPort::AddRequestAuthInfo(StunMessage* msg) {
@@ -1338,7 +1340,8 @@
}
TurnAllocateRequest::TurnAllocateRequest(TurnPort* port)
- : StunRequest(port->request_manager(),
+ : StunRequest(port->env(),
+ port->request_manager(),
std::make_unique<TurnMessage>(TURN_ALLOCATE_REQUEST)),
port_(port) {
StunMessage* message = mutable_msg();
@@ -1536,7 +1539,8 @@
}
TurnRefreshRequest::TurnRefreshRequest(TurnPort* port, int lifetime /*= -1*/)
- : StunRequest(port->request_manager(),
+ : StunRequest(port->env(),
+ port->request_manager(),
std::make_unique<TurnMessage>(TURN_REFRESH_REQUEST)),
port_(port) {
StunMessage* message = mutable_msg();
@@ -1623,6 +1627,7 @@
TurnEntry* entry,
const SocketAddress& ext_addr)
: StunRequest(
+ port->env(),
port->request_manager(),
std::make_unique<TurnMessage>(TURN_CREATE_PERMISSION_REQUEST)),
port_(port),
@@ -1692,7 +1697,8 @@
TurnEntry* entry,
uint16_t channel_id,
const SocketAddress& ext_addr)
- : StunRequest(port->request_manager(),
+ : StunRequest(port->env(),
+ port->request_manager(),
std::make_unique<TurnMessage>(TURN_CHANNEL_BIND_REQUEST)),
port_(port),
entry_(entry),
diff --git a/p2p/client/basic_port_allocator_unittest.cc b/p2p/client/basic_port_allocator_unittest.cc
index 2d543ee..48eaff1 100644
--- a/p2p/client/basic_port_allocator_unittest.cc
+++ b/p2p/client/basic_port_allocator_unittest.cc
@@ -145,7 +145,7 @@
port->GetProtocol() == webrtc::PROTO_UDP)) {
EXPECT_EQ(
static_cast<const webrtc::UDPPort*>(port)->stun_keepalive_delay(),
- expected);
+ webrtc::TimeDelta::Millis(expected));
}
}
}
