Delay probes after route change until transport is writable

Ensure probes are not created until after the transport becomes writable even if the network route change.
DTLS negotiation must complete before there is a point in sending probes.

Bug: webrtc:14928
Change-Id: Ib3cb93aef9f38e306b094dd700ed595cf9eb3f32
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/301362
Reviewed-by: Diep Bui <diepbp@webrtc.org>
Commit-Queue: Per Kjellander <perkj@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#39870}

modules/congestion_controller/goog_cc/goog_cc_network_control_unittest.cc

@@ -23,6 +23,7 @@
 #include "test/gtest.h"
 #include "test/scenario/scenario.h"
 
+using ::testing::IsEmpty;
 using ::testing::NiceMock;
 
 namespace webrtc {
@@ -359,6 +360,28 @@ TEST(GoogCcNetworkControllerTest, ProbeOnRouteChange) {
   update = controller->OnProcessInterval({.at_time = current_time});
 }
 
+TEST(GoogCcNetworkControllerTest, ProbeAfterRouteChangeWhenTransportWritable) {
+  NetworkControllerTestFixture fixture;
+  std::unique_ptr<NetworkControllerInterface> controller =
+      fixture.CreateController();
+  Timestamp current_time = Timestamp::Millis(123);
+
+  NetworkControlUpdate update = controller->OnNetworkAvailability(
+      {.at_time = current_time, .network_available = false});
+  EXPECT_THAT(update.probe_cluster_configs, IsEmpty());
+
+  update = controller->OnNetworkRouteChange(
+      CreateRouteChange(current_time, 2 * kInitialBitrate, DataRate::Zero(),
+                        20 * kInitialBitrate));
+  // Transport is not writable. So not point in sending a probe.
+  EXPECT_THAT(update.probe_cluster_configs, IsEmpty());
+
+  // Probe is sent when transport becomes writable.
+  update = controller->OnNetworkAvailability(
+      {.at_time = current_time, .network_available = true});
+  EXPECT_THAT(update.probe_cluster_configs, Not(IsEmpty()));
+}
+
 // Bandwidth estimation is updated when feedbacks are received.
 // Feedbacks which show an increasing delay cause the estimation to be reduced.
 TEST(GoogCcNetworkControllerTest, UpdatesDelayBasedEstimate) {

modules/congestion_controller/goog_cc/probe_controller.cc

@@ -157,7 +157,8 @@ ProbeControllerConfig::~ProbeControllerConfig() = default;
 
 ProbeController::ProbeController(const FieldTrialsView* key_value_config,
                                  RtcEventLog* event_log)
-    : enable_periodic_alr_probing_(false),
+    : network_available_(true),
+      enable_periodic_alr_probing_(false),
       in_rapid_recovery_experiment_(absl::StartsWith(
           key_value_config->Lookup(kBweRapidRecoveryExperiment),
           "Enabled")),
@@ -368,7 +369,6 @@ void ProbeController::SetNetworkStateEstimate(
 }
 
 void ProbeController::Reset(Timestamp at_time) {
-  network_available_ = true;
   bandwidth_limited_cause_ = BandwidthLimitedCause::kDelayBasedLimited;
   state_ = State::kInit;
   min_bitrate_to_probe_further_ = DataRate::PlusInfinity();

modules/congestion_controller/goog_cc/probe_controller.h

@@ -136,7 +136,8 @@ class ProbeController {
   void SetNetworkStateEstimate(webrtc::NetworkStateEstimate estimate);
 
   // Resets the ProbeController to a state equivalent to as if it was just
-  // created EXCEPT for `enable_periodic_alr_probing_`.
+  // created EXCEPT for `enable_periodic_alr_probing_` and
+  // `network_available_`.
   void Reset(Timestamp at_time);
 
   ABSL_MUST_USE_RESULT std::vector<ProbeClusterConfig> Process(

modules/congestion_controller/goog_cc/probe_controller_unittest.cc

@@ -21,6 +21,7 @@
 #include "test/gmock.h"
 #include "test/gtest.h"
 
+using ::testing::IsEmpty;
 using ::testing::NiceMock;
 
 namespace webrtc {
@@ -428,6 +429,34 @@ TEST(ProbeControllerTest, PeriodicProbingAfterReset) {
   EXPECT_EQ(probes[0].target_data_rate, kStartBitrate * 2);
 }
 
+TEST(ProbeControllerTest, NoProbesWhenTransportIsNotWritable) {
+  ProbeControllerFixture fixture;
+  std::unique_ptr<ProbeController> probe_controller =
+      fixture.CreateController();
+  probe_controller->EnablePeriodicAlrProbing(true);
+
+  std::vector<ProbeClusterConfig> probes =
+      probe_controller->OnNetworkAvailability({.network_available = false});
+  EXPECT_THAT(probes, IsEmpty());
+  EXPECT_THAT(probe_controller->SetBitrates(kMinBitrate, kStartBitrate,
+                                            kMaxBitrate, fixture.CurrentTime()),
+              IsEmpty());
+  fixture.AdvanceTime(TimeDelta::Seconds(10));
+  EXPECT_THAT(probe_controller->Process(fixture.CurrentTime()), IsEmpty());
+
+  // Controller is reset after a network route change.
+  // But, a probe should not be sent since the transport is not writable.
+  // Transport is not writable until after DTLS negotiation completes.
+  // However, the bitrate constraints may change.
+  probe_controller->Reset(fixture.CurrentTime());
+  EXPECT_THAT(
+      probe_controller->SetBitrates(2 * kMinBitrate, 2 * kStartBitrate,
+                                    2 * kMaxBitrate, fixture.CurrentTime()),
+      IsEmpty());
+  fixture.AdvanceTime(TimeDelta::Seconds(10));
+  EXPECT_THAT(probe_controller->Process(fixture.CurrentTime()), IsEmpty());
+}
+
 TEST(ProbeControllerTest, TestExponentialProbingOverflow) {
   ProbeControllerFixture fixture;
   std::unique_ptr<ProbeController> probe_controller =