Adopt StreamId in SctpDataChannelControllerInterface
Bug: webrtc:11547
Change-Id: Iea2d706228b5a533eb7fae84613462165d7c9b54
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298300
Auto-Submit: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Danil Chapovalov <danilchap@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39618}
diff --git a/pc/BUILD.gn b/pc/BUILD.gn
index 812df05..7e07843 100644
--- a/pc/BUILD.gn
+++ b/pc/BUILD.gn
@@ -517,7 +517,6 @@
deps = [
"../api:libjingle_peerconnection_api",
"../api:priority",
- "../api:sequence_checker",
"../api/transport:datagram_transport_interface",
"../media:media_channel",
"../media:rtc_data_sctp_transport_internal",
@@ -527,7 +526,6 @@
"../rtc_base:copy_on_write_buffer",
"../rtc_base:logging",
"../rtc_base:ssl",
- "../rtc_base/system:no_unique_address",
]
absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ]
}
diff --git a/pc/data_channel_controller.cc b/pc/data_channel_controller.cc
index 23e7a14..e307418 100644
--- a/pc/data_channel_controller.cc
+++ b/pc/data_channel_controller.cc
@@ -33,7 +33,7 @@
return has_used_data_channels_;
}
-bool DataChannelController::SendData(int sid,
+bool DataChannelController::SendData(StreamId sid,
const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result) {
@@ -43,21 +43,21 @@
return false;
}
-void DataChannelController::AddSctpDataStream(int sid) {
+void DataChannelController::AddSctpDataStream(StreamId sid) {
if (data_channel_transport()) {
network_thread()->BlockingCall([this, sid] {
if (data_channel_transport()) {
- data_channel_transport()->OpenChannel(sid);
+ data_channel_transport()->OpenChannel(sid.stream_id_int());
}
});
}
}
-void DataChannelController::RemoveSctpDataStream(int sid) {
+void DataChannelController::RemoveSctpDataStream(StreamId sid) {
if (data_channel_transport()) {
network_thread()->BlockingCall([this, sid] {
if (data_channel_transport()) {
- data_channel_transport()->CloseChannel(sid);
+ data_channel_transport()->CloseChannel(sid.stream_id_int());
}
});
}
@@ -375,7 +375,7 @@
}
bool DataChannelController::DataChannelSendData(
- int sid,
+ StreamId sid,
const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result) {
@@ -386,7 +386,8 @@
RTC_DCHECK(data_channel_transport());
RTCError error = network_thread()->BlockingCall([this, sid, params, payload] {
- return data_channel_transport()->SendData(sid, params, payload);
+ return data_channel_transport()->SendData(sid.stream_id_int(), params,
+ payload);
});
if (error.ok()) {
diff --git a/pc/data_channel_controller.h b/pc/data_channel_controller.h
index 4dc1d97..936f9d5 100644
--- a/pc/data_channel_controller.h
+++ b/pc/data_channel_controller.h
@@ -47,12 +47,12 @@
// Implements
// SctpDataChannelProviderInterface.
- bool SendData(int sid,
+ bool SendData(StreamId sid,
const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result) override;
- void AddSctpDataStream(int sid) override;
- void RemoveSctpDataStream(int sid) override;
+ void AddSctpDataStream(StreamId sid) override;
+ void RemoveSctpDataStream(StreamId sid) override;
bool ReadyToSendData() const override;
void OnChannelStateChanged(SctpDataChannel* channel,
DataChannelInterface::DataState state) override;
@@ -124,7 +124,7 @@
RTC_RUN_ON(signaling_thread());
// Called from SendData when data_channel_transport() is true.
- bool DataChannelSendData(int sid,
+ bool DataChannelSendData(StreamId sid,
const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result);
diff --git a/pc/data_channel_unittest.cc b/pc/data_channel_unittest.cc
index 265340f..4eeeac1 100644
--- a/pc/data_channel_unittest.cc
+++ b/pc/data_channel_unittest.cc
@@ -134,12 +134,10 @@
EXPECT_TRUE(controller_->IsConnected(dc.get()));
// The sid is not set yet, so it should not have added the streams.
- EXPECT_FALSE(controller_->IsSendStreamAdded(dc->id()));
- EXPECT_FALSE(controller_->IsRecvStreamAdded(dc->id()));
+ EXPECT_FALSE(controller_->IsStreamAdded(dc->sid()));
dc->SetSctpSid(StreamId(0));
- EXPECT_TRUE(controller_->IsSendStreamAdded(dc->id()));
- EXPECT_TRUE(controller_->IsRecvStreamAdded(dc->id()));
+ EXPECT_TRUE(controller_->IsStreamAdded(dc->sid()));
}
// Tests the state of the data channel.
diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index 34bc909..5c534c9 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -211,7 +211,7 @@
// Try to connect to the transport in case the transport channel already
// exists.
if (id_.HasValue()) {
- controller_->AddSctpDataStream(id_.stream_id_int());
+ controller_->AddSctpDataStream(id_);
}
}
@@ -370,7 +370,7 @@
RTC_DCHECK_EQ(state_, kConnecting);
id_ = sid;
- controller_->AddSctpDataStream(sid.stream_id_int());
+ controller_->AddSctpDataStream(sid);
}
void SctpDataChannel::OnClosingProcedureStartedRemotely() {
@@ -407,7 +407,7 @@
// The sid may have been unassigned when controller_->ConnectDataChannel was
// done. So always add the streams even if connected_to_transport_ is true.
if (id_.HasValue()) {
- controller_->AddSctpDataStream(id_.stream_id_int());
+ controller_->AddSctpDataStream(id_);
}
}
@@ -584,7 +584,7 @@
// afterwards.
if (!started_closing_procedure_ && controller_ && id_.HasValue()) {
started_closing_procedure_ = true;
- controller_->RemoveSctpDataStream(id_.stream_id_int());
+ controller_->RemoveSctpDataStream(id_);
}
}
} else {
@@ -671,8 +671,8 @@
buffer.binary ? DataMessageType::kBinary : DataMessageType::kText;
cricket::SendDataResult send_result = cricket::SDR_SUCCESS;
- bool success = controller_->SendData(id_.stream_id_int(), send_params,
- buffer.data, &send_result);
+ bool success =
+ controller_->SendData(id_, send_params, buffer.data, &send_result);
if (success) {
++messages_sent_;
@@ -749,8 +749,7 @@
send_params.type = DataMessageType::kControl;
cricket::SendDataResult send_result = cricket::SDR_SUCCESS;
- bool retval = controller_->SendData(id_.stream_id_int(), send_params, buffer,
- &send_result);
+ bool retval = controller_->SendData(id_, send_params, buffer, &send_result);
if (retval) {
RTC_DLOG(LS_VERBOSE) << "Sent CONTROL message on channel "
<< id_.stream_id_int();
diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h
index 628a905..e813f2b 100644
--- a/pc/sctp_data_channel.h
+++ b/pc/sctp_data_channel.h
@@ -39,15 +39,15 @@
class SctpDataChannelControllerInterface {
public:
// Sends the data to the transport.
- virtual bool SendData(int sid,
+ virtual bool SendData(StreamId sid,
const SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result) = 0;
// Adds the data channel SID to the transport for SCTP.
- virtual void AddSctpDataStream(int sid) = 0;
+ virtual void AddSctpDataStream(StreamId sid) = 0;
// Begins the closing procedure by sending an outgoing stream reset. Still
// need to wait for callbacks to tell when this completes.
- virtual void RemoveSctpDataStream(int sid) = 0;
+ virtual void RemoveSctpDataStream(StreamId sid) = 0;
// Returns true if the transport channel is ready to send data.
virtual bool ReadyToSendData() const = 0;
// Notifies the controller of state changes.
diff --git a/pc/sctp_utils.cc b/pc/sctp_utils.cc
index 3677a9a..54742c2 100644
--- a/pc/sctp_utils.cc
+++ b/pc/sctp_utils.cc
@@ -16,7 +16,6 @@
#include "absl/types/optional.h"
#include "api/priority.h"
-#include "media/sctp/sctp_transport_internal.h"
#include "rtc_base/byte_buffer.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/logging.h"
@@ -47,53 +46,6 @@
DCO_PRIORITY_HIGH = 1024,
};
-StreamId::StreamId() : id_(absl::nullopt) {
- thread_checker_.Detach();
-}
-
-StreamId::StreamId(int id)
- : id_(id >= cricket::kMinSctpSid && id <= cricket::kSpecMaxSctpSid
- ? absl::optional<uint16_t>(static_cast<uint16_t>(id))
- : absl::nullopt) {
- thread_checker_.Detach();
-}
-
-StreamId::StreamId(const StreamId& sid) : id_(sid.id_) {}
-
-bool StreamId::HasValue() const {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- return id_.has_value();
-}
-
-int StreamId::stream_id_int() const {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- return id_.has_value() ? static_cast<int>(id_.value().value()) : -1;
-}
-
-void StreamId::reset() {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- id_ = absl::nullopt;
-}
-
-StreamId& StreamId::operator=(const StreamId& sid) {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- RTC_DCHECK_RUN_ON(&sid.thread_checker_);
- id_ = sid.id_;
- return *this;
-}
-
-bool StreamId::operator==(const StreamId& sid) const {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- RTC_DCHECK_RUN_ON(&sid.thread_checker_);
- return id_ == sid.id_;
-}
-
-bool StreamId::operator<(const StreamId& sid) const {
- RTC_DCHECK_RUN_ON(&thread_checker_);
- RTC_DCHECK_RUN_ON(&sid.thread_checker_);
- return id_ < sid.id_;
-}
-
bool IsOpenMessage(const rtc::CopyOnWriteBuffer& payload) {
// Format defined at
// https://www.rfc-editor.org/rfc/rfc8832#section-5.1
diff --git a/pc/sctp_utils.h b/pc/sctp_utils.h
index d0c66de..868a8be 100644
--- a/pc/sctp_utils.h
+++ b/pc/sctp_utils.h
@@ -14,13 +14,12 @@
#include <string>
#include "api/data_channel_interface.h"
-#include "api/sequence_checker.h"
#include "api/transport/data_channel_transport_interface.h"
#include "media/base/media_channel.h"
+#include "media/sctp/sctp_transport_internal.h"
#include "net/dcsctp/public/types.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/ssl_stream_adapter.h" // For SSLRole
-#include "rtc_base/system/no_unique_address.h"
namespace rtc {
class CopyOnWriteBuffer;
@@ -36,9 +35,13 @@
// this class or the internal dcsctp::StreamID type.
class StreamId {
public:
- StreamId();
- explicit StreamId(int id);
- explicit StreamId(const StreamId& sid);
+ StreamId() = default;
+ explicit StreamId(int id)
+ : id_(id >= cricket::kMinSctpSid && id <= cricket::kSpecMaxSctpSid
+ ? absl::optional<uint16_t>(static_cast<uint16_t>(id))
+ : absl::nullopt) {}
+ StreamId(const StreamId& sid) = default;
+ StreamId& operator=(const StreamId& sid) = default;
// Returns `true` if a valid stream id is contained, in the range of
// kMinSctpSid - kSpecMaxSctpSid ([0..0xffff]). Note that this
@@ -46,22 +49,23 @@
// the limit that is internally used by `SctpSidAllocator`. Sid values may
// be assigned to `StreamId` outside of `SctpSidAllocator` and have a higher
// id value than supplied by `SctpSidAllocator`, yet is still valid.
- bool HasValue() const;
+ bool HasValue() const { return id_.has_value(); }
// Provided for compatibility with existing code that hasn't been updated
// to use `StreamId` directly. New code should not use 'int' for the stream
// id but rather `StreamId` directly.
- int stream_id_int() const;
- void reset();
+ int stream_id_int() const {
+ return id_.has_value() ? static_cast<int>(id_.value().value()) : -1;
+ }
- StreamId& operator=(const StreamId& sid);
- bool operator==(const StreamId& sid) const;
- bool operator<(const StreamId& sid) const;
+ void reset() { id_ = absl::nullopt; }
+
+ bool operator==(const StreamId& sid) const { return id_ == sid.id_; }
+ bool operator<(const StreamId& sid) const { return id_ < sid.id_; }
bool operator!=(const StreamId& sid) const { return !(operator==(sid)); }
private:
- RTC_NO_UNIQUE_ADDRESS webrtc::SequenceChecker thread_checker_;
- absl::optional<dcsctp::StreamID> id_ RTC_GUARDED_BY(thread_checker_);
+ absl::optional<dcsctp::StreamID> id_;
};
// Read the message type and return true if it's an OPEN message.
diff --git a/pc/test/fake_data_channel_controller.h b/pc/test/fake_data_channel_controller.h
index 8cb8098..4801c19 100644
--- a/pc/test/fake_data_channel_controller.h
+++ b/pc/test/fake_data_channel_controller.h
@@ -44,7 +44,7 @@
return channel;
}
- bool SendData(int sid,
+ bool SendData(webrtc::StreamId sid,
const webrtc::SendDataParams& params,
const rtc::CopyOnWriteBuffer& payload,
cricket::SendDataResult* result) override {
@@ -60,28 +60,26 @@
return false;
}
- last_sid_ = sid;
+ last_sid_ = sid.stream_id_int();
last_send_data_params_ = params;
return true;
}
- void AddSctpDataStream(int sid) override {
- RTC_CHECK(sid >= 0);
+ void AddSctpDataStream(webrtc::StreamId sid) override {
+ RTC_CHECK(sid.HasValue());
if (!transport_available_) {
return;
}
- send_ssrcs_.insert(sid);
- recv_ssrcs_.insert(sid);
+ known_stream_ids_.insert(sid);
}
- void RemoveSctpDataStream(int sid) override {
- RTC_CHECK(sid >= 0);
- send_ssrcs_.erase(sid);
- recv_ssrcs_.erase(sid);
+ void RemoveSctpDataStream(webrtc::StreamId sid) override {
+ RTC_CHECK(sid.HasValue());
+ known_stream_ids_.erase(sid);
// Unlike the real SCTP transport, act like the closing procedure finished
// instantly, doing the same snapshot thing as below.
auto it = absl::c_find_if(connected_channels_,
- [&](const auto* c) { return c->id() == sid; });
+ [&](const auto* c) { return c->sid() == sid; });
// This path mimics the DCC's OnChannelClosed handler since the FDCC
// (this class) doesn't have a transport that would do that.
if (it != connected_channels_.end())
@@ -146,12 +144,8 @@
return connected_channels_.find(data_channel) != connected_channels_.end();
}
- bool IsSendStreamAdded(uint32_t stream) const {
- return send_ssrcs_.find(stream) != send_ssrcs_.end();
- }
-
- bool IsRecvStreamAdded(uint32_t stream) const {
- return recv_ssrcs_.find(stream) != recv_ssrcs_.end();
+ bool IsStreamAdded(webrtc::StreamId id) const {
+ return known_stream_ids_.find(id) != known_stream_ids_.end();
}
int channels_opened() const { return channels_opened_; }
@@ -167,8 +161,7 @@
int channels_closed_ = 0;
int channels_opened_ = 0;
std::set<webrtc::SctpDataChannel*> connected_channels_;
- std::set<uint32_t> send_ssrcs_;
- std::set<uint32_t> recv_ssrcs_;
+ std::set<webrtc::StreamId> known_stream_ids_;
rtc::WeakPtrFactory<FakeDataChannelController> weak_factory_{this};
};
#endif // PC_TEST_FAKE_DATA_CHANNEL_CONTROLLER_H_
