Remove the `SctpDataChannel::config_` member variable.
Instead there are direct member variables for the various relevant
states, some weren't needed, some can be const but the `id` member
in particular needs special handling and can't be const.
For dealing with the stream id, we now have SctpSid. A class that does range validation, checks thread safety, handles the special `-1` case (for what's essentially an unsigned 16 bit int). Using a special type
for this also has the effect that range checking happens more
consistently (although I'm not modifying the structs in api/).
With upcoming steps of avoiding thread hops, the ID may need to
migrate to the network thread, which the thread checks will help with.
Along the way, update SctpSidAllocator to use flat_set instead of std::set and moving some of the sctp data channel code to the cc file
to help with more accurately tracking code coverage.
Bug: webrtc:11547
Change-Id: Iea6e7647ab8f93052044c5afbcc449115206b4e9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/296444
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39539}
diff --git a/pc/sctp_utils.h b/pc/sctp_utils.h
index da85445..d0c66de 100644
--- a/pc/sctp_utils.h
+++ b/pc/sctp_utils.h
@@ -14,9 +14,13 @@
#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 "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;
@@ -25,6 +29,41 @@
namespace webrtc {
struct DataChannelInit;
+// Wraps the `uint16_t` sctp data channel stream id value and does range
+// checking. The class interface is `int` based to ease with DataChannelInit
+// compatibility and types used in `DataChannelController`'s interface. Going
+// forward, `int` compatibility won't be needed and we can either just use
+// this class or the internal dcsctp::StreamID type.
+class StreamId {
+ public:
+ StreamId();
+ explicit StreamId(int id);
+ explicit StreamId(const StreamId& sid);
+
+ // Returns `true` if a valid stream id is contained, in the range of
+ // kMinSctpSid - kSpecMaxSctpSid ([0..0xffff]). Note that this
+ // is different than having `kMaxSctpSid` as the upper bound, which is
+ // 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;
+
+ // 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();
+
+ StreamId& operator=(const StreamId& sid);
+ bool operator==(const StreamId& sid) const;
+ bool operator<(const StreamId& sid) const;
+ 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_);
+};
+
// Read the message type and return true if it's an OPEN message.
bool IsOpenMessage(const rtc::CopyOnWriteBuffer& payload);
@@ -35,9 +74,15 @@
bool ParseDataChannelOpenAckMessage(const rtc::CopyOnWriteBuffer& payload);
bool WriteDataChannelOpenMessage(const std::string& label,
+ const std::string& protocol,
+ absl::optional<Priority> priority,
+ bool ordered,
+ absl::optional<int> max_retransmits,
+ absl::optional<int> max_retransmit_time,
+ rtc::CopyOnWriteBuffer* payload);
+bool WriteDataChannelOpenMessage(const std::string& label,
const DataChannelInit& config,
rtc::CopyOnWriteBuffer* payload);
-
void WriteDataChannelOpenAckMessage(rtc::CopyOnWriteBuffer* payload);
} // namespace webrtc
