From ee0766fcc787bf7d1eaaabdb266a919e736bddc9 Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
Date: Sun, 16 Oct 2022 12:28:25 +0200
Subject: [PATCH] [TurnPort] Update CreateOrRefreshEntry function, step 1.

TurnEntry objects need to be more aware of which specific connection
instances are associated with an entry so that entries don't get
removed only based on the address while connection instances remain.

Bug: chromium:1374310
Change-Id: I8a5d9f70ef9e74497a01e2e2cba924d5e6f518a8
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/279440
Reviewed-by: Jonas Oreland <jonaso@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#38404}
---
 p2p/base/turn_port.cc | 55 ++++++++++++++++++++++++-------------------
 p2p/base/turn_port.h  |  3 +++
 2 files changed, 34 insertions(+), 24 deletions(-)

diff --git a/p2p/base/turn_port.cc b/p2p/base/turn_port.cc
index 54684516e3..8023659358 100644
--- a/p2p/base/turn_port.cc
+++ b/p2p/base/turn_port.cc
@@ -1206,6 +1206,12 @@ bool TurnPort::CreateOrRefreshEntry(const rtc::SocketAddress& addr,
   return CreateOrRefreshEntry(addr, channel_number, "");
 }
 
+bool TurnPort::CreateOrRefreshEntry(Connection* conn, int channel_number) {
+  return CreateOrRefreshEntry(conn->remote_candidate().address(),
+                              channel_number,
+                              conn->remote_candidate().username());
+}
+
 bool TurnPort::CreateOrRefreshEntry(const rtc::SocketAddress& addr,
                                     int channel_number,
                                     absl::string_view remote_ufrag) {
@@ -1214,33 +1220,34 @@ bool TurnPort::CreateOrRefreshEntry(const rtc::SocketAddress& addr,
     entry = new TurnEntry(this, channel_number, addr, remote_ufrag);
     entries_.push_back(entry);
     return true;
-  } else {
-    if (entry->destruction_timestamp()) {
-      // Destruction should have only been scheduled (indicated by
-      // destruction_timestamp being set) if there were no connections using
-      // this address.
-      RTC_DCHECK(!GetConnection(addr));
-      // Resetting the destruction timestamp will ensure that any queued
-      // destruction tasks, when executed, will see that the timestamp doesn't
-      // match and do nothing. We do this because (currently) there's not a
-      // convenient way to cancel queued tasks.
-      entry->reset_destruction_timestamp();
-    } else {
-      // The only valid reason for destruction not being scheduled is that
-      // there's still one connection.
-      RTC_DCHECK(GetConnection(addr));
-    }
+  }
 
-    if (field_trials().IsEnabled("WebRTC-TurnAddMultiMapping")) {
-      if (entry->get_remote_ufrag() != remote_ufrag) {
-        RTC_LOG(LS_INFO) << ToString()
-                         << ": remote ufrag updated."
-                            " Sending new permission request";
-        entry->set_remote_ufrag(remote_ufrag);
-        entry->SendCreatePermissionRequest(0);
-      }
+  if (entry->destruction_timestamp()) {
+    // Destruction should have only been scheduled (indicated by
+    // destruction_timestamp being set) if there were no connections using
+    // this address.
+    RTC_DCHECK(!GetConnection(addr));
+    // Resetting the destruction timestamp will ensure that any queued
+    // destruction tasks, when executed, will see that the timestamp doesn't
+    // match and do nothing. We do this because (currently) there's not a
+    // convenient way to cancel queued tasks.
+    entry->reset_destruction_timestamp();
+  } else {
+    // The only valid reason for destruction not being scheduled is that
+    // there's still one connection.
+    RTC_DCHECK(GetConnection(addr));
+  }
+
+  if (field_trials().IsEnabled("WebRTC-TurnAddMultiMapping")) {
+    if (entry->get_remote_ufrag() != remote_ufrag) {
+      RTC_LOG(LS_INFO) << ToString()
+                       << ": remote ufrag updated."
+                          " Sending new permission request";
+      entry->set_remote_ufrag(remote_ufrag);
+      entry->SendCreatePermissionRequest(0);
     }
   }
+
   return false;
 }
 
diff --git a/p2p/base/turn_port.h b/p2p/base/turn_port.h
index e51468770a..50d78ced61 100644
--- a/p2p/base/turn_port.h
+++ b/p2p/base/turn_port.h
@@ -231,7 +231,10 @@ class TurnPort : public Port {
 
   // NOTE: This method needs to be accessible for StunPort
   // return true if entry was created (i.e channel_number consumed).
+  // TODO(tommi): Remove this method in favor of the one that accepts a
+  // `Connection` pointer.
   bool CreateOrRefreshEntry(const rtc::SocketAddress& addr, int channel_number);
+  bool CreateOrRefreshEntry(Connection* conn, int channel_number);
 
   bool CreateOrRefreshEntry(const rtc::SocketAddress& addr,
                             int channel_number,