Fire a signal when the transport channel state changes

This fixes an issue that sometimes the transport channel state changes but
the transportcontroller is not notified.

BUG=5907
R=deadbeef@webrtc.org, pthatcher@webrtc.org

Review URL: https://codereview.webrtc.org/1996693002 .

Cr-Commit-Position: refs/heads/master@{#12880}
This commit is contained in:
Honghai Zhang 2016-05-24 13:15:02 -07:00
parent 6c96314b42
commit 1590c3937c
11 changed files with 51 additions and 25 deletions

View File

@ -129,8 +129,8 @@ DtlsTransportChannelWrapper::DtlsTransportChannelWrapper(
&DtlsTransportChannelWrapper::OnRouteChange);
channel_->SignalSelectedCandidatePairChanged.connect(
this, &DtlsTransportChannelWrapper::OnSelectedCandidatePairChanged);
channel_->SignalConnectionRemoved.connect(this,
&DtlsTransportChannelWrapper::OnConnectionRemoved);
channel_->SignalStateChanged.connect(
this, &DtlsTransportChannelWrapper::OnChannelStateChanged);
channel_->SignalReceivingState.connect(this,
&DtlsTransportChannelWrapper::OnReceivingState);
}
@ -671,10 +671,10 @@ void DtlsTransportChannelWrapper::OnSelectedCandidatePairChanged(
last_sent_packet_id);
}
void DtlsTransportChannelWrapper::OnConnectionRemoved(
void DtlsTransportChannelWrapper::OnChannelStateChanged(
TransportChannelImpl* channel) {
ASSERT(channel == channel_);
SignalConnectionRemoved(this);
SignalStateChanged(this);
}
void DtlsTransportChannelWrapper::Reconnect() {

View File

@ -221,7 +221,7 @@ class DtlsTransportChannelWrapper : public TransportChannelImpl {
TransportChannel* channel,
CandidatePairInterface* selected_candidate_pair,
int last_sent_packet_id);
void OnConnectionRemoved(TransportChannelImpl* channel);
void OnChannelStateChanged(TransportChannelImpl* channel);
void Reconnect();
rtc::Thread* worker_thread_; // Everything should occur on this thread.

View File

@ -170,8 +170,10 @@ class FakeTransportChannel : public TransportChannelImpl,
connection_count_ = connection_count;
if (connection_count)
had_connection_ = true;
// In this fake transport channel, |connection_count_| determines the
// transport channel state.
if (connection_count_ < old_connection_count)
SignalConnectionRemoved(this);
SignalStateChanged(this);
}
void SetCandidatesGatheringComplete() {

View File

@ -357,7 +357,6 @@ TransportChannelState P2PTransportChannel::ComputeState() const {
}
}
LOG_J(LS_VERBOSE, this) << "Ice is completed for this channel.";
return TransportChannelState::STATE_COMPLETED;
}
@ -1209,7 +1208,13 @@ void P2PTransportChannel::SwitchBestConnectionTo(Connection* conn) {
// change, it should be called after all the connection states have changed. For
// example, we call this at the end of SortConnections.
void P2PTransportChannel::UpdateState() {
state_ = ComputeState();
TransportChannelState state = ComputeState();
if (state_ != state) {
LOG_J(LS_INFO, this) << "Transport channel state changed from " << state_
<< " to " << state;
state_ = state;
SignalStateChanged(this);
}
bool writable = best_connection_ && best_connection_->writable();
set_writable(writable);
@ -1472,9 +1477,6 @@ void P2PTransportChannel::OnConnectionDestroyed(Connection* connection) {
}
UpdateState();
// SignalConnectionRemoved should be called after the channel state is
// updated because the receiver of the event may access the channel state.
SignalConnectionRemoved(this);
}
// When a port is destroyed remove it from our list of ports to use for

View File

@ -2008,6 +2008,8 @@ class P2PTransportChannelPingTest : public testing::Test,
this, &P2PTransportChannelPingTest::OnSelectedCandidatePairChanged);
ch->SignalReadyToSend.connect(this,
&P2PTransportChannelPingTest::OnReadyToSend);
ch->SignalStateChanged.connect(
this, &P2PTransportChannelPingTest::OnChannelStateChanged);
}
cricket::Candidate CreateHostCandidate(const std::string& ip,
@ -2077,6 +2079,9 @@ class P2PTransportChannelPingTest : public testing::Test,
void OnReadyToSend(cricket::TransportChannel* channel) {
channel_ready_to_send_ = true;
}
void OnChannelStateChanged(cricket::TransportChannelImpl* channel) {
channel_state_ = channel->GetState();
}
cricket::CandidatePairInterface* last_selected_candidate_pair() {
return last_selected_candidate_pair_;
@ -2084,6 +2089,7 @@ class P2PTransportChannelPingTest : public testing::Test,
int last_sent_packet_id() { return last_sent_packet_id_; }
bool channel_ready_to_send() { return channel_ready_to_send_; }
void reset_channel_ready_to_send() { channel_ready_to_send_ = false; }
cricket::TransportChannelState channel_state() { return channel_state_; }
private:
std::unique_ptr<rtc::PhysicalSocketServer> pss_;
@ -2092,6 +2098,7 @@ class P2PTransportChannelPingTest : public testing::Test,
cricket::CandidatePairInterface* last_selected_candidate_pair_ = nullptr;
int last_sent_packet_id_ = -1;
bool channel_ready_to_send_ = false;
cricket::TransportChannelState channel_state_ = cricket::STATE_INIT;
};
TEST_F(P2PTransportChannelPingTest, TestTriggeredChecks) {
@ -2145,6 +2152,21 @@ TEST_F(P2PTransportChannelPingTest, TestNoTriggeredChecksWhenWritable) {
EXPECT_EQ(conn2, FindNextPingableConnectionAndPingIt(&ch));
}
TEST_F(P2PTransportChannelPingTest, TestSignalStateChanged) {
cricket::FakePortAllocator pa(rtc::Thread::Current(), nullptr);
cricket::P2PTransportChannel ch("state change", 1, &pa);
PrepareChannel(&ch);
ch.Connect();
ch.MaybeStartGathering();
ch.AddRemoteCandidate(CreateHostCandidate("1.1.1.1", 1, 1));
cricket::Connection* conn1 = WaitForConnectionTo(&ch, "1.1.1.1", 1);
ASSERT_TRUE(conn1 != nullptr);
// Pruning the connection reduces the set of active connections and changes
// the channel state.
conn1->Prune();
EXPECT_EQ_WAIT(cricket::STATE_FAILED, channel_state(), kDefaultTimeout);
}
// Test adding remote candidates with different ufrags. If a remote candidate
// is added with an old ufrag, it will be discarded. If it is added with a
// ufrag that was not seen before, it will be used to create connections

View File

@ -1059,7 +1059,7 @@ void Connection::OnReadyToSend() {
void Connection::Prune() {
if (!pruned_ || active()) {
LOG_J(LS_VERBOSE, this) << "Connection pruned";
LOG_J(LS_INFO, this) << "Connection pruned";
pruned_ = true;
requests_.Clear();
set_write_state(STATE_WRITE_TIMEOUT);

View File

@ -100,9 +100,8 @@ class TransportChannelImpl : public TransportChannel {
// agents.
sigslot::signal1<TransportChannelImpl*> SignalRoleConflict;
// Emitted whenever the number of connections available to the transport
// channel decreases.
sigslot::signal1<TransportChannelImpl*> SignalConnectionRemoved;
// Emitted whenever the transport channel state changed.
sigslot::signal1<TransportChannelImpl*> SignalStateChanged;
private:
RTC_DISALLOW_COPY_AND_ASSIGN(TransportChannelImpl);

View File

@ -177,8 +177,8 @@ TransportChannel* TransportController::CreateTransportChannel_n(
this, &TransportController::OnChannelCandidatesRemoved_n);
channel->SignalRoleConflict.connect(
this, &TransportController::OnChannelRoleConflict_n);
channel->SignalConnectionRemoved.connect(
this, &TransportController::OnChannelConnectionRemoved_n);
channel->SignalStateChanged.connect(
this, &TransportController::OnChannelStateChanged_n);
channels_.insert(channels_.end(), RefCountedChannel(channel))->AddRef();
// Adding a channel could cause aggregate state to change.
UpdateAggregateStates_n();
@ -592,12 +592,12 @@ void TransportController::OnChannelRoleConflict_n(
}
}
void TransportController::OnChannelConnectionRemoved_n(
void TransportController::OnChannelStateChanged_n(
TransportChannelImpl* channel) {
RTC_DCHECK(network_thread_->IsCurrent());
LOG(LS_INFO) << channel->transport_name() << " TransportChannel "
<< channel->component()
<< " connection removed. Check if state is complete.";
<< " state changed. Check if state is complete.";
UpdateAggregateStates_n();
}

View File

@ -198,7 +198,7 @@ class TransportController : public sigslot::has_slots<>,
void OnChannelCandidatesRemoved_n(TransportChannelImpl* channel,
const Candidates& candidates);
void OnChannelRoleConflict_n(TransportChannelImpl* channel);
void OnChannelConnectionRemoved_n(TransportChannelImpl* channel);
void OnChannelStateChanged_n(TransportChannelImpl* channel);
void UpdateAggregateStates_n();

View File

@ -145,8 +145,8 @@ QuicTransportChannel::QuicTransportChannel(TransportChannelImpl* channel)
&QuicTransportChannel::OnRouteChange);
channel_->SignalSelectedCandidatePairChanged.connect(
this, &QuicTransportChannel::OnSelectedCandidatePairChanged);
channel_->SignalConnectionRemoved.connect(
this, &QuicTransportChannel::OnConnectionRemoved);
channel_->SignalStateChanged.connect(
this, &QuicTransportChannel::OnChannelStateChanged);
channel_->SignalReceivingState.connect(
this, &QuicTransportChannel::OnReceivingState);
@ -401,9 +401,10 @@ void QuicTransportChannel::OnSelectedCandidatePairChanged(
last_sent_packet_id);
}
void QuicTransportChannel::OnConnectionRemoved(TransportChannelImpl* channel) {
void QuicTransportChannel::OnChannelStateChanged(
TransportChannelImpl* channel) {
ASSERT(channel == channel_.get());
SignalConnectionRemoved(this);
SignalStateChanged(this);
}
bool QuicTransportChannel::MaybeStartQuic() {

View File

@ -247,7 +247,7 @@ class QuicTransportChannel : public TransportChannelImpl,
TransportChannel* channel,
CandidatePairInterface* selected_candidate_pair,
int last_sent_packet_id);
void OnConnectionRemoved(TransportChannelImpl* channel);
void OnChannelStateChanged(TransportChannelImpl* channel);
// Callbacks for |quic_|.
// Called when |quic_| has established the crypto handshake.