From 141a4de0727cb2eba7d21e896e4ecf72b5be1502 Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Thu, 29 Apr 2021 12:49:25 +0200 Subject: [PATCH] datachannel: Check we transition the state properly on remote close Existing test would only check the channel was not open instead of checking for a proper transition to closing then closed. Bug: webrtc:12614 Change-Id: Iab74d6252ca1411c0fe8c1ee4ca09bc6d81af90c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/216388 Reviewed-by: Harald Alvestrand Commit-Queue: Florent Castelli Cr-Commit-Position: refs/heads/master@{#33874} --- pc/data_channel_integrationtest.cc | 18 ++++++++++++++++-- pc/test/mock_peer_connection_observers.h | 13 +++++++++---- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/pc/data_channel_integrationtest.cc b/pc/data_channel_integrationtest.cc index 11f8cb1433..e8e845e648 100644 --- a/pc/data_channel_integrationtest.cc +++ b/pc/data_channel_integrationtest.cc @@ -362,8 +362,22 @@ TEST_P(DataChannelIntegrationTest, CalleeClosesSctpDataChannel) { // Close the data channel on the callee side, and wait for it to reach the // "closed" state on both sides. callee()->data_channel()->Close(); - EXPECT_TRUE_WAIT(!caller()->data_observer()->IsOpen(), kDefaultTimeout); - EXPECT_TRUE_WAIT(!callee()->data_observer()->IsOpen(), kDefaultTimeout); + + DataChannelInterface::DataState expected_states[] = { + DataChannelInterface::DataState::kConnecting, + DataChannelInterface::DataState::kOpen, + DataChannelInterface::DataState::kClosing, + DataChannelInterface::DataState::kClosed}; + + EXPECT_EQ_WAIT(DataChannelInterface::DataState::kClosed, + caller()->data_observer()->state(), kDefaultTimeout); + EXPECT_THAT(caller()->data_observer()->states(), + ::testing::ElementsAreArray(expected_states)); + + EXPECT_EQ_WAIT(DataChannelInterface::DataState::kClosed, + callee()->data_observer()->state(), kDefaultTimeout); + EXPECT_THAT(callee()->data_observer()->states(), + ::testing::ElementsAreArray(expected_states)); } TEST_P(DataChannelIntegrationTest, SctpDataChannelConfigSentToOtherSide) { diff --git a/pc/test/mock_peer_connection_observers.h b/pc/test/mock_peer_connection_observers.h index 1d2bde15d5..413339dbf7 100644 --- a/pc/test/mock_peer_connection_observers.h +++ b/pc/test/mock_peer_connection_observers.h @@ -359,20 +359,20 @@ class MockDataChannelObserver : public webrtc::DataChannelObserver { explicit MockDataChannelObserver(webrtc::DataChannelInterface* channel) : channel_(channel) { channel_->RegisterObserver(this); - state_ = channel_->state(); + states_.push_back(channel_->state()); } virtual ~MockDataChannelObserver() { channel_->UnregisterObserver(); } void OnBufferedAmountChange(uint64_t previous_amount) override {} - void OnStateChange() override { state_ = channel_->state(); } + void OnStateChange() override { states_.push_back(channel_->state()); } void OnMessage(const DataBuffer& buffer) override { messages_.push_back( {std::string(buffer.data.data(), buffer.data.size()), buffer.binary}); } - bool IsOpen() const { return state_ == DataChannelInterface::kOpen; } + bool IsOpen() const { return state() == DataChannelInterface::kOpen; } std::vector messages() const { return messages_; } std::string last_message() const { if (messages_.empty()) @@ -387,9 +387,14 @@ class MockDataChannelObserver : public webrtc::DataChannelObserver { } size_t received_message_count() const { return messages_.size(); } + DataChannelInterface::DataState state() const { return states_.back(); } + const std::vector& states() const { + return states_; + } + private: rtc::scoped_refptr channel_; - DataChannelInterface::DataState state_; + std::vector states_; std::vector messages_; };