Safeguard SctpDataChannel against detached controller

Since the lifetime of an SctpDataChannel is not strictly controlled
by its controller, the controller might go away before the channel
does. This CL guards against this.

Bug: webrtc:13931
Change-Id: I07046fe896d1a66bf89287429beb0587382a13a9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261940
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36852}
This commit is contained in:
Harald Alvestrand
2022-05-11 09:35:36 +00:00
committed by WebRTC LUCI CQ
parent 5fee7dde86
commit 9e5aeb9d92
10 changed files with 167 additions and 110 deletions

View File

@ -2524,6 +2524,7 @@ if (rtc_include_tests && !build_with_chromium) {
":data_channel_controller",
":pc_test_utils",
":peer_connection_internal",
":sctp_data_channel",
"../test:test_support",
]
}
@ -2671,7 +2672,7 @@ if (rtc_include_tests && !build_with_chromium) {
sources = [
"test/fake_audio_capture_module.cc",
"test/fake_audio_capture_module.h",
"test/fake_data_channel_provider.h",
"test/fake_data_channel_controller.h",
"test/fake_peer_connection_base.h",
"test/fake_peer_connection_for_stats.h",
"test/fake_periodic_video_source.h",

View File

@ -22,6 +22,15 @@
namespace webrtc {
DataChannelController::~DataChannelController() {
// Since channels may have multiple owners, we cannot guarantee that
// they will be deallocated before destroying the controller.
// Therefore, detach them from the controller.
for (auto channel : sctp_data_channels_) {
channel->DetachFromController();
}
}
bool DataChannelController::HasDataChannels() const {
RTC_DCHECK_RUN_ON(signaling_thread());
return !sctp_data_channels_.empty();

View File

@ -34,10 +34,11 @@ namespace webrtc {
class PeerConnectionInternal;
class DataChannelController : public SctpDataChannelProviderInterface,
class DataChannelController : public SctpDataChannelControllerInterface,
public DataChannelSink {
public:
explicit DataChannelController(PeerConnectionInternal* pc) : pc_(pc) {}
~DataChannelController();
// Not copyable or movable.
DataChannelController(DataChannelController&) = delete;

View File

@ -13,6 +13,7 @@
#include <memory>
#include "pc/peer_connection_internal.h"
#include "pc/sctp_data_channel.h"
#include "pc/test/mock_peer_connection_internal.h"
#include "test/gmock.h"
#include "test/gtest.h"
@ -44,7 +45,7 @@ TEST_F(DataChannelControllerTest, CreateDataChannelEarlyRelease) {
auto channel = dcc.InternalCreateDataChannelWithProxy(
"label",
std::make_unique<InternalDataChannelInit>(DataChannelInit()).get());
channel = nullptr; // Should call destructor of channel
channel = nullptr; // dcc holds a reference to channel, so not destroyed yet
}
TEST_F(DataChannelControllerTest, CreateDataChannelLateRelease) {
@ -56,5 +57,19 @@ TEST_F(DataChannelControllerTest, CreateDataChannelLateRelease) {
channel = nullptr;
}
TEST_F(DataChannelControllerTest, CloseAfterControllerDestroyed) {
auto dcc = std::make_unique<DataChannelController>(pc_.get());
auto channel = dcc->InternalCreateDataChannelWithProxy(
"label",
std::make_unique<InternalDataChannelInit>(DataChannelInit()).get());
// Connect to provider
auto inner_channel =
DowncastProxiedDataChannelInterfaceToSctpDataChannelForTesting(
channel.get());
dcc->ConnectDataChannel(inner_channel);
dcc.reset();
channel->Close();
}
} // namespace
} // namespace webrtc

View File

@ -23,7 +23,7 @@
#include "media/sctp/sctp_transport_internal.h"
#include "pc/sctp_data_channel.h"
#include "pc/sctp_utils.h"
#include "pc/test/fake_data_channel_provider.h"
#include "pc/test/fake_data_channel_controller.h"
#include "rtc_base/copy_on_write_buffer.h"
#include "rtc_base/gunit.h"
#include "rtc_base/ssl_stream_adapter.h"
@ -72,27 +72,27 @@ class FakeDataChannelObserver : public webrtc::DataChannelObserver {
size_t on_buffered_amount_change_count_;
};
// TODO(deadbeef): The fact that these tests use a fake provider makes them not
// too valuable. Should rewrite using the
// TODO(deadbeef): The fact that these tests use a fake controller makes them
// not too valuable. Should rewrite using the
// peerconnection_datachannel_unittest.cc infrastructure.
// TODO(bugs.webrtc.org/11547): Incorporate a dedicated network thread.
class SctpDataChannelTest : public ::testing::Test {
protected:
SctpDataChannelTest()
: provider_(new FakeDataChannelProvider()),
webrtc_data_channel_(SctpDataChannel::Create(provider_.get(),
: controller_(new FakeDataChannelController()),
webrtc_data_channel_(SctpDataChannel::Create(controller_.get(),
"test",
init_,
rtc::Thread::Current(),
rtc::Thread::Current())) {}
void SetChannelReady() {
provider_->set_transport_available(true);
controller_->set_transport_available(true);
webrtc_data_channel_->OnTransportChannelCreated();
if (webrtc_data_channel_->id() < 0) {
webrtc_data_channel_->SetSctpSid(0);
}
provider_->set_ready_to_send(true);
controller_->set_ready_to_send(true);
}
void AddObserver() {
@ -101,7 +101,7 @@ class SctpDataChannelTest : public ::testing::Test {
}
webrtc::InternalDataChannelInit init_;
std::unique_ptr<FakeDataChannelProvider> provider_;
std::unique_ptr<FakeDataChannelController> controller_;
std::unique_ptr<FakeDataChannelObserver> observer_;
rtc::scoped_refptr<SctpDataChannel> webrtc_data_channel_;
};
@ -122,29 +122,29 @@ class StateSignalsListener : public sigslot::has_slots<> {
// Verifies that the data channel is connected to the transport after creation.
TEST_F(SctpDataChannelTest, ConnectedToTransportOnCreated) {
provider_->set_transport_available(true);
controller_->set_transport_available(true);
rtc::scoped_refptr<SctpDataChannel> dc =
SctpDataChannel::Create(provider_.get(), "test1", init_,
SctpDataChannel::Create(controller_.get(), "test1", init_,
rtc::Thread::Current(), rtc::Thread::Current());
EXPECT_TRUE(provider_->IsConnected(dc.get()));
EXPECT_TRUE(controller_->IsConnected(dc.get()));
// The sid is not set yet, so it should not have added the streams.
EXPECT_FALSE(provider_->IsSendStreamAdded(dc->id()));
EXPECT_FALSE(provider_->IsRecvStreamAdded(dc->id()));
EXPECT_FALSE(controller_->IsSendStreamAdded(dc->id()));
EXPECT_FALSE(controller_->IsRecvStreamAdded(dc->id()));
dc->SetSctpSid(0);
EXPECT_TRUE(provider_->IsSendStreamAdded(dc->id()));
EXPECT_TRUE(provider_->IsRecvStreamAdded(dc->id()));
EXPECT_TRUE(controller_->IsSendStreamAdded(dc->id()));
EXPECT_TRUE(controller_->IsRecvStreamAdded(dc->id()));
}
// Verifies that the data channel is connected to the transport if the transport
// is not available initially and becomes available later.
TEST_F(SctpDataChannelTest, ConnectedAfterTransportBecomesAvailable) {
EXPECT_FALSE(provider_->IsConnected(webrtc_data_channel_.get()));
EXPECT_FALSE(controller_->IsConnected(webrtc_data_channel_.get()));
provider_->set_transport_available(true);
controller_->set_transport_available(true);
webrtc_data_channel_->OnTransportChannelCreated();
EXPECT_TRUE(provider_->IsConnected(webrtc_data_channel_.get()));
EXPECT_TRUE(controller_->IsConnected(webrtc_data_channel_.get()));
}
// Tests the state of the data channel.
@ -170,7 +170,7 @@ TEST_F(SctpDataChannelTest, StateTransition) {
EXPECT_EQ(state_signals_listener.opened_count(), 1);
EXPECT_EQ(state_signals_listener.closed_count(), 1);
// Verifies that it's disconnected from the transport.
EXPECT_FALSE(provider_->IsConnected(webrtc_data_channel_.get()));
EXPECT_FALSE(controller_->IsConnected(webrtc_data_channel_.get()));
}
// Tests that DataChannel::buffered_amount() is correct after the channel is
@ -186,7 +186,7 @@ TEST_F(SctpDataChannelTest, BufferedAmountWhenBlocked) {
EXPECT_EQ(successful_send_count,
observer_->on_buffered_amount_change_count());
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
const int number_of_packets = 3;
for (int i = 0; i < number_of_packets; ++i) {
@ -197,7 +197,7 @@ TEST_F(SctpDataChannelTest, BufferedAmountWhenBlocked) {
EXPECT_EQ(successful_send_count,
observer_->on_buffered_amount_change_count());
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
successful_send_count += number_of_packets;
EXPECT_EQ(0U, webrtc_data_channel_->buffered_amount());
EXPECT_EQ(successful_send_count,
@ -210,12 +210,12 @@ TEST_F(SctpDataChannelTest, QueuedDataSentWhenUnblocked) {
AddObserver();
SetChannelReady();
webrtc::DataBuffer buffer("abcd");
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
EXPECT_TRUE(webrtc_data_channel_->Send(buffer));
EXPECT_EQ(0U, observer_->on_buffered_amount_change_count());
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
SetChannelReady();
EXPECT_EQ(0U, webrtc_data_channel_->buffered_amount());
EXPECT_EQ(1U, observer_->on_buffered_amount_change_count());
@ -227,7 +227,7 @@ TEST_F(SctpDataChannelTest, BlockedWhenSendQueuedDataNoCrash) {
AddObserver();
SetChannelReady();
webrtc::DataBuffer buffer("abcd");
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
EXPECT_TRUE(webrtc_data_channel_->Send(buffer));
EXPECT_EQ(0U, observer_->on_buffered_amount_change_count());
@ -237,7 +237,7 @@ TEST_F(SctpDataChannelTest, BlockedWhenSendQueuedDataNoCrash) {
EXPECT_EQ(0U, observer_->on_buffered_amount_change_count());
// Unblock the channel to send queued data again, there should be no crash.
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
SetChannelReady();
EXPECT_EQ(0U, webrtc_data_channel_->buffered_amount());
EXPECT_EQ(1U, observer_->on_buffered_amount_change_count());
@ -262,7 +262,7 @@ TEST_F(SctpDataChannelTest, VerifyMessagesAndBytesSent) {
EXPECT_EQ(0U, webrtc_data_channel_->bytes_sent());
// Send three buffers while not blocked.
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
EXPECT_TRUE(webrtc_data_channel_->Send(buffers[0]));
EXPECT_TRUE(webrtc_data_channel_->Send(buffers[1]));
EXPECT_TRUE(webrtc_data_channel_->Send(buffers[2]));
@ -272,7 +272,7 @@ TEST_F(SctpDataChannelTest, VerifyMessagesAndBytesSent) {
EXPECT_EQ(bytes_sent, webrtc_data_channel_->bytes_sent());
// Send three buffers while blocked, queuing the buffers.
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
EXPECT_TRUE(webrtc_data_channel_->Send(buffers[3]));
EXPECT_TRUE(webrtc_data_channel_->Send(buffers[4]));
EXPECT_TRUE(webrtc_data_channel_->Send(buffers[5]));
@ -283,7 +283,7 @@ TEST_F(SctpDataChannelTest, VerifyMessagesAndBytesSent) {
EXPECT_EQ(bytes_sent, webrtc_data_channel_->bytes_sent());
// Unblock and make sure everything was sent.
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
EXPECT_EQ_WAIT(0U, webrtc_data_channel_->buffered_amount(), kDefaultTimeout);
bytes_sent += bytes_queued;
EXPECT_EQ(6U, webrtc_data_channel_->messages_sent());
@ -298,18 +298,18 @@ TEST_F(SctpDataChannelTest, OpenMessageSent) {
SetChannelReady();
EXPECT_GE(webrtc_data_channel_->id(), 0);
EXPECT_EQ(webrtc::DataMessageType::kControl,
provider_->last_send_data_params().type);
EXPECT_EQ(provider_->last_sid(), webrtc_data_channel_->id());
controller_->last_send_data_params().type);
EXPECT_EQ(controller_->last_sid(), webrtc_data_channel_->id());
}
TEST_F(SctpDataChannelTest, QueuedOpenMessageSent) {
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
SetChannelReady();
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
EXPECT_EQ(webrtc::DataMessageType::kControl,
provider_->last_send_data_params().type);
EXPECT_EQ(provider_->last_sid(), webrtc_data_channel_->id());
controller_->last_send_data_params().type);
EXPECT_EQ(controller_->last_sid(), webrtc_data_channel_->id());
}
// Tests that the DataChannel created after transport gets ready can enter OPEN
@ -319,7 +319,7 @@ TEST_F(SctpDataChannelTest, LateCreatedChannelTransitionToOpen) {
webrtc::InternalDataChannelInit init;
init.id = 1;
rtc::scoped_refptr<SctpDataChannel> dc =
SctpDataChannel::Create(provider_.get(), "test1", init,
SctpDataChannel::Create(controller_.get(), "test1", init,
rtc::Thread::Current(), rtc::Thread::Current());
EXPECT_EQ(webrtc::DataChannelInterface::kConnecting, dc->state());
EXPECT_TRUE_WAIT(webrtc::DataChannelInterface::kOpen == dc->state(), 1000);
@ -333,7 +333,7 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceivesOpenAck) {
init.id = 1;
init.ordered = false;
rtc::scoped_refptr<SctpDataChannel> dc =
SctpDataChannel::Create(provider_.get(), "test1", init,
SctpDataChannel::Create(controller_.get(), "test1", init,
rtc::Thread::Current(), rtc::Thread::Current());
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000);
@ -341,7 +341,7 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceivesOpenAck) {
// Sends a message and verifies it's ordered.
webrtc::DataBuffer buffer("some data");
ASSERT_TRUE(dc->Send(buffer));
EXPECT_TRUE(provider_->last_send_data_params().ordered);
EXPECT_TRUE(controller_->last_send_data_params().ordered);
// Emulates receiving an OPEN_ACK message.
cricket::ReceiveDataParams params;
@ -353,7 +353,7 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceivesOpenAck) {
// Sends another message and verifies it's unordered.
ASSERT_TRUE(dc->Send(buffer));
EXPECT_FALSE(provider_->last_send_data_params().ordered);
EXPECT_FALSE(controller_->last_send_data_params().ordered);
}
// Tests that an unordered DataChannel sends unordered data after any DATA
@ -364,7 +364,7 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceiveData) {
init.id = 1;
init.ordered = false;
rtc::scoped_refptr<SctpDataChannel> dc =
SctpDataChannel::Create(provider_.get(), "test1", init,
SctpDataChannel::Create(controller_.get(), "test1", init,
rtc::Thread::Current(), rtc::Thread::Current());
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000);
@ -378,7 +378,7 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceiveData) {
// Sends a message and verifies it's unordered.
ASSERT_TRUE(dc->Send(buffer));
EXPECT_FALSE(provider_->last_send_data_params().ordered);
EXPECT_FALSE(controller_->last_send_data_params().ordered);
}
// Tests that the channel can't open until it's successfully sent the OPEN
@ -386,37 +386,37 @@ TEST_F(SctpDataChannelTest, SendUnorderedAfterReceiveData) {
TEST_F(SctpDataChannelTest, OpenWaitsForOpenMesssage) {
webrtc::DataBuffer buffer("foo");
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
SetChannelReady();
EXPECT_EQ(webrtc::DataChannelInterface::kConnecting,
webrtc_data_channel_->state());
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen,
webrtc_data_channel_->state(), 1000);
EXPECT_EQ(webrtc::DataMessageType::kControl,
provider_->last_send_data_params().type);
controller_->last_send_data_params().type);
}
// Tests that close first makes sure all queued data gets sent.
TEST_F(SctpDataChannelTest, QueuedCloseFlushes) {
webrtc::DataBuffer buffer("foo");
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
SetChannelReady();
EXPECT_EQ(webrtc::DataChannelInterface::kConnecting,
webrtc_data_channel_->state());
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen,
webrtc_data_channel_->state(), 1000);
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
webrtc_data_channel_->Send(buffer);
webrtc_data_channel_->Close();
provider_->set_send_blocked(false);
controller_->set_send_blocked(false);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state(), 1000);
EXPECT_TRUE(webrtc_data_channel_->error().ok());
EXPECT_EQ(webrtc::DataMessageType::kText,
provider_->last_send_data_params().type);
controller_->last_send_data_params().type);
}
// Tests that messages are sent with the right id.
@ -425,7 +425,7 @@ TEST_F(SctpDataChannelTest, SendDataId) {
SetChannelReady();
webrtc::DataBuffer buffer("data");
EXPECT_TRUE(webrtc_data_channel_->Send(buffer));
EXPECT_EQ(1, provider_->last_sid());
EXPECT_EQ(1, controller_->last_sid());
}
// Tests that the incoming messages with wrong ids are rejected.
@ -468,11 +468,11 @@ TEST_F(SctpDataChannelTest, NoMsgSentIfNegotiatedAndNotFromOpenMsg) {
SetChannelReady();
rtc::scoped_refptr<SctpDataChannel> dc =
SctpDataChannel::Create(provider_.get(), "test1", config,
SctpDataChannel::Create(controller_.get(), "test1", config,
rtc::Thread::Current(), rtc::Thread::Current());
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000);
EXPECT_EQ(0, provider_->last_sid());
EXPECT_EQ(0, controller_->last_sid());
}
// Tests that DataChannel::messages_received() and DataChannel::bytes_received()
@ -532,14 +532,14 @@ TEST_F(SctpDataChannelTest, OpenAckSentIfCreatedFromOpenMessage) {
SetChannelReady();
rtc::scoped_refptr<SctpDataChannel> dc =
SctpDataChannel::Create(provider_.get(), "test1", config,
SctpDataChannel::Create(controller_.get(), "test1", config,
rtc::Thread::Current(), rtc::Thread::Current());
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kOpen, dc->state(), 1000);
EXPECT_EQ(config.id, provider_->last_sid());
EXPECT_EQ(config.id, controller_->last_sid());
EXPECT_EQ(webrtc::DataMessageType::kControl,
provider_->last_send_data_params().type);
controller_->last_send_data_params().type);
}
// Tests the OPEN_ACK role assigned by InternalDataChannelInit.
@ -565,7 +565,7 @@ TEST_F(SctpDataChannelTest, OpenWhenSendBufferFull) {
memset(buffer.MutableData(), 0, buffer.size());
webrtc::DataBuffer packet(buffer, true);
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
for (size_t i = 0;
i < webrtc::DataChannelInterface::MaxSendQueueSize() / packetSize; ++i) {
@ -583,7 +583,7 @@ TEST_F(SctpDataChannelTest, OpenWhenSendBufferFull) {
TEST_F(SctpDataChannelTest, ClosedOnTransportError) {
SetChannelReady();
webrtc::DataBuffer buffer("abcd");
provider_->set_transport_error();
controller_->set_transport_error();
EXPECT_TRUE(webrtc_data_channel_->Send(buffer));
@ -631,7 +631,7 @@ TEST_F(SctpDataChannelTest, SendEmptyData) {
// Tests that a channel can be closed without being opened or assigned an sid.
TEST_F(SctpDataChannelTest, NeverOpened) {
provider_->set_transport_available(true);
controller_->set_transport_available(true);
webrtc_data_channel_->OnTransportChannelCreated();
webrtc_data_channel_->Close();
}
@ -646,7 +646,7 @@ TEST_F(SctpDataChannelTest, TransportDestroyedWhileDataBuffered) {
webrtc::DataBuffer packet(buffer, true);
// Send a packet while sending is blocked so it ends up buffered.
provider_->set_send_blocked(true);
controller_->set_send_blocked(true);
EXPECT_TRUE(webrtc_data_channel_->Send(packet));
// Tell the data channel that its transport is being destroyed.
@ -655,7 +655,7 @@ TEST_F(SctpDataChannelTest, TransportDestroyedWhileDataBuffered) {
webrtc::RTCError error(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA, "");
error.set_error_detail(webrtc::RTCErrorDetailType::SCTP_FAILURE);
webrtc_data_channel_->OnTransportChannelClosed(error);
provider_.reset(nullptr);
controller_.reset(nullptr);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state(), kDefaultTimeout);
EXPECT_FALSE(webrtc_data_channel_->error().ok());
@ -677,7 +677,7 @@ TEST_F(SctpDataChannelTest, TransportGotErrorCode) {
error.set_sctp_cause_code(
static_cast<uint16_t>(cricket::SctpErrorCauseCode::kProtocolViolation));
webrtc_data_channel_->OnTransportChannelClosed(error);
provider_.reset(nullptr);
controller_.reset(nullptr);
EXPECT_EQ_WAIT(webrtc::DataChannelInterface::kClosed,
webrtc_data_channel_->state(), kDefaultTimeout);
EXPECT_FALSE(webrtc_data_channel_->error().ok());

View File

@ -50,7 +50,7 @@
#include "p2p/base/port.h"
#include "pc/media_stream.h"
#include "pc/stream_collection.h"
#include "pc/test/fake_data_channel_provider.h"
#include "pc/test/fake_data_channel_controller.h"
#include "pc/test/fake_peer_connection_for_stats.h"
#include "pc/test/mock_data_channel.h"
#include "pc/test/mock_rtp_receiver_internal.h"
@ -1649,13 +1649,13 @@ TEST_F(RTCStatsCollectorTest, CollectRTCPeerConnectionStats) {
}
// TODO(bugs.webrtc.org/11547): Supply a separate network thread.
FakeDataChannelProvider provider;
FakeDataChannelController controller;
rtc::scoped_refptr<SctpDataChannel> dummy_channel_a = SctpDataChannel::Create(
&provider, "DummyChannelA", InternalDataChannelInit(),
&controller, "DummyChannelA", InternalDataChannelInit(),
rtc::Thread::Current(), rtc::Thread::Current());
pc_->SignalSctpDataChannelCreated()(dummy_channel_a.get());
rtc::scoped_refptr<SctpDataChannel> dummy_channel_b = SctpDataChannel::Create(
&provider, "DummyChannelB", InternalDataChannelInit(),
&controller, "DummyChannelB", InternalDataChannelInit(),
rtc::Thread::Current(), rtc::Thread::Current());
pc_->SignalSctpDataChannelCreated()(dummy_channel_b.get());

View File

@ -141,13 +141,13 @@ bool SctpSidAllocator::IsSidAvailable(int sid) const {
}
rtc::scoped_refptr<SctpDataChannel> SctpDataChannel::Create(
SctpDataChannelProviderInterface* provider,
SctpDataChannelControllerInterface* controller,
const std::string& label,
const InternalDataChannelInit& config,
rtc::Thread* signaling_thread,
rtc::Thread* network_thread) {
auto channel = rtc::make_ref_counted<SctpDataChannel>(
config, provider, label, signaling_thread, network_thread);
config, controller, label, signaling_thread, network_thread);
if (!channel->Init()) {
return nullptr;
}
@ -163,7 +163,7 @@ rtc::scoped_refptr<DataChannelInterface> SctpDataChannel::CreateProxy(
}
SctpDataChannel::SctpDataChannel(const InternalDataChannelInit& config,
SctpDataChannelProviderInterface* provider,
SctpDataChannelControllerInterface* controller,
const std::string& label,
rtc::Thread* signaling_thread,
rtc::Thread* network_thread)
@ -173,11 +173,16 @@ SctpDataChannel::SctpDataChannel(const InternalDataChannelInit& config,
label_(label),
config_(config),
observer_(nullptr),
provider_(provider) {
controller_(controller) {
RTC_DCHECK_RUN_ON(signaling_thread_);
RTC_UNUSED(network_thread_);
}
void SctpDataChannel::DetachFromController() {
RTC_DCHECK_RUN_ON(signaling_thread_);
controller_detached_ = true;
}
bool SctpDataChannel::Init() {
RTC_DCHECK_RUN_ON(signaling_thread_);
if (config_.id < -1 ||
@ -214,7 +219,8 @@ bool SctpDataChannel::Init() {
// This has to be done async because the upper layer objects (e.g.
// Chrome glue and WebKit) are not wired up properly until after this
// function returns.
if (provider_->ReadyToSendData()) {
RTC_DCHECK(!controller_detached_);
if (controller_->ReadyToSendData()) {
AddRef();
rtc::Thread::Current()->PostTask(ToQueuedTask(
[this] {
@ -330,7 +336,8 @@ void SctpDataChannel::SetSctpSid(int sid) {
}
const_cast<InternalDataChannelInit&>(config_).id = sid;
provider_->AddSctpDataStream(sid);
RTC_DCHECK(!controller_detached_);
controller_->AddSctpDataStream(sid);
}
void SctpDataChannel::OnClosingProcedureStartedRemotely(int sid) {
@ -356,20 +363,23 @@ void SctpDataChannel::OnClosingProcedureComplete(int sid) {
// all pending data and transitioned to kClosing already.
RTC_DCHECK_EQ(state_, kClosing);
RTC_DCHECK(queued_send_data_.Empty());
DisconnectFromProvider();
DisconnectFromTransport();
SetState(kClosed);
}
}
void SctpDataChannel::OnTransportChannelCreated() {
RTC_DCHECK_RUN_ON(signaling_thread_);
if (!connected_to_provider_) {
connected_to_provider_ = provider_->ConnectDataChannel(this);
if (controller_detached_) {
return;
}
// The sid may have been unassigned when provider_->ConnectDataChannel was
// done. So always add the streams even if connected_to_provider_ is true.
if (!connected_to_transport_) {
connected_to_transport_ = controller_->ConnectDataChannel(this);
}
// The sid may have been unassigned when controller_->ConnectDataChannel was
// done. So always add the streams even if connected_to_transport_ is true.
if (config_.id >= 0) {
provider_->AddSctpDataStream(config_.id);
controller_->AddSctpDataStream(config_.id);
}
}
@ -472,8 +482,8 @@ void SctpDataChannel::CloseAbruptlyWithError(RTCError error) {
return;
}
if (connected_to_provider_) {
DisconnectFromProvider();
if (connected_to_transport_) {
DisconnectFromTransport();
}
// Closing abruptly means any queued data gets thrown away.
@ -503,7 +513,7 @@ void SctpDataChannel::UpdateState() {
switch (state_) {
case kConnecting: {
if (connected_to_provider_) {
if (connected_to_transport_) {
if (handshake_state_ == kHandshakeShouldSendOpen) {
rtc::CopyOnWriteBuffer payload;
WriteDataChannelOpenMessage(label_, config_, &payload);
@ -534,10 +544,10 @@ void SctpDataChannel::UpdateState() {
// to complete; after calling RemoveSctpDataStream,
// OnClosingProcedureComplete will end up called asynchronously
// afterwards.
if (connected_to_provider_ && !started_closing_procedure_ &&
config_.id >= 0) {
if (connected_to_transport_ && !started_closing_procedure_ &&
!controller_detached_ && config_.id >= 0) {
started_closing_procedure_ = true;
provider_->RemoveSctpDataStream(config_.id);
controller_->RemoveSctpDataStream(config_.id);
}
}
break;
@ -564,13 +574,13 @@ void SctpDataChannel::SetState(DataState state) {
}
}
void SctpDataChannel::DisconnectFromProvider() {
void SctpDataChannel::DisconnectFromTransport() {
RTC_DCHECK_RUN_ON(signaling_thread_);
if (!connected_to_provider_)
if (!connected_to_transport_ || controller_detached_)
return;
provider_->DisconnectDataChannel(this);
connected_to_provider_ = false;
controller_->DisconnectDataChannel(this);
connected_to_transport_ = false;
}
void SctpDataChannel::DeliverQueuedReceivedData() {
@ -609,6 +619,9 @@ bool SctpDataChannel::SendDataMessage(const DataBuffer& buffer,
bool queue_if_blocked) {
RTC_DCHECK_RUN_ON(signaling_thread_);
SendDataParams send_params;
if (controller_detached_) {
return false;
}
send_params.ordered = config_.ordered;
// Send as ordered if it is still going through OPEN/ACK signaling.
@ -626,7 +639,7 @@ bool SctpDataChannel::SendDataMessage(const DataBuffer& buffer,
cricket::SendDataResult send_result = cricket::SDR_SUCCESS;
bool success =
provider_->SendData(config_.id, send_params, buffer.data, &send_result);
controller_->SendData(config_.id, send_params, buffer.data, &send_result);
if (success) {
++messages_sent_;
@ -688,6 +701,9 @@ bool SctpDataChannel::SendControlMessage(const rtc::CopyOnWriteBuffer& buffer) {
RTC_DCHECK(writable_);
RTC_DCHECK_GE(config_.id, 0);
if (controller_detached_) {
return false;
}
bool is_open_message = handshake_state_ == kHandshakeShouldSendOpen;
RTC_DCHECK(!is_open_message || !config_.negotiated);
@ -700,7 +716,7 @@ bool SctpDataChannel::SendControlMessage(const rtc::CopyOnWriteBuffer& buffer) {
cricket::SendDataResult send_result = cricket::SDR_SUCCESS;
bool retval =
provider_->SendData(config_.id, send_params, buffer, &send_result);
controller_->SendData(config_.id, send_params, buffer, &send_result);
if (retval) {
RTC_LOG(LS_VERBOSE) << "Sent CONTROL message on channel " << config_.id;
@ -726,4 +742,10 @@ void SctpDataChannel::ResetInternalIdAllocatorForTesting(int new_value) {
g_unique_id = new_value;
}
SctpDataChannel* DowncastProxiedDataChannelInterfaceToSctpDataChannelForTesting(
DataChannelInterface* channel) {
return static_cast<SctpDataChannel*>(
static_cast<DataChannelProxy*>(channel)->internal());
}
} // namespace webrtc

View File

@ -37,7 +37,7 @@ class SctpDataChannel;
// TODO(deadbeef): Get rid of this and have SctpDataChannel depend on
// SctpTransportInternal (pure virtual SctpTransport interface) instead.
class SctpDataChannelProviderInterface {
class SctpDataChannelControllerInterface {
public:
// Sends the data to the transport.
virtual bool SendData(int sid,
@ -57,7 +57,7 @@ class SctpDataChannelProviderInterface {
virtual bool ReadyToSendData() const = 0;
protected:
virtual ~SctpDataChannelProviderInterface() {}
virtual ~SctpDataChannelControllerInterface() {}
};
// TODO(tommi): Change to not inherit from DataChannelInit but to have it as
@ -120,7 +120,7 @@ class SctpDataChannel : public DataChannelInterface,
public sigslot::has_slots<> {
public:
static rtc::scoped_refptr<SctpDataChannel> Create(
SctpDataChannelProviderInterface* provider,
SctpDataChannelControllerInterface* controller,
const std::string& label,
const InternalDataChannelInit& config,
rtc::Thread* signaling_thread,
@ -131,6 +131,9 @@ class SctpDataChannel : public DataChannelInterface,
static rtc::scoped_refptr<DataChannelInterface> CreateProxy(
rtc::scoped_refptr<SctpDataChannel> channel);
// Invalidate the link to the controller (DataChannelController);
void DetachFromController();
void RegisterObserver(DataChannelObserver* observer) override;
void UnregisterObserver() override;
@ -178,10 +181,10 @@ class SctpDataChannel : public DataChannelInterface,
// Specializations of CloseAbruptlyWithError
void CloseAbruptlyWithDataChannelFailure(const std::string& message);
// Slots for provider to connect signals to.
// Slots for controller to connect signals to.
//
// TODO(deadbeef): Make these private once we're hooking up signals ourselves,
// instead of relying on SctpDataChannelProviderInterface.
// instead of relying on SctpDataChannelControllerInterface.
// Called when the SctpTransport's ready to use. That can happen when we've
// finished negotiation, or if the channel was created after negotiation has
@ -223,7 +226,7 @@ class SctpDataChannel : public DataChannelInterface,
protected:
SctpDataChannel(const InternalDataChannelInit& config,
SctpDataChannelProviderInterface* client,
SctpDataChannelControllerInterface* client,
const std::string& label,
rtc::Thread* signaling_thread,
rtc::Thread* network_thread);
@ -242,7 +245,7 @@ class SctpDataChannel : public DataChannelInterface,
bool Init();
void UpdateState();
void SetState(DataState state);
void DisconnectFromProvider();
void DisconnectFromTransport();
void DeliverQueuedReceivedData();
@ -266,11 +269,12 @@ class SctpDataChannel : public DataChannelInterface,
uint64_t bytes_sent_ RTC_GUARDED_BY(signaling_thread_) = 0;
uint32_t messages_received_ RTC_GUARDED_BY(signaling_thread_) = 0;
uint64_t bytes_received_ RTC_GUARDED_BY(signaling_thread_) = 0;
SctpDataChannelProviderInterface* const provider_
SctpDataChannelControllerInterface* const controller_
RTC_GUARDED_BY(signaling_thread_);
bool controller_detached_ RTC_GUARDED_BY(signaling_thread_) = false;
HandshakeState handshake_state_ RTC_GUARDED_BY(signaling_thread_) =
kHandshakeInit;
bool connected_to_provider_ RTC_GUARDED_BY(signaling_thread_) = false;
bool connected_to_transport_ RTC_GUARDED_BY(signaling_thread_) = false;
bool writable_ RTC_GUARDED_BY(signaling_thread_) = false;
// Did we already start the graceful SCTP closing procedure?
bool started_closing_procedure_ RTC_GUARDED_BY(signaling_thread_) = false;
@ -281,6 +285,11 @@ class SctpDataChannel : public DataChannelInterface,
PacketQueue queued_send_data_ RTC_GUARDED_BY(signaling_thread_);
};
// Downcast a PeerConnectionInterface that points to a proxy object
// to its underlying SctpDataChannel object. For testing only.
SctpDataChannel* DowncastProxiedDataChannelInterfaceToSctpDataChannelForTesting(
DataChannelInterface* channel);
} // namespace webrtc
#endif // PC_SCTP_DATA_CHANNEL_H_

View File

@ -8,23 +8,23 @@
* be found in the AUTHORS file in the root of the source tree.
*/
#ifndef PC_TEST_FAKE_DATA_CHANNEL_PROVIDER_H_
#define PC_TEST_FAKE_DATA_CHANNEL_PROVIDER_H_
#ifndef PC_TEST_FAKE_DATA_CHANNEL_CONTROLLER_H_
#define PC_TEST_FAKE_DATA_CHANNEL_CONTROLLER_H_
#include <set>
#include "pc/sctp_data_channel.h"
#include "rtc_base/checks.h"
class FakeDataChannelProvider
: public webrtc::SctpDataChannelProviderInterface {
class FakeDataChannelController
: public webrtc::SctpDataChannelControllerInterface {
public:
FakeDataChannelProvider()
FakeDataChannelController()
: send_blocked_(false),
transport_available_(false),
ready_to_send_(false),
transport_error_(false) {}
virtual ~FakeDataChannelProvider() {}
virtual ~FakeDataChannelController() {}
bool SendData(int sid,
const webrtc::SendDataParams& params,
@ -157,4 +157,4 @@ class FakeDataChannelProvider
std::set<uint32_t> send_ssrcs_;
std::set<uint32_t> recv_ssrcs_;
};
#endif // PC_TEST_FAKE_DATA_CHANNEL_PROVIDER_H_
#endif // PC_TEST_FAKE_DATA_CHANNEL_CONTROLLER_H_

View File

@ -22,7 +22,7 @@
#include "pc/channel.h"
#include "pc/channel_manager.h"
#include "pc/stream_collection.h"
#include "pc/test/fake_data_channel_provider.h"
#include "pc/test/fake_data_channel_controller.h"
#include "pc/test/fake_peer_connection_base.h"
namespace webrtc {
@ -250,7 +250,7 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase {
void AddSctpDataChannel(const std::string& label,
const InternalDataChannelInit& init) {
// TODO(bugs.webrtc.org/11547): Supply a separate network thread.
AddSctpDataChannel(SctpDataChannel::Create(&data_channel_provider_, label,
AddSctpDataChannel(SctpDataChannel::Create(&data_channel_controller_, label,
init, rtc::Thread::Current(),
rtc::Thread::Current()));
}
@ -431,7 +431,7 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase {
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>>
transceivers_;
FakeDataChannelProvider data_channel_provider_;
FakeDataChannelController data_channel_controller_;
std::vector<rtc::scoped_refptr<SctpDataChannel>> sctp_data_channels_;