Merge SignalPortPruned and SignalPortsRemoved.
These two signals have the same purpose and is kind of redundant.
Rename to SignalPortsPruned.
BUG=
R=pthatcher@webrtc.org, zhihuang@webrtc.org
Review URL: https://codereview.webrtc.org/2176743003 .
Cr-Commit-Position: refs/heads/master@{#13562}
diff --git a/webrtc/p2p/base/p2ptransportchannel.cc b/webrtc/p2p/base/p2ptransportchannel.cc
index 5238da8..c12e8db 100644
--- a/webrtc/p2p/base/p2ptransportchannel.cc
+++ b/webrtc/p2p/base/p2ptransportchannel.cc
@@ -136,9 +136,7 @@
session->set_generation(static_cast<uint32_t>(allocator_sessions_.size()));
session->SignalPortReady.connect(this, &P2PTransportChannel::OnPortReady);
- session->SignalPortsRemoved.connect(this,
- &P2PTransportChannel::OnPortsRemoved);
- session->SignalPortPruned.connect(this, &P2PTransportChannel::OnPortPruned);
+ session->SignalPortsPruned.connect(this, &P2PTransportChannel::OnPortsPruned);
session->SignalCandidatesReady.connect(
this, &P2PTransportChannel::OnCandidatesReady);
session->SignalCandidatesRemoved.connect(
@@ -149,7 +147,7 @@
// We now only want to apply new candidates that we receive to the ports
// created by this new session because these are replacing those of the
// previous sessions.
- removed_ports_.insert(removed_ports_.end(), ports_.begin(), ports_.end());
+ pruned_ports_.insert(pruned_ports_.end(), ports_.begin(), ports_.end());
ports_.clear();
allocator_sessions_.push_back(std::move(session));
@@ -242,7 +240,7 @@
}
// Update role on removed ports as well, because they may still have
// connections alive that should be using the correct role.
- for (PortInterface* port : removed_ports_) {
+ for (PortInterface* port : pruned_ports_) {
port->SetIceRole(ice_role);
}
}
@@ -250,7 +248,7 @@
void P2PTransportChannel::SetIceTiebreaker(uint64_t tiebreaker) {
ASSERT(worker_thread_ == rtc::Thread::Current());
- if (!ports_.empty() || !removed_ports_.empty()) {
+ if (!ports_.empty() || !pruned_ports_.empty()) {
LOG(LS_ERROR)
<< "Attempt to change tiebreaker after Port has been allocated.";
return;
@@ -1678,20 +1676,19 @@
ASSERT(worker_thread_ == rtc::Thread::Current());
ports_.erase(std::remove(ports_.begin(), ports_.end(), port), ports_.end());
- removed_ports_.erase(
- std::remove(removed_ports_.begin(), removed_ports_.end(), port),
- removed_ports_.end());
+ pruned_ports_.erase(
+ std::remove(pruned_ports_.begin(), pruned_ports_.end(), port),
+ pruned_ports_.end());
LOG(INFO) << "Removed port because it is destroyed: " << ports_.size()
<< " remaining";
}
-void P2PTransportChannel::OnPortsRemoved(
+void P2PTransportChannel::OnPortsPruned(
PortAllocatorSession* session,
const std::vector<PortInterface*>& ports) {
ASSERT(worker_thread_ == rtc::Thread::Current());
- LOG(LS_INFO) << "Remove " << ports.size() << " ports";
for (PortInterface* port : ports) {
- if (RemovePort(port)) {
+ if (OnPortPruned(port)) {
LOG(INFO) << "Removed port: " << port->ToString() << " " << ports_.size()
<< " remaining";
}
@@ -1730,22 +1727,14 @@
MSG_REGATHER_ON_FAILED_NETWORKS);
}
-void P2PTransportChannel::OnPortPruned(PortAllocatorSession* session,
- PortInterface* port) {
- if (RemovePort(port)) {
- LOG(INFO) << "Removed port because it is pruned: " << port->ToString()
- << " " << ports_.size() << " remaining";
- }
-}
-
-bool P2PTransportChannel::RemovePort(PortInterface* port) {
+bool P2PTransportChannel::OnPortPruned(PortInterface* port) {
auto it = std::find(ports_.begin(), ports_.end(), port);
// Don't need to do anything if the port has been deleted from the port list.
if (it == ports_.end()) {
return false;
}
ports_.erase(it);
- removed_ports_.push_back(port);
+ pruned_ports_.push_back(port);
return true;
}
diff --git a/webrtc/p2p/base/p2ptransportchannel.h b/webrtc/p2p/base/p2ptransportchannel.h
index 692b098..f363adc 100644
--- a/webrtc/p2p/base/p2ptransportchannel.h
+++ b/webrtc/p2p/base/p2ptransportchannel.h
@@ -276,11 +276,8 @@
void AddConnection(Connection* connection);
void OnPortReady(PortAllocatorSession *session, PortInterface* port);
- // TODO(honghaiz): Merge the two methods OnPortsRemoved and OnPortPruned but
- // still log the reason of removing.
- void OnPortsRemoved(PortAllocatorSession* session,
- const std::vector<PortInterface*>& ports);
- void OnPortPruned(PortAllocatorSession* session, PortInterface* port);
+ void OnPortsPruned(PortAllocatorSession* session,
+ const std::vector<PortInterface*>& ports);
void OnCandidatesReady(PortAllocatorSession *session,
const std::vector<Candidate>& candidates);
void OnCandidatesRemoved(PortAllocatorSession* session,
@@ -294,11 +291,11 @@
bool port_muxed);
// When a port is destroyed, remove it from both lists |ports_|
- // and |removed_ports_|.
+ // and |pruned_ports_|.
void OnPortDestroyed(PortInterface* port);
- // When removing a port, move it from |ports_| to |removed_ports_|.
+ // When pruning a port, move it from |ports_| to |pruned_ports_|.
// Returns true if the port is found and removed from |ports_|.
- bool RemovePort(PortInterface* port);
+ bool OnPortPruned(PortInterface* port);
void OnRoleConflict(PortInterface* port);
void OnConnectionStateChange(Connection* connection);
@@ -366,11 +363,11 @@
// |ports_| contains ports that are used to form new connections when
// new remote candidates are added.
std::vector<PortInterface*> ports_;
- // |removed_ports_| contains ports that have been removed from |ports_| and
+ // |pruned_ports_| contains ports that have been removed from |ports_| and
// are not being used to form new connections, but that aren't yet destroyed.
// They may have existing connections, and they still fire signals such as
// SignalUnknownAddress.
- std::vector<PortInterface*> removed_ports_;
+ std::vector<PortInterface*> pruned_ports_;
// |connections_| is a sorted list with the first one always be the
// |selected_connection_| when it's not nullptr. The combination of
diff --git a/webrtc/p2p/base/p2ptransportchannel_unittest.cc b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
index 6673f8f..c9600dd 100644
--- a/webrtc/p2p/base/p2ptransportchannel_unittest.cc
+++ b/webrtc/p2p/base/p2ptransportchannel_unittest.cc
@@ -3550,7 +3550,7 @@
// Make a fake signal to remove the ports in the p2ptransportchannel. then
// change the ICE role and expect it to be updated.
std::vector<PortInterface*> ports(1, conn->port());
- ch.allocator_session()->SignalPortsRemoved(ch.allocator_session(), ports);
+ ch.allocator_session()->SignalPortsPruned(ch.allocator_session(), ports);
ch.SetIceRole(ICEROLE_CONTROLLED);
EXPECT_EQ(ICEROLE_CONTROLLED, conn->port()->GetIceRole());
}
diff --git a/webrtc/p2p/base/portallocator.h b/webrtc/p2p/base/portallocator.h
index 152474d..5deae7f 100644
--- a/webrtc/p2p/base/portallocator.h
+++ b/webrtc/p2p/base/portallocator.h
@@ -191,11 +191,12 @@
virtual bool CandidatesAllocationDone() const = 0;
sigslot::signal2<PortAllocatorSession*, PortInterface*> SignalPortReady;
- // Ports should be signaled to be removed when the networks of the ports
- // failed (either because the interface is down, or because there is no
- // connection on the interface).
+ // Fires this signal when the network of the ports failed (either because the
+ // interface is down, or because there is no connection on the interface),
+ // or when TURN ports are pruned because a higher-priority TURN port becomes
+ // ready(pairable).
sigslot::signal2<PortAllocatorSession*, const std::vector<PortInterface*>&>
- SignalPortsRemoved;
+ SignalPortsPruned;
sigslot::signal2<PortAllocatorSession*,
const std::vector<Candidate>&> SignalCandidatesReady;
// Candidates should be signaled to be removed when the port that generated
@@ -203,11 +204,6 @@
sigslot::signal2<PortAllocatorSession*, const std::vector<Candidate>&>
SignalCandidatesRemoved;
sigslot::signal1<PortAllocatorSession*> SignalCandidatesAllocationDone;
- // A TURN port is pruned if a higher-priority TURN port becomes ready
- // (pairable). When it is pruned, it will not be used for creating
- // connections and its candidates will not be sent to the remote side
- // if they have not been sent.
- sigslot::signal2<PortAllocatorSession*, PortInterface*> SignalPortPruned;
virtual uint32_t generation() { return generation_; }
virtual void set_generation(uint32_t generation) { generation_ = generation; }
diff --git a/webrtc/p2p/client/basicportallocator.cc b/webrtc/p2p/client/basicportallocator.cc
index 5e4930c..46c2892 100644
--- a/webrtc/p2p/client/basicportallocator.cc
+++ b/webrtc/p2p/client/basicportallocator.cc
@@ -695,12 +695,12 @@
// Note: We should check whether any candidates may become ready after this
// because there we will check whether the candidate is generated by the ready
// ports, which may include this port.
- bool pruned_port = false;
+ bool pruned = false;
if (CandidatePairable(c, port) && !data->has_pairable_candidate()) {
data->set_has_pairable_candidate(true);
if (prune_turn_ports_ && port->Type() == RELAY_PORT_TYPE) {
- pruned_port = PruneTurnPorts(port);
+ pruned = PruneTurnPorts(port);
}
// If the current port is not pruned yet, SignalPortReady.
if (!data->pruned()) {
@@ -726,7 +726,7 @@
}
// If we have pruned any port, maybe need to signal port allocation done.
- if (pruned_port) {
+ if (pruned) {
MaybeSignalCandidatesAllocationDone();
}
}
@@ -745,7 +745,6 @@
}
bool BasicPortAllocatorSession::PruneTurnPorts(Port* newly_pairable_turn_port) {
- bool pruned_port = false;
// Note: We determine the same network based only on their network names. So
// if an IPv4 address and an IPv6 address have the same network name, they
// are considered the same network here.
@@ -754,18 +753,24 @@
// |port| is already in the list of ports, so the best port cannot be nullptr.
RTC_CHECK(best_turn_port != nullptr);
+ bool pruned = false;
+ std::vector<PortInterface*> pruned_ports;
for (PortData& data : ports_) {
if (data.port()->Network()->name() == network_name &&
data.port()->Type() == RELAY_PORT_TYPE && !data.pruned() &&
ComparePort(data.port(), best_turn_port) < 0) {
data.set_pruned();
- pruned_port = true;
+ pruned = true;
if (data.port() != newly_pairable_turn_port) {
- SignalPortPruned(this, data.port());
+ pruned_ports.push_back(data.port());
}
}
}
- return pruned_port;
+ if (!pruned_ports.empty()) {
+ LOG(LS_INFO) << "Pruned " << pruned_ports.size() << " ports";
+ SignalPortsPruned(this, pruned_ports);
+ }
+ return pruned;
}
void BasicPortAllocatorSession::OnPortComplete(Port* port) {
@@ -946,7 +951,8 @@
}
}
if (!ports_to_remove.empty()) {
- SignalPortsRemoved(this, ports_to_remove);
+ LOG(LS_INFO) << "Removed " << ports_to_remove.size() << " ports";
+ SignalPortsPruned(this, ports_to_remove);
}
if (!candidates_to_remove.empty()) {
SignalCandidatesRemoved(this, candidates_to_remove);
diff --git a/webrtc/p2p/client/basicportallocator_unittest.cc b/webrtc/p2p/client/basicportallocator_unittest.cc
index 4696f23..9f91aa0 100644
--- a/webrtc/p2p/client/basicportallocator_unittest.cc
+++ b/webrtc/p2p/client/basicportallocator_unittest.cc
@@ -239,8 +239,8 @@
sid, content_name, component, ice_ufrag, ice_pwd);
session->SignalPortReady.connect(this,
&BasicPortAllocatorTest::OnPortReady);
- session->SignalPortPruned.connect(this,
- &BasicPortAllocatorTest::OnPortPruned);
+ session->SignalPortsPruned.connect(this,
+ &BasicPortAllocatorTest::OnPortsPruned);
session->SignalCandidatesReady.connect(
this, &BasicPortAllocatorTest::OnCandidatesReady);
session->SignalCandidatesAllocationDone.connect(
@@ -415,13 +415,18 @@
EXPECT_NE(ready_ports.end(),
std::find(ready_ports.begin(), ready_ports.end(), port));
}
- void OnPortPruned(PortAllocatorSession* ses, PortInterface* port) {
- LOG(LS_INFO) << "OnPortPruned: " << port->ToString();
- ports_.erase(std::remove(ports_.begin(), ports_.end(), port), ports_.end());
- // Make sure the pruned port is not in ReadyPorts.
+ void OnPortsPruned(PortAllocatorSession* ses,
+ const std::vector<PortInterface*>& ports_pruned) {
+ LOG(LS_INFO) << "Number of ports pruned: " << ports_pruned.size();
auto ready_ports = ses->ReadyPorts();
- EXPECT_EQ(ready_ports.end(),
- std::find(ready_ports.begin(), ready_ports.end(), port));
+ auto new_end = ports_.end();
+ for (PortInterface* port : ports_pruned) {
+ new_end = std::remove(ports_.begin(), new_end, port);
+ // Make sure the pruned port is not in ReadyPorts.
+ EXPECT_EQ(ready_ports.end(),
+ std::find(ready_ports.begin(), ready_ports.end(), port));
+ }
+ ports_.erase(new_end, ports_.end());
}
void OnCandidatesReady(PortAllocatorSession* ses,
