Unset sinks when deleting CompositeDataChannelTransport.
This fixes a DCHECK during teardown in the case when the primary
DataChannelTranspot (eg. DatagramTransport) is successfully negotiated.
DatagramTransport expects the DataSink to be unset before it's deleted.
This was not caught by existing tests because the fallback transport
(SctpDataChannelTransport) does not have the same DCHECK.
Also adds a regression test for the issue, in which SCTP is available
as a fallback but DataChannelTransport is negotiated successfully.
Bug: webrtc:9719
Change-Id: I414d964d3c85d3d01cdb5e34d6b248659a613c39
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/154365
Commit-Queue: Bjorn Mellem <mellem@webrtc.org>
Reviewed-by: Seth Hampson <shampson@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29292}
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 33fc9b9..465dca1 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -3599,6 +3599,54 @@
ASSERT_TRUE(ExpectNewFrames(media_expectations));
}
+// Tests that the data channel transport works correctly when datagram transport
+// negotiation succeeds and does not fall back to SCTP.
+TEST_P(PeerConnectionIntegrationTest,
+ DatagramTransportDataChannelDoesNotFallbackToSctp) {
+ PeerConnectionInterface::RTCConfiguration rtc_config;
+ rtc_config.rtcp_mux_policy = PeerConnectionInterface::kRtcpMuxPolicyRequire;
+ rtc_config.bundle_policy = PeerConnectionInterface::kBundlePolicyMaxBundle;
+ rtc_config.use_datagram_transport_for_data_channels = true;
+
+ // Configure one endpoint to use datagram transport for data channels while
+ // the other does not.
+ ASSERT_TRUE(CreatePeerConnectionWrappersWithConfigAndMediaTransportFactory(
+ rtc_config, rtc_config, loopback_media_transports()->first_factory(),
+ loopback_media_transports()->second_factory()));
+ ConnectFakeSignaling();
+
+ // The caller offers a data channel using either datagram transport or SCTP.
+ caller()->CreateDataChannel();
+ caller()->AddAudioVideoTracks();
+ callee()->AddAudioVideoTracks();
+ caller()->CreateAndSetAndSignalOffer();
+ ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+
+ // Ensure that the data channel transport is ready.
+ loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable);
+ loopback_media_transports()->FlushAsyncInvokes();
+
+ // Negotiation should succeed, allowing the data channel to be established.
+ ASSERT_NE(nullptr, caller()->data_channel());
+ ASSERT_TRUE_WAIT(callee()->data_channel() != nullptr, kDefaultTimeout);
+ EXPECT_TRUE_WAIT(caller()->data_observer()->IsOpen(), kDefaultTimeout);
+ EXPECT_TRUE_WAIT(callee()->data_observer()->IsOpen(), kDefaultTimeout);
+
+ // Ensure data can be sent in both directions.
+ std::string data = "hello world";
+ caller()->data_channel()->Send(DataBuffer(data));
+ EXPECT_EQ_WAIT(data, callee()->data_observer()->last_message(),
+ kDefaultTimeout);
+ callee()->data_channel()->Send(DataBuffer(data));
+ EXPECT_EQ_WAIT(data, caller()->data_observer()->last_message(),
+ kDefaultTimeout);
+
+ // Ensure that failure of the datagram negotiation doesn't impede media flow.
+ MediaExpectations media_expectations;
+ media_expectations.ExpectBidirectionalAudioAndVideo();
+ ASSERT_TRUE(ExpectNewFrames(media_expectations));
+}
+
#endif // HAVE_SCTP
// This test sets up a call between two parties with a datagram transport data
@@ -3620,7 +3668,7 @@
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
- // Ensure that the media transport is ready.
+ // Ensure that the data channel transport is ready.
loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable);
loopback_media_transports()->FlushAsyncInvokes();
@@ -3706,7 +3754,7 @@
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
- // Ensure that the media transport is ready.
+ // Ensure that the data channel transport is ready.
loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable);
loopback_media_transports()->FlushAsyncInvokes();
@@ -3743,7 +3791,7 @@
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
- // Ensure that the media transport is ready.
+ // Ensure that the data channel transport is ready.
loopback_media_transports()->SetState(webrtc::MediaTransportState::kWritable);
loopback_media_transports()->FlushAsyncInvokes();