commit 18408391ad4c947ad1a3eeae424663f15e6346ef
author Sameer Vijaykar <samvi@google.com> Wed Sep 14 09:49:47 2022
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Sep 14 14:11:09 2022
tree a9bdf681ec1f5154baafaa7083f55e9adf68e45b
parent 80ed9b8eb9b0413027fe82335e2a85bbfbc97c1a

Keep state internally of connections added to P2PTransportChannel (#4/n)

P2PTransportChannel currently relies on the ICE controller to keep track of this, even though P2PTransportChannel is actually supposed to hold the mutable connections.

Reading connections from the ICE controller also leaks some internal state from the ICE controller through the ordering of connections, which isn't strictly part of the interface. This change is a step towards fixing this.

This change is functionally no-op for now. The internal state will be used behind a field-trial in a future CL. That is also when some tests will be updated to work with the new internal state.

Bug: webrtc:14367, webrtc:1413
Change-Id: I6f8c5d805c780411fe940926f192fd2d6ce86d29
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/275081
Commit-Queue: Sameer Vijaykar <samvi@google.com>
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38080}

p2p/base/p2p_transport_channel.cc

diff --git a/p2p/base/p2p_transport_channel.cc b/p2p/base/p2p_transport_channel.cc
index 213cfbd..82d77bb 100644
--- a/p2p/base/p2p_transport_channel.cc
+++ b/p2p/base/p2p_transport_channel.cc
@@ -223,7 +223,7 @@
   std::vector<Connection*> copy(connections().begin(), connections().end());
   for (Connection* connection : copy) {
     connection->SignalDestroyed.disconnect(this);
-    ice_controller_->OnConnectionDestroyed(connection);
+    RemoveConnection(connection);
     connection->Destroy();
   }
   resolvers_.clear();
@@ -281,6 +281,7 @@
   LogCandidatePairConfig(connection,
                          webrtc::IceCandidatePairConfigType::kAdded);
 
+  connections_.push_back(connection);
   ice_controller_->AddConnection(connection);
 }
 
@@ -1693,7 +1694,7 @@
   RTC_DCHECK_RUN_ON(network_thread_);
   RTC_DCHECK(FindConnection(connection));
   connection->SignalDestroyed.disconnect(this);
-  ice_controller_->OnConnectionDestroyed(connection);
+  RemoveConnection(connection);
   RTC_DCHECK(!FindConnection(connection));
   if (selected_connection_ == connection)
     selected_connection_ = nullptr;
@@ -2045,7 +2046,7 @@
       update_selected_connection = true;
     }
     connection->SignalDestroyed.disconnect(this);
-    ice_controller_->OnConnectionDestroyed(connection);
+    RemoveConnection(connection);
     connection->Destroy();
   }
 
@@ -2172,7 +2173,7 @@
   // use it.
 
   // Remove this connection from the list.
-  ice_controller_->OnConnectionDestroyed(connection);
+  RemoveConnection(connection);
 
   RTC_LOG(LS_INFO) << ToString() << ": Removed connection " << connection
                    << " (" << connections().size() << " remaining)";
@@ -2193,6 +2194,14 @@
   }
 }
 
+void P2PTransportChannel::RemoveConnection(const Connection* connection) {
+  RTC_DCHECK_RUN_ON(network_thread_);
+  auto it = absl::c_find(connections_, connection);
+  RTC_DCHECK(it != connections_.end());
+  connections_.erase(it);
+  ice_controller_->OnConnectionDestroyed(connection);
+}
+
 // When a port is destroyed, remove it from our list of ports to use for
 // connection attempts.
 void P2PTransportChannel::OnPortDestroyed(PortInterface* port) {

p2p/base/p2p_transport_channel.h

diff --git a/p2p/base/p2p_transport_channel.h b/p2p/base/p2p_transport_channel.h
index 981bd9a..5e56df6 100644
--- a/p2p/base/p2p_transport_channel.h
+++ b/p2p/base/p2p_transport_channel.h
@@ -200,6 +200,8 @@
   int check_receiving_interval() const;
   absl::optional<rtc::NetworkRoute> network_route() const override;
 
+  void RemoveConnection(const Connection* connection);
+
   // Helper method used only in unittest.
   rtc::DiffServCodePoint DefaultDscpValue() const;
 
@@ -427,6 +429,7 @@
   std::vector<PortInterface*> pruned_ports_ RTC_GUARDED_BY(network_thread_);
 
   Connection* selected_connection_ RTC_GUARDED_BY(network_thread_) = nullptr;
+  std::vector<Connection*> connections_ RTC_GUARDED_BY(network_thread_);
 
   std::vector<RemoteCandidate> remote_candidates_
       RTC_GUARDED_BY(network_thread_);