From 46053e4aae3650035438a5ecda751ff44ad1bb25 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= <hbos@webrtc.org>
Date: Fri, 20 Jan 2023 13:18:53 +0100
Subject: [PATCH] Handle the case of missing certificates.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Creating a data channel or negotiating it can make the SCTP transport
name go from nothing (empty string) to something. Inside the
RTCStatsCollector this is relevant because which transports we have
affect which certificates we should cache, so this is an instance of
having to call ClearStatsCache().

The bug is that we don't. This CL fixes the bug.

I tried to create unittests to cover this, but I was unable to
reproduce the race in a testing environment (if I did it would have
hit an RTC_DCHECK). Not ideal... but I hope we can land it anyway since
the fix is trivial and clearing the cache in response to API calls is
worst case harmless.

Bug: webrtc:14844
Change-Id: Ia7174cde040839e5555237db6de285297120b123
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/291112
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39160}
---
 pc/peer_connection.cc | 12 +++++++++---
 pc/peer_connection.h  |  2 ++
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/pc/peer_connection.cc b/pc/peer_connection.cc
index 36ffd1122f..2e7434083a 100644
--- a/pc/peer_connection.cc
+++ b/pc/peer_connection.cc
@@ -2097,7 +2097,7 @@ void PeerConnection::SetSctpDataMid(const std::string& mid) {
 void PeerConnection::ResetSctpDataMid() {
   RTC_DCHECK_RUN_ON(signaling_thread());
   sctp_mid_s_.reset();
-  sctp_transport_name_s_.clear();
+  SetSctpTransportName("");
 }
 
 void PeerConnection::OnSctpDataChannelClosed(DataChannelInterface* channel) {
@@ -2305,6 +2305,12 @@ absl::optional<std::string> PeerConnection::sctp_transport_name() const {
   return absl::optional<std::string>();
 }
 
+void PeerConnection::SetSctpTransportName(std::string sctp_transport_name) {
+  RTC_DCHECK_RUN_ON(signaling_thread());
+  sctp_transport_name_s_ = std::move(sctp_transport_name);
+  ClearStatsCache();
+}
+
 absl::optional<std::string> PeerConnection::sctp_mid() const {
   RTC_DCHECK_RUN_ON(signaling_thread());
   return sctp_mid_s_;
@@ -2534,7 +2540,7 @@ bool PeerConnection::SetupDataChannelTransport_n(const std::string& mid) {
         SafeTask(signaling_thread_safety_.flag(),
                  [this, name = dtls_transport->transport_name()] {
                    RTC_DCHECK_RUN_ON(signaling_thread());
-                   sctp_transport_name_s_ = std::move(name);
+                   SetSctpTransportName(std::move(name));
                  }));
   }
 
@@ -2922,7 +2928,7 @@ bool PeerConnection::OnTransportChanged(
           [this,
            name = std::string(dtls_transport->internal()->transport_name())] {
             RTC_DCHECK_RUN_ON(signaling_thread());
-            sctp_transport_name_s_ = std::move(name);
+            SetSctpTransportName(std::move(name));
           }));
     }
   }
diff --git a/pc/peer_connection.h b/pc/peer_connection.h
index a4b21f0e01..850142b958 100644
--- a/pc/peer_connection.h
+++ b/pc/peer_connection.h
@@ -597,6 +597,8 @@ class PeerConnection : public PeerConnectionInternal,
       rtc::scoped_refptr<DtlsTransport> dtls_transport,
       DataChannelTransportInterface* data_channel_transport) override;
 
+  void SetSctpTransportName(std::string sctp_transport_name);
+
   std::function<void(const rtc::CopyOnWriteBuffer& packet,
                      int64_t packet_time_us)>
   InitializeRtcpCallback();