If gather_continually is set to true, keep the last port allocator session running while stopping all existing process of getting ports (when p2ptransportchannel first becomes writable).
BUG=5034
Review URL: https://codereview.webrtc.org/1359363003
Cr-Original-Commit-Position: refs/heads/master@{#10110}
Cr-Mirrored-From: https://chromium.googlesource.com/external/webrtc
Cr-Mirrored-Commit: 98db68fdaa12c5bfca8b0004eba24d034f32de71
diff --git a/p2p/base/p2ptransportchannel.cc b/p2p/base/p2ptransportchannel.cc
index 15ac006..cce20f4 100644
--- a/p2p/base/p2ptransportchannel.cc
+++ b/p2p/base/p2ptransportchannel.cc
@@ -1070,12 +1070,21 @@
// was writable, go into the writable state.
void P2PTransportChannel::HandleWritable() {
ASSERT(worker_thread_ == rtc::Thread::Current());
- if (!writable()) {
- for (uint32 i = 0; i < allocator_sessions_.size(); ++i) {
- if (allocator_sessions_[i]->IsGettingPorts()) {
- allocator_sessions_[i]->StopGettingPorts();
- }
+ if (writable()) {
+ return;
+ }
+
+ for (PortAllocatorSession* session : allocator_sessions_) {
+ if (!session->IsGettingPorts()) {
+ continue;
}
+ // If gathering continually, keep the last session running so that it
+ // will gather candidates if the networks change.
+ if (gather_continually_ && session == allocator_sessions_.back()) {
+ session->ClearGettingPorts();
+ break;
+ }
+ session->StopGettingPorts();
}
was_writable_ = true;
diff --git a/p2p/base/p2ptransportchannel_unittest.cc b/p2p/base/p2ptransportchannel_unittest.cc
index 8c25d82..4add040 100644
--- a/p2p/base/p2ptransportchannel_unittest.cc
+++ b/p2p/base/p2ptransportchannel_unittest.cc
@@ -1439,6 +1439,34 @@
DestroyChannels();
}
+// Test that if continual gathering is set to true, ICE gathering state will
+// not change to "Complete", and vice versa.
+TEST_F(P2PTransportChannelTest, TestContinualGathering) {
+ ConfigureEndpoints(OPEN, OPEN, kDefaultPortAllocatorFlags,
+ kDefaultPortAllocatorFlags);
+ SetAllocationStepDelay(0, kDefaultStepDelay);
+ SetAllocationStepDelay(1, kDefaultStepDelay);
+ CreateChannels(1);
+ cricket::IceConfig config = CreateIceConfig(1000, true);
+ ep1_ch1()->SetIceConfig(config);
+ // By default, ep2 does not gather continually.
+
+ EXPECT_TRUE_WAIT_MARGIN(ep1_ch1() != NULL && ep2_ch1() != NULL &&
+ ep1_ch1()->receiving() && ep1_ch1()->writable() &&
+ ep2_ch1()->receiving() && ep2_ch1()->writable(),
+ 1000, 1000);
+ WAIT(cricket::IceGatheringState::kIceGatheringComplete ==
+ ep1_ch1()->gathering_state(),
+ 1000);
+ EXPECT_EQ(cricket::IceGatheringState::kIceGatheringGathering,
+ ep1_ch1()->gathering_state());
+ // By now, ep2 should have completed gathering.
+ EXPECT_EQ(cricket::IceGatheringState::kIceGatheringComplete,
+ ep2_ch1()->gathering_state());
+
+ DestroyChannels();
+}
+
// Test what happens when we have 2 users behind the same NAT. This can lead
// to interesting behavior because the STUN server will only give out the
// address of the outermost NAT.
diff --git a/p2p/base/portallocator.h b/p2p/base/portallocator.h
index 6e3efa8..69b5ed0 100644
--- a/p2p/base/portallocator.h
+++ b/p2p/base/portallocator.h
@@ -91,6 +91,9 @@
// Starts gathering STUN and Relay configurations.
virtual void StartGettingPorts() = 0;
virtual void StopGettingPorts() = 0;
+ // Only stop the existing gathering process but may start new ones if needed.
+ virtual void ClearGettingPorts() = 0;
+ // Whether the process of getting ports has been stopped.
virtual bool IsGettingPorts() = 0;
sigslot::signal2<PortAllocatorSession*, PortInterface*> SignalPortReady;
diff --git a/p2p/client/basicportallocator.cc b/p2p/client/basicportallocator.cc
index 11d0cef..3c77b4f 100644
--- a/p2p/client/basicportallocator.cc
+++ b/p2p/client/basicportallocator.cc
@@ -192,10 +192,14 @@
void BasicPortAllocatorSession::StopGettingPorts() {
ASSERT(rtc::Thread::Current() == network_thread_);
running_ = false;
+ network_thread_->Post(this, MSG_CONFIG_STOP);
+ ClearGettingPorts();
+}
+
+void BasicPortAllocatorSession::ClearGettingPorts() {
network_thread_->Clear(this, MSG_ALLOCATE);
for (uint32 i = 0; i < sequences_.size(); ++i)
sequences_[i]->Stop();
- network_thread_->Post(this, MSG_CONFIG_STOP);
}
void BasicPortAllocatorSession::OnMessage(rtc::Message *message) {
diff --git a/p2p/client/basicportallocator.h b/p2p/client/basicportallocator.h
index 37ce1a0..ac2cfbf 100644
--- a/p2p/client/basicportallocator.h
+++ b/p2p/client/basicportallocator.h
@@ -112,6 +112,7 @@
virtual void StartGettingPorts();
virtual void StopGettingPorts();
+ virtual void ClearGettingPorts();
virtual bool IsGettingPorts() { return running_; }
protected:
diff --git a/p2p/client/fakeportallocator.h b/p2p/client/fakeportallocator.h
index 9e53bc0..dca86f6 100644
--- a/p2p/client/fakeportallocator.h
+++ b/p2p/client/fakeportallocator.h
@@ -63,6 +63,8 @@
virtual void StopGettingPorts() { running_ = false; }
virtual bool IsGettingPorts() { return running_; }
+ virtual void ClearGettingPorts() {}
+
int port_config_count() { return port_config_count_; }
void AddPort(cricket::Port* port) {
diff --git a/p2p/client/portallocator_unittest.cc b/p2p/client/portallocator_unittest.cc
index 2893807..92f995a 100644
--- a/p2p/client/portallocator_unittest.cc
+++ b/p2p/client/portallocator_unittest.cc
@@ -1279,3 +1279,45 @@
kClientAddr);
EXPECT_EQ(4U, candidates_.size());
}
+
+TEST_F(PortAllocatorTest, TestStopGettingPorts) {
+ AddInterface(kClientAddr);
+ allocator_->set_step_delay(cricket::kDefaultStepDelay);
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+ ASSERT_EQ_WAIT(2U, candidates_.size(), 1000);
+ EXPECT_EQ(2U, ports_.size());
+ session_->StopGettingPorts();
+ EXPECT_TRUE_WAIT(candidate_allocation_done_, 1000);
+
+ // After stopping getting ports, adding a new interface will not start
+ // getting ports again.
+ candidates_.clear();
+ ports_.clear();
+ candidate_allocation_done_ = false;
+ network_manager_.AddInterface(kClientAddr2);
+ rtc::Thread::Current()->ProcessMessages(1000);
+ EXPECT_EQ(0U, candidates_.size());
+ EXPECT_EQ(0U, ports_.size());
+}
+
+TEST_F(PortAllocatorTest, TestClearGettingPorts) {
+ AddInterface(kClientAddr);
+ allocator_->set_step_delay(cricket::kDefaultStepDelay);
+ EXPECT_TRUE(CreateSession(cricket::ICE_CANDIDATE_COMPONENT_RTP));
+ session_->StartGettingPorts();
+ ASSERT_EQ_WAIT(2U, candidates_.size(), 1000);
+ EXPECT_EQ(2U, ports_.size());
+ session_->ClearGettingPorts();
+ WAIT(candidate_allocation_done_, 1000);
+ EXPECT_FALSE(candidate_allocation_done_);
+
+ // After clearing getting ports, adding a new interface will start getting
+ // ports again.
+ candidates_.clear();
+ ports_.clear();
+ candidate_allocation_done_ = false;
+ network_manager_.AddInterface(kClientAddr2);
+ ASSERT_EQ_WAIT(2U, candidates_.size(), 1000);
+ EXPECT_EQ(2U, ports_.size());
+}
