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}
This commit is contained in:
Honghai Zhang 2016-07-28 13:20:15 -07:00
parent 8bc773430f
commit 8eeecabd33
6 changed files with 51 additions and 58 deletions

View File

@ -136,9 +136,7 @@ void P2PTransportChannel::AddAllocatorSession(
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 @@ void P2PTransportChannel::AddAllocatorSession(
// 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 @@ void P2PTransportChannel::SetIceRole(IceRole ice_role) {
}
// 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::SetIceRole(IceRole ice_role) {
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 @@ void P2PTransportChannel::OnPortDestroyed(PortInterface* port) {
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 @@ void P2PTransportChannel::OnRegatherOnFailedNetworks() {
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;
}

View File

@ -276,11 +276,8 @@ class P2PTransportChannel : public TransportChannelImpl,
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 @@ class P2PTransportChannel : public TransportChannelImpl,
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 @@ class P2PTransportChannel : public TransportChannelImpl,
// |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

View File

@ -3550,7 +3550,7 @@ TEST_F(P2PTransportChannelPingTest, TestIceRoleUpdatedOnRemovedPort) {
// 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());
}

View File

@ -191,11 +191,12 @@ class PortAllocatorSession : public sigslot::has_slots<> {
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 @@ class PortAllocatorSession : public sigslot::has_slots<> {
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; }

View File

@ -695,12 +695,12 @@ void BasicPortAllocatorSession::OnCandidateReady(
// 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 @@ void BasicPortAllocatorSession::OnCandidateReady(
}
// If we have pruned any port, maybe need to signal port allocation done.
if (pruned_port) {
if (pruned) {
MaybeSignalCandidatesAllocationDone();
}
}
@ -745,7 +745,6 @@ Port* BasicPortAllocatorSession::GetBestTurnPortForNetwork(
}
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 @@ bool BasicPortAllocatorSession::PruneTurnPorts(Port* newly_pairable_turn_port) {
// |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 @@ void BasicPortAllocatorSession::RemovePortsAndCandidates(
}
}
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);

View File

@ -239,8 +239,8 @@ class BasicPortAllocatorTest : public testing::Test,
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 @@ class BasicPortAllocatorTest : public testing::Test,
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,