Implement max-channels for SCTP datachannels.

This involves catching another callback from usrsctp.
It also moves the definition of "connected" a little later
in the sequence: From "ready to send data" to the reception
of the SCTP_COMM_UP event.

Bug: chromium:943976
Change-Id: Ib9e1b17d0cc356f19cdfa675159b29bf1efdcb55
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/137435
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#28004}

media/sctp/sctp_transport.cc

@@ -1047,7 +1047,12 @@ void SctpTransport::OnNotificationAssocChange(const sctp_assoc_change& change) {
   RTC_DCHECK_RUN_ON(network_thread_);
   switch (change.sac_state) {
     case SCTP_COMM_UP:
-      RTC_LOG(LS_VERBOSE) << "Association change SCTP_COMM_UP";
+      RTC_LOG(LS_VERBOSE) << "Association change SCTP_COMM_UP, stream # is "
+                          << change.sac_outbound_streams << " outbound, "
+                          << change.sac_inbound_streams << " inbound.";
+      max_outbound_streams_ = change.sac_outbound_streams;
+      max_inbound_streams_ = change.sac_inbound_streams;
+      SignalAssociationChangeCommunicationUp();
       break;
     case SCTP_COMM_LOST:
       RTC_LOG(LS_INFO) << "Association change SCTP_COMM_LOST";

media/sctp/sctp_transport.h

@@ -80,6 +80,12 @@ class SctpTransport : public SctpTransportInternal,
                 SendDataResult* result = nullptr) override;
   bool ReadyToSendData() override;
   int max_message_size() const override { return max_message_size_; }
+  absl::optional<int> max_outbound_streams() const override {
+    return max_outbound_streams_;
+  }
+  absl::optional<int> max_inbound_streams() const override {
+    return max_inbound_streams_;
+  }
   void set_debug_name_for_testing(const char* debug_name) override {
     debug_name_ = debug_name;
   }
@@ -208,6 +214,9 @@ class SctpTransport : public SctpTransportInternal,
   const char* debug_name_ = "SctpTransport";
   // Hides usrsctp interactions from this header file.
   class UsrSctpWrapper;
+  // Number of channels negotiated. Not set before negotiation completes.
+  absl::optional<int> max_outbound_streams_;
+  absl::optional<int> max_inbound_streams_;
 
   RTC_DISALLOW_COPY_AND_ASSIGN(SctpTransport);
 };

media/sctp/sctp_transport_internal.h

@@ -114,8 +114,14 @@ class SctpTransportInternal {
   virtual bool ReadyToSendData() = 0;
   // Returns the current max message size, set with Start().
   virtual int max_message_size() const = 0;
+  // Returns the current negotiated max # of outbound streams.
+  // Will return absl::nullopt if negotiation is incomplete.
+  virtual absl::optional<int> max_outbound_streams() const = 0;
+  // Returns the current negotiated max # of inbound streams.
+  virtual absl::optional<int> max_inbound_streams() const = 0;
 
   sigslot::signal0<> SignalReadyToSendData;
+  sigslot::signal0<> SignalAssociationChangeCommunicationUp;
   // ReceiveDataParams includes SID, seq num, timestamp, etc. CopyOnWriteBuffer
   // contains message payload.
   sigslot::signal2<const ReceiveDataParams&, const rtc::CopyOnWriteBuffer&>

pc/sctp_transport.cc

@@ -10,6 +10,7 @@
 
 #include "pc/sctp_transport.h"
 
+#include <algorithm>
 #include <utility>
 
 namespace webrtc {
@@ -20,8 +21,8 @@ SctpTransport::SctpTransport(
       info_(SctpTransportState::kNew),
       internal_sctp_transport_(std::move(internal)) {
   RTC_DCHECK(internal_sctp_transport_.get());
-  internal_sctp_transport_->SignalReadyToSendData.connect(
+  internal_sctp_transport_->SignalAssociationChangeCommunicationUp.connect(
-      this, &SctpTransport::OnInternalReadyToSendData);
+      this, &SctpTransport::OnAssociationChangeCommunicationUp);
   // TODO(https://bugs.webrtc.org/10360): Add handlers for transport closing.
 
   if (dtls_transport_) {
@@ -143,7 +144,21 @@ void SctpTransport::UpdateInformation(SctpTransportState state) {
   }
 }
 
-void SctpTransport::OnInternalReadyToSendData() {
+void SctpTransport::OnAssociationChangeCommunicationUp() {
+  RTC_DCHECK_RUN_ON(owner_thread_);
+  {
+    rtc::CritScope scope(&lock_);
+    RTC_DCHECK(internal_sctp_transport_);
+    if (internal_sctp_transport_->max_outbound_streams() &&
+        internal_sctp_transport_->max_inbound_streams()) {
+      int max_channels =
+          std::min(*(internal_sctp_transport_->max_outbound_streams()),
+                   *(internal_sctp_transport_->max_inbound_streams()));
+      // Record max channels.
+      info_ = SctpTransportInformation(info_.state(), info_.dtls_transport(),
+                                       info_.MaxMessageSize(), max_channels);
+    }
+  }
   UpdateInformation(SctpTransportState::kConnected);
 }
 

pc/sctp_transport.h

@@ -62,6 +62,7 @@ class SctpTransport : public SctpTransportInterface,
  private:
   void UpdateInformation(SctpTransportState state);
   void OnInternalReadyToSendData();
+  void OnAssociationChangeCommunicationUp();
   void OnInternalClosingProcedureStartedRemotely(int sid);
   void OnInternalClosingProcedureComplete(int sid);
 

pc/sctp_transport_unittest.cc

@@ -21,6 +21,7 @@
 #include "test/gtest.h"
 
 constexpr int kDefaultTimeout = 1000;  // milliseconds
+constexpr int kTestMaxSctpStreams = 1234;
 
 using cricket::FakeDtlsTransport;
 using ::testing::ElementsAre;
@@ -45,9 +46,19 @@ class FakeCricketSctpTransport : public cricket::SctpTransportInternal {
   bool ReadyToSendData() override { return true; }
   void set_debug_name_for_testing(const char* debug_name) override {}
   int max_message_size() const override { return 0; }
+  absl::optional<int> max_outbound_streams() const override {
+    return max_outbound_streams_;
+  }
+  absl::optional<int> max_inbound_streams() const override {
+    return max_inbound_streams_;
+  }
   // Methods exposed for testing
   void SendSignalReadyToSendData() { SignalReadyToSendData(); }
 
+  void SendSignalAssociationChangeCommunicationUp() {
+    SignalAssociationChangeCommunicationUp();
+  }
+
   void SendSignalClosingProcedureStartedRemotely() {
     SignalClosingProcedureStartedRemotely(1);
   }
@@ -55,13 +66,24 @@ class FakeCricketSctpTransport : public cricket::SctpTransportInternal {
   void SendSignalClosingProcedureComplete() {
     SignalClosingProcedureComplete(1);
   }
+  void set_max_outbound_streams(int streams) {
+    max_outbound_streams_ = streams;
+  }
+  void set_max_inbound_streams(int streams) { max_inbound_streams_ = streams; }
+
+ private:
+  absl::optional<int> max_outbound_streams_;
+  absl::optional<int> max_inbound_streams_;
 };
 
 }  // namespace
 
 class TestSctpTransportObserver : public SctpTransportObserverInterface {
  public:
+  TestSctpTransportObserver() : info_(SctpTransportState::kNew) {}
+
   void OnStateChange(SctpTransportInformation info) override {
+    info_ = info;
     states_.push_back(info.state());
   }
 
@@ -75,8 +97,11 @@ class TestSctpTransportObserver : public SctpTransportObserverInterface {
 
   const std::vector<SctpTransportState>& States() { return states_; }
 
+  const SctpTransportInformation LastReceivedInformation() { return info_; }
+
  private:
   std::vector<SctpTransportState> states_;
+  SctpTransportInformation info_;
 };
 
 class SctpTransportTest : public ::testing::Test {
@@ -102,6 +127,11 @@ class SctpTransportTest : public ::testing::Test {
 
   void CompleteSctpHandshake() {
     CricketSctpTransport()->SendSignalReadyToSendData();
+    // The computed MaxChannels shall be the minimum of the outgoing
+    // and incoming # of streams.
+    CricketSctpTransport()->set_max_outbound_streams(kTestMaxSctpStreams);
+    CricketSctpTransport()->set_max_inbound_streams(kTestMaxSctpStreams + 1);
+    CricketSctpTransport()->SendSignalAssociationChangeCommunicationUp();
   }
 
   FakeCricketSctpTransport* CricketSctpTransport() {
@@ -149,4 +179,20 @@ TEST_F(SctpTransportTest, CloseWhenClearing) {
                  kDefaultTimeout);
 }
 
+TEST_F(SctpTransportTest, MaxChannelsSignalled) {
+  CreateTransport();
+  transport()->RegisterObserver(observer());
+  AddDtlsTransport();
+  EXPECT_FALSE(transport()->Information().MaxChannels());
+  EXPECT_FALSE(observer_.LastReceivedInformation().MaxChannels());
+  CompleteSctpHandshake();
+  ASSERT_EQ_WAIT(SctpTransportState::kConnected, observer_.State(),
+                 kDefaultTimeout);
+  EXPECT_TRUE(transport()->Information().MaxChannels());
+  EXPECT_EQ(kTestMaxSctpStreams, *(transport()->Information().MaxChannels()));
+  EXPECT_TRUE(observer_.LastReceivedInformation().MaxChannels());
+  EXPECT_EQ(kTestMaxSctpStreams,
+            *(observer_.LastReceivedInformation().MaxChannels()));
+}
+
 }  // namespace webrtc

pc/test/fake_sctp_transport.h

@@ -38,6 +38,8 @@ class FakeSctpTransport : public cricket::SctpTransportInternal {
   void set_debug_name_for_testing(const char* debug_name) override {}
 
   int max_message_size() const { return max_message_size_; }
+  absl::optional<int> max_outbound_streams() const { return absl::nullopt; }
+  absl::optional<int> max_inbound_streams() const { return absl::nullopt; }
   int local_port() const { return *local_port_; }
   int remote_port() const { return *remote_port_; }