admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[StunRequest] Remove Construct and Prepare methods.

Browse Source 
Construction in StunRequest and derived classes now happens in
the ctor.

Bug: none
Change-Id: I803f6fd2b6d0ac1d9426304d1e1de10dec855eed
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/264942
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37126}
This commit is contained in:
Tommi 2022-06-03 14:37:31 +02:00 committed by WebRTC LUCI CQ
parent 1062c15c2d
commit 159f313649
5 changed files with 22 additions and 70 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
p2p/base/stun_port.cc
View File

@ -40,17 +40,14 @@ class StunBindingRequest : public StunRequest {
StunBindingRequest(UDPPort* port,
const rtc::SocketAddress& addr,
int64_t start_time)
: StunRequest(port->request_manager()),
: StunRequest(port->request_manager(),
std::make_unique<StunMessage>(STUN_BINDING_REQUEST)),
port_(port),
server_addr_(addr),
start_time_(start_time) {}
const rtc::SocketAddress& server_addr() const { return server_addr_; }
void Prepare(StunMessage* message) override {
message->SetType(STUN_BINDING_REQUEST);
}
void OnResponse(StunMessage* response) override {
const StunAddressAttribute* addr_attr =
response->GetAddress(STUN_ATTR_MAPPED_ADDRESS);

13
p2p/base/stun_request.cc
View File

@ -57,7 +57,6 @@ void StunRequestManager::Send(StunRequest* request) {
void StunRequestManager::SendDelayed(StunRequest* request, int delay) {
RTC_DCHECK_RUN_ON(thread_);
RTC_DCHECK_EQ(this, request->manager());
request->Construct();
auto [iter, was_inserted] =
requests_.emplace(request->id(), absl::WrapUnique(request));
RTC_DCHECK(was_inserted);
@ -212,18 +211,6 @@ StunRequest::~StunRequest() {
manager_.network_thread()->Clear(this);
}
void StunRequest::Construct() {
// TODO(tommi): The implementation assumes that Construct() is only called
// once (see `StunRequestManager::SendDelayed` below). However, these steps to
// construct a request object are odd to have at this level (virtual method
// called from the parent class, _after_ construction) and also triggered
// from a different class... via a "Send" method.
// Remove `Prepare`, `Construct` and make construction of the message objects
// separate from the StunRequest and StunRequestManager classes.
Prepare(msg_.get());
RTC_DCHECK_NE(msg_->type(), 0);
}
int StunRequest::type() {
RTC_DCHECK(msg_ != NULL);
return msg_->type();

9
p2p/base/stun_request.h
View File

@ -114,14 +114,7 @@ class StunRequest : public rtc::MessageHandler {
protected:
friend class StunRequestManager;
// Causes our wrapped StunMessage to be Prepared.
// Only called by StunRequestManager.
// TODO(tommi): get rid of this (see cc file).
void Construct();
// Fills in a request object to be sent. Note that request's transaction ID
// will already be set and cannot be changed.
virtual void Prepare(StunMessage* message) {}
StunMessage* mutable_msg() { return msg_.get(); }
// Called when the message receives a response or times out.
virtual void OnResponse(StunMessage* response) {}

27
p2p/base/stun_request_unittest.cc
View File

@ -77,16 +77,9 @@ class StunRequestTest : public ::testing::Test {
// Forwards results to the test class.
class StunRequestThunker : public StunRequest {
public:
StunRequestThunker(StunRequestManager& manager,
StunMessageType message_type,
StunRequestTest* test)
: StunRequest(manager, CreateStunMessage(message_type)), test_(test) {
Construct(); // Triggers a call to `Prepare()` which sets the type.
}
StunRequestThunker(StunRequestManager& manager, StunRequestTest* test)
: StunRequest(manager), test_(test) {
Construct(); // Triggers a call to `Prepare()` which sets the type.
}
: StunRequest(manager, CreateStunMessage(STUN_BINDING_REQUEST)),
test_(test) {}
std::unique_ptr<StunMessage> CreateResponseMessage(StunMessageType type) {
return CreateStunMessage(type, msg());
@ -99,16 +92,12 @@ class StunRequestThunker : public StunRequest {
}
virtual void OnTimeout() { test_->OnTimeout(); }
virtual void Prepare(StunMessage* message) {
message->SetType(STUN_BINDING_REQUEST);
}
StunRequestTest* test_;
};
// Test handling of a normal binding response.
TEST_F(StunRequestTest, TestSuccess) {
auto* request = new StunRequestThunker(manager_, STUN_BINDING_REQUEST, this);
auto* request = new StunRequestThunker(manager_, this);
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_RESPONSE);
manager_.Send(request);
@ -122,7 +111,7 @@ TEST_F(StunRequestTest, TestSuccess) {
// Test handling of an error binding response.
TEST_F(StunRequestTest, TestError) {
auto* request = new StunRequestThunker(manager_, STUN_BINDING_REQUEST, this);
auto* request = new StunRequestThunker(manager_, this);
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_ERROR_RESPONSE);
manager_.Send(request);
@ -136,7 +125,7 @@ TEST_F(StunRequestTest, TestError) {
// Test handling of a binding response with the wrong transaction id.
TEST_F(StunRequestTest, TestUnexpected) {
auto* request = new StunRequestThunker(manager_, STUN_BINDING_REQUEST, this);
auto* request = new StunRequestThunker(manager_, this);
std::unique_ptr<StunMessage> res = CreateStunMessage(STUN_BINDING_RESPONSE);
manager_.Send(request);
@ -151,7 +140,7 @@ TEST_F(StunRequestTest, TestUnexpected) {
// Test that requests are sent at the right times.
TEST_F(StunRequestTest, TestBackoff) {
rtc::ScopedFakeClock fake_clock;
auto* request = new StunRequestThunker(manager_, STUN_BINDING_REQUEST, this);
auto* request = new StunRequestThunker(manager_, this);
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_RESPONSE);
@ -176,7 +165,7 @@ TEST_F(StunRequestTest, TestBackoff) {
// Test that we timeout properly if no response is received.
TEST_F(StunRequestTest, TestTimeout) {
rtc::ScopedFakeClock fake_clock;
auto* request = new StunRequestThunker(manager_, STUN_BINDING_REQUEST, this);
auto* request = new StunRequestThunker(manager_, this);
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_RESPONSE);
@ -213,7 +202,7 @@ TEST_F(StunRequestTest, TestNoEmptyRequest) {
// 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_, STUN_BINDING_REQUEST, this);
auto* request = new StunRequestThunker(manager_, this);
std::unique_ptr<StunMessage> res =
request->CreateResponseMessage(STUN_BINDING_ERROR_RESPONSE);

36
p2p/base/turn_port.cc
View File

@ -73,7 +73,6 @@ static int GetRelayPreference(cricket::ProtocolType proto) {
class TurnAllocateRequest : public StunRequest {
public:
explicit TurnAllocateRequest(TurnPort* port);
void Prepare(StunMessage* message) override;
void OnSent() override;
void OnResponse(StunMessage* response) override;
void OnErrorResponse(StunMessage* response) override;
@ -90,17 +89,14 @@ class TurnAllocateRequest : public StunRequest {
class TurnRefreshRequest : public StunRequest {
public:
explicit TurnRefreshRequest(TurnPort* port);
void Prepare(StunMessage* message) override;
explicit TurnRefreshRequest(TurnPort* port, int lifetime = -1);
void OnSent() override;
void OnResponse(StunMessage* response) override;
void OnErrorResponse(StunMessage* response) override;
void OnTimeout() override;
void set_lifetime(int lifetime) { lifetime_ = lifetime; }
private:
TurnPort* port_;
int lifetime_;
};
class TurnCreatePermissionRequest : public StunRequest,
@ -110,7 +106,6 @@ class TurnCreatePermissionRequest : public StunRequest,
TurnEntry* entry,
const rtc::SocketAddress& ext_addr,
const std::string& remote_ufrag);
void Prepare(StunMessage* message) override;
void OnSent() override;
void OnResponse(StunMessage* response) override;
void OnErrorResponse(StunMessage* response) override;
@ -131,7 +126,6 @@ class TurnChannelBindRequest : public StunRequest, public sigslot::has_slots<> {
TurnEntry* entry,
int channel_id,
const rtc::SocketAddress& ext_addr);
void Prepare(StunMessage* message) override;
void OnSent() override;
void OnResponse(StunMessage* response) override;
void OnErrorResponse(StunMessage* response) override;
@ -933,8 +927,7 @@ void TurnPort::Release() {
request_manager_.Clear();
// Send refresh with lifetime 0.
TurnRefreshRequest* req = new TurnRefreshRequest(this);
req->set_lifetime(0);
TurnRefreshRequest* req = new TurnRefreshRequest(this, 0);
SendRequest(req, 0);
state_ = STATE_RECEIVEONLY;
@ -1376,9 +1369,8 @@ void TurnPort::MaybeAddTurnLoggingId(StunMessage* msg) {
TurnAllocateRequest::TurnAllocateRequest(TurnPort* port)
: StunRequest(port->request_manager(),
std::make_unique<TurnMessage>(TURN_ALLOCATE_REQUEST)),
port_(port) {}
void TurnAllocateRequest::Prepare(StunMessage* message) {
port_(port) {
StunMessage* message = mutable_msg();
// Create the request as indicated in RFC 5766, Section 6.1.
RTC_DCHECK_EQ(message->type(), TURN_ALLOCATE_REQUEST);
auto transport_attr =
@ -1563,19 +1555,17 @@ void TurnAllocateRequest::OnTryAlternate(StunMessage* response, int code) {
TurnPort::MSG_TRY_ALTERNATE_SERVER);
}
TurnRefreshRequest::TurnRefreshRequest(TurnPort* port)
TurnRefreshRequest::TurnRefreshRequest(TurnPort* port, int lifetime /*= -1*/)
: StunRequest(port->request_manager(),
std::make_unique<TurnMessage>(TURN_REFRESH_REQUEST)),
port_(port),
lifetime_(-1) {}
void TurnRefreshRequest::Prepare(StunMessage* message) {
port_(port) {
StunMessage* message = mutable_msg();
// Create the request as indicated in RFC 5766, Section 7.1.
// No attributes need to be included.
RTC_DCHECK_EQ(message->type(), TURN_REFRESH_REQUEST);
if (lifetime_ > -1) {
if (lifetime > -1) {
message->AddAttribute(
std::make_unique<StunUInt32Attribute>(STUN_ATTR_LIFETIME, lifetime_));
std::make_unique<StunUInt32Attribute>(STUN_ATTR_LIFETIME, lifetime));
}
port_->AddRequestAuthInfo(message);
@ -1657,9 +1647,7 @@ TurnCreatePermissionRequest::TurnCreatePermissionRequest(
remote_ufrag_(remote_ufrag) {
entry_->SignalDestroyed.connect(
this, &TurnCreatePermissionRequest::OnEntryDestroyed);
}
void TurnCreatePermissionRequest::Prepare(StunMessage* message) {
StunMessage* message = mutable_msg();
// Create the request as indicated in RFC5766, Section 9.1.
message->SetType(TURN_CREATE_PERMISSION_REQUEST);
message->AddAttribute(std::make_unique<StunXorAddressAttribute>(
@ -1731,9 +1719,7 @@ TurnChannelBindRequest::TurnChannelBindRequest(
ext_addr_(ext_addr) {
entry_->SignalDestroyed.connect(this,
&TurnChannelBindRequest::OnEntryDestroyed);
}
void TurnChannelBindRequest::Prepare(StunMessage* message) {
StunMessage* message = mutable_msg();
// Create the request as indicated in RFC5766, Section 11.1.
RTC_DCHECK_EQ(message->type(), TURN_CHANNEL_BIND_REQUEST);
message->AddAttribute(std::make_unique<StunUInt32Attribute>(