Add tests for DC odd/even numbering in a few more cases.

Also expands integration_test_helpers to deal with multiple
datachannels.

The bug has not yet been triggered.

Bug: webrtc:13668
Change-Id: I82a0fdae0cc32815c250a691b56c614bfd6d606b
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/251602
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35980}

pc/data_channel_integrationtest.cc

@@ -663,6 +663,55 @@ TEST_P(DataChannelIntegrationTest, DtlsRoleIsSetWhenReversed) {
   EXPECT_EQ(caller()->data_channel()->id(), 0);
 }
 
+TEST_P(DataChannelIntegrationTest,
+       DtlsRoleIsSetWhenReversedWithChannelCollision) {
+  ASSERT_TRUE(CreatePeerConnectionWrappers());
+  ConnectFakeSignaling();
+  caller()->CreateDataChannel();
+
+  callee()->SetReceivedSdpMunger([this](cricket::SessionDescription* desc) {
+    MakeActiveSctpOffer(desc);
+    callee()->CreateDataChannel();
+  });
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+  ASSERT_TRUE_WAIT(caller()->data_observer()->IsOpen(), kDefaultTimeout);
+  ASSERT_EQ_WAIT(callee()->data_channels().size(), 2U, kDefaultTimeout);
+  ASSERT_EQ_WAIT(caller()->data_channels().size(), 2U, kDefaultTimeout);
+  EXPECT_TRUE(caller()
+                  ->pc()
+                  ->GetSctpTransport()
+                  ->Information()
+                  .dtls_transport()
+                  ->Information()
+                  .role());
+  EXPECT_EQ(caller()
+                ->pc()
+                ->GetSctpTransport()
+                ->Information()
+                .dtls_transport()
+                ->Information()
+                .role(),
+            DtlsTransportTlsRole::kClient);
+  EXPECT_EQ(callee()
+                ->pc()
+                ->GetSctpTransport()
+                ->Information()
+                .dtls_transport()
+                ->Information()
+                .role(),
+            DtlsTransportTlsRole::kServer);
+  // ID should be assigned according to the odd/even rule based on role; client
+  // gets even numbers, server gets odd ones.
+  // RFC 8832 section 6.
+  ASSERT_EQ(caller()->data_channels().size(), 2U);
+  ASSERT_EQ(callee()->data_channels().size(), 2U);
+  EXPECT_EQ(caller()->data_channels()[0]->id(), 0);
+  EXPECT_EQ(caller()->data_channels()[1]->id(), 1);
+  EXPECT_EQ(callee()->data_channels()[0]->id(), 1);
+  EXPECT_EQ(callee()->data_channels()[1]->id(), 0);
+}
+
 // Test that transport stats are generated by the RTCStatsCollector for a
 // connection that only involves data channels. This is a regression test for
 // crbug.com/826972.

pc/peer_connection.cc

@@ -2169,10 +2169,12 @@ bool PeerConnection::GetSctpSslRole(rtc::SSLRole* role) {
         });
     if (!dtls_role && sdp_handler_->is_caller().has_value()) {
       // This works fine if we are the offerer, but can be a mistake if
-      // we are the answerer and the remote offer is PASSIVE. In that
+      // we are the answerer and the remote offer is ACTIVE. In that
       // case, we will guess the role wrong.
       // TODO(bugs.webrtc.org/13668): Check if this actually happens.
-      RTC_LOG(LS_ERROR) << "Possible risk: DTLS role guesser is active";
+      RTC_LOG(LS_ERROR)
+          << "Possible risk: DTLS role guesser is active, is_caller is "
+          << *sdp_handler_->is_caller();
       dtls_role =
           *sdp_handler_->is_caller() ? rtc::SSL_SERVER : rtc::SSL_CLIENT;
     }

pc/test/integration_test_helpers.h

@@ -389,14 +389,29 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
 
   void CreateDataChannel(const std::string& label,
                          const webrtc::DataChannelInit* init) {
-    data_channel_ = pc()->CreateDataChannel(label, init);
-    ASSERT_TRUE(data_channel_.get() != nullptr);
-    data_observer_.reset(new MockDataChannelObserver(data_channel_));
+    data_channels_.push_back(pc()->CreateDataChannel(label, init));
+    ASSERT_TRUE(data_channels_.back().get() != nullptr);
+    data_observers_.push_back(
+        std::make_unique<MockDataChannelObserver>(data_channels_.back()));
+  }
+
+  // Return the last observed data channel.
+  DataChannelInterface* data_channel() {
+    if (data_channels_.size() == 0) {
+      return nullptr;
+    }
+    return data_channels_.back();
+  }
+  // Return all data channels.
+  const std::vector<rtc::scoped_refptr<DataChannelInterface>>& data_channels() {
+    return data_channels_;
   }
 
-  DataChannelInterface* data_channel() { return data_channel_; }
   const MockDataChannelObserver* data_observer() const {
-    return data_observer_.get();
+    if (data_observers_.size() == 0) {
+      return nullptr;
+    }
+    return data_observers_.back().get();
   }
 
   int audio_frames_received() const {
@@ -1094,8 +1109,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
   void OnDataChannel(
       rtc::scoped_refptr<DataChannelInterface> data_channel) override {
     RTC_LOG(LS_INFO) << debug_name_ << ": OnDataChannel";
-    data_channel_ = data_channel;
-    data_observer_.reset(new MockDataChannelObserver(data_channel));
+    data_channels_.push_back(data_channel);
+    data_observers_.push_back(
+        std::make_unique<MockDataChannelObserver>(data_channel));
   }
 
   std::string debug_name_;
@@ -1138,8 +1154,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver,
   std::function<void(cricket::SessionDescription*)> generated_sdp_munger_;
   std::function<void()> remote_offer_handler_;
   rtc::MockAsyncResolver* remote_async_resolver_ = nullptr;
-  rtc::scoped_refptr<DataChannelInterface> data_channel_;
-  std::unique_ptr<MockDataChannelObserver> data_observer_;
+  // All data channels either created or observed on this peerconnection
+  std::vector<rtc::scoped_refptr<DataChannelInterface>> data_channels_;
+  std::vector<std::unique_ptr<MockDataChannelObserver>> data_observers_;
 
   std::vector<std::unique_ptr<MockRtpReceiverObserver>> rtp_receiver_observers_;