Revert "Moves TransportFeedbackAdapter to TaskQueue."

This reverts commit 62d01cde6f6ec1fa91b1e5234a7922ad1a4ce036.

Reason for revert: Causes SIGSEGV in webrtc::RTPSender::BuildRtxPacket.

Original change's description:
> Moves TransportFeedbackAdapter to TaskQueue.
>
> Bug: webrtc:9883
> Change-Id: Id87e281751d98043f4470df5a71d458f4cd654c1
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/158793
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#30037}

TBR=sprang@webrtc.org,srte@webrtc.org

# Not skipping CQ checks because original CL landed > 1 day ago.

No-Try: True
Bug: webrtc:9883
Change-Id: If54bdb8694144fae3fafbabd72d1ac1198e51aa6
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/161726
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30052}

3 files changed, 67 insertions, 53 deletions

diff --git a/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc b/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc
index 8cf4d17a9f..2a8a224a81 100644
--- a/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc
+++ b/modules/congestion_controller/bbr/bbr_network_controller_unittest.cc
@@ -155,8 +155,8 @@ TEST_F(BbrNetworkControllerTest, UpdatesTargetSendRate) {
   ret_net->UpdateConfig(
       [](NetworkSimulationConfig* c) { c->delay = TimeDelta::ms(200); });
 
-  s.RunFor(TimeDelta::seconds(35));
-  EXPECT_NEAR(client->send_bandwidth().kbps(), 180, 50);
+  s.RunFor(TimeDelta::seconds(40));
+  EXPECT_NEAR(client->send_bandwidth().kbps(), 200, 40);
 }
 
 }  // namespace test
diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.cc b/modules/congestion_controller/rtp/transport_feedback_adapter.cc
index df52ef1b2a..b070b0e23a 100644
--- a/modules/congestion_controller/rtp/transport_feedback_adapter.cc
+++ b/modules/congestion_controller/rtp/transport_feedback_adapter.cc
@@ -73,6 +73,7 @@ TransportFeedbackAdapter::~TransportFeedbackAdapter() {
 void TransportFeedbackAdapter::RegisterStreamFeedbackObserver(
     std::vector<uint32_t> ssrcs,
     StreamFeedbackObserver* observer) {
+  rtc::CritScope cs(&observers_lock_);
   RTC_DCHECK(observer);
   RTC_DCHECK(absl::c_find_if(observers_, [=](const auto& pair) {
                return pair.second == observer;
@@ -82,6 +83,7 @@ void TransportFeedbackAdapter::RegisterStreamFeedbackObserver(
 
 void TransportFeedbackAdapter::DeRegisterStreamFeedbackObserver(
     StreamFeedbackObserver* observer) {
+  rtc::CritScope cs(&observers_lock_);
   RTC_DCHECK(observer);
   const auto it = absl::c_find_if(
       observers_, [=](const auto& pair) { return pair.second == observer; });
@@ -92,31 +94,35 @@ void TransportFeedbackAdapter::DeRegisterStreamFeedbackObserver(
 void TransportFeedbackAdapter::AddPacket(const RtpPacketSendInfo& packet_info,
                                          size_t overhead_bytes,
                                          Timestamp creation_time) {
-  PacketFeedback packet;
-  packet.creation_time = creation_time;
-  packet.sent.sequence_number =
-      seq_num_unwrapper_.Unwrap(packet_info.transport_sequence_number);
-  packet.sent.size = DataSize::bytes(packet_info.length + overhead_bytes);
-  packet.local_net_id = local_net_id_;
-  packet.remote_net_id = remote_net_id_;
-  packet.sent.pacing_info = packet_info.pacing_info;
-  if (packet_info.has_rtp_sequence_number) {
-    packet.ssrc = packet_info.ssrc;
-    packet.rtp_sequence_number = packet_info.rtp_sequence_number;
-  }
+  {
+    rtc::CritScope cs(&lock_);
+    PacketFeedback packet;
+    packet.creation_time = creation_time;
+    packet.sent.sequence_number =
+        seq_num_unwrapper_.Unwrap(packet_info.transport_sequence_number);
+    packet.sent.size = DataSize::bytes(packet_info.length + overhead_bytes);
+    packet.local_net_id = local_net_id_;
+    packet.remote_net_id = remote_net_id_;
+    packet.sent.pacing_info = packet_info.pacing_info;
+    if (packet_info.has_rtp_sequence_number) {
+      packet.ssrc = packet_info.ssrc;
+      packet.rtp_sequence_number = packet_info.rtp_sequence_number;
+    }
 
-  while (!history_.empty() &&
-         creation_time - history_.begin()->second.creation_time >
-             kSendTimeHistoryWindow) {
-    // TODO(sprang): Warn if erasing (too many) old items?
-    if (history_.begin()->second.sent.sequence_number > last_ack_seq_num_)
-      in_flight_.RemoveInFlightPacketBytes(history_.begin()->second);
-    history_.erase(history_.begin());
+    while (!history_.empty() &&
+           creation_time - history_.begin()->second.creation_time >
+               kSendTimeHistoryWindow) {
+      // TODO(sprang): Warn if erasing (too many) old items?
+      if (history_.begin()->second.sent.sequence_number > last_ack_seq_num_)
+        in_flight_.RemoveInFlightPacketBytes(history_.begin()->second);
+      history_.erase(history_.begin());
+    }
+    history_.insert(std::make_pair(packet.sent.sequence_number, packet));
   }
-  history_.insert(std::make_pair(packet.sent.sequence_number, packet));
 }
 absl::optional<SentPacket> TransportFeedbackAdapter::ProcessSentPacket(
     const rtc::SentPacket& sent_packet) {
+  rtc::CritScope cs(&lock_);
   auto send_time = Timestamp::ms(sent_packet.send_time_ms);
   // TODO(srte): Only use one way to indicate that packet feedback is used.
   if (sent_packet.info.included_in_feedback || sent_packet.packet_id != -1) {
@@ -165,37 +171,41 @@ TransportFeedbackAdapter::ProcessTransportFeedback(
   std::vector<PacketFeedback> feedback_vector;
   TransportPacketsFeedback msg;
   msg.feedback_time = feedback_receive_time;
-  msg.prior_in_flight =
-      in_flight_.GetOutstandingData(local_net_id_, remote_net_id_);
-  feedback_vector =
-      ProcessTransportFeedbackInner(feedback, feedback_receive_time);
-  if (feedback_vector.empty())
-    return absl::nullopt;
+  {
+    rtc::CritScope cs(&lock_);
+    msg.prior_in_flight =
+        in_flight_.GetOutstandingData(local_net_id_, remote_net_id_);
+    feedback_vector =
+        ProcessTransportFeedbackInner(feedback, feedback_receive_time);
+    if (feedback_vector.empty())
+      return absl::nullopt;
 
-  for (const PacketFeedback& fb : feedback_vector) {
-    PacketResult res;
-    res.sent_packet = fb.sent;
-    res.receive_time = fb.receive_time;
-    msg.packet_feedbacks.push_back(res);
-  }
-  auto it = history_.find(last_ack_seq_num_);
-  if (it != history_.end()) {
-    msg.first_unacked_send_time = it->second.sent.send_time;
+    for (const PacketFeedback& fb : feedback_vector) {
+      PacketResult res;
+      res.sent_packet = fb.sent;
+      res.receive_time = fb.receive_time;
+      msg.packet_feedbacks.push_back(res);
+    }
+    auto it = history_.find(last_ack_seq_num_);
+    if (it != history_.end()) {
+      msg.first_unacked_send_time = it->second.sent.send_time;
+    }
+    msg.data_in_flight =
+        in_flight_.GetOutstandingData(local_net_id_, remote_net_id_);
   }
-  msg.data_in_flight =
-      in_flight_.GetOutstandingData(local_net_id_, remote_net_id_);
-
   SignalObservers(feedback_vector);
   return msg;
 }
 
 void TransportFeedbackAdapter::SetNetworkIds(uint16_t local_id,
                                              uint16_t remote_id) {
+  rtc::CritScope cs(&lock_);
   local_net_id_ = local_id;
   remote_net_id_ = remote_id;
 }
 
 DataSize TransportFeedbackAdapter::GetOutstandingData() const {
+  rtc::CritScope cs(&lock_);
   return in_flight_.GetOutstandingData(local_net_id_, remote_net_id_);
 }
 
@@ -280,6 +290,7 @@ TransportFeedbackAdapter::ProcessTransportFeedbackInner(
 
 void TransportFeedbackAdapter::SignalObservers(
     const std::vector<PacketFeedback>& feedback_vector) {
+  rtc::CritScope cs(&observers_lock_);
   for (auto& observer : observers_) {
     std::vector<StreamFeedbackObserver::StreamPacketInfo> selected_feedback;
     for (const auto& packet : feedback_vector) {
diff --git a/modules/congestion_controller/rtp/transport_feedback_adapter.h b/modules/congestion_controller/rtp/transport_feedback_adapter.h
index 392e15c8fa..699c6ed489 100644
--- a/modules/congestion_controller/rtp/transport_feedback_adapter.h
+++ b/modules/congestion_controller/rtp/transport_feedback_adapter.h
@@ -87,33 +87,36 @@ class TransportFeedbackAdapter : public StreamFeedbackProvider {
 
   std::vector<PacketFeedback> ProcessTransportFeedbackInner(
       const rtcp::TransportFeedback& feedback,
-      Timestamp feedback_time);
+      Timestamp feedback_time) RTC_RUN_ON(&lock_);
 
   void SignalObservers(
       const std::vector<PacketFeedback>& packet_feedback_vector);
 
-  DataSize pending_untracked_size_ = DataSize::Zero();
-  Timestamp last_send_time_ = Timestamp::MinusInfinity();
-  Timestamp last_untracked_send_time_ = Timestamp::MinusInfinity();
-  SequenceNumberUnwrapper seq_num_unwrapper_;
-  std::map<int64_t, PacketFeedback> history_;
+  rtc::CriticalSection lock_;
+  DataSize pending_untracked_size_ RTC_GUARDED_BY(&lock_) = DataSize::Zero();
+  Timestamp last_send_time_ RTC_GUARDED_BY(&lock_) = Timestamp::MinusInfinity();
+  Timestamp last_untracked_send_time_ RTC_GUARDED_BY(&lock_) =
+      Timestamp::MinusInfinity();
+  SequenceNumberUnwrapper seq_num_unwrapper_ RTC_GUARDED_BY(&lock_);
+  std::map<int64_t, PacketFeedback> history_ RTC_GUARDED_BY(&lock_);
 
   // Sequence numbers are never negative, using -1 as it always < a real
   // sequence number.
-  int64_t last_ack_seq_num_ = -1;
-  InFlightBytesTracker in_flight_;
+  int64_t last_ack_seq_num_ RTC_GUARDED_BY(&lock_) = -1;
+  InFlightBytesTracker in_flight_ RTC_GUARDED_BY(&lock_);
 
-  Timestamp current_offset_ = Timestamp::MinusInfinity();
-  TimeDelta last_timestamp_ = TimeDelta::MinusInfinity();
+  Timestamp current_offset_ RTC_GUARDED_BY(&lock_) = Timestamp::MinusInfinity();
+  TimeDelta last_timestamp_ RTC_GUARDED_BY(&lock_) = TimeDelta::MinusInfinity();
 
-  uint16_t local_net_id_ = 0;
-  uint16_t remote_net_id_ = 0;
+  uint16_t local_net_id_ RTC_GUARDED_BY(&lock_) = 0;
+  uint16_t remote_net_id_ RTC_GUARDED_BY(&lock_) = 0;
 
+  rtc::CriticalSection observers_lock_;
   // Maps a set of ssrcs to corresponding observer. Vectors are used rather than
   // set/map to ensure that the processing order is consistent independently of
   // the randomized ssrcs.
   std::vector<std::pair<std::vector<uint32_t>, StreamFeedbackObserver*>>
-      observers_;
+      observers_ RTC_GUARDED_BY(&observers_lock_);
 };
 
 }  // namespace webrtc