Fix exponential probing in ProbeController.

https://codereview.webrtc.org/2504023002 broke exponential probing.
After that change ProbeController stops exponential probes prematurely:
it goes to kProbingComplete state if SetEstimatedBitrate() is called
with bitrate lower than min_bitrate_to_probe_further_bps_, which always
happens with the first pair of probes. As result it wasn't sending
repeated probes as it should. This change fixes that issue by moving
probe expieration logic to ProbeContoller::Process(). This also ensures
that the controller goes to kProbingComplete state as soon as probing
timeout expired, without waiting for the next SetEstimatedBitrate()
call.

BUG=669421

Review-Url: https://codereview.webrtc.org/2546613003
Cr-Commit-Position: refs/heads/master@{#15392}

webrtc/modules/congestion_controller/probe_controller.cc

@@ -121,28 +121,17 @@ void ProbeController::SetEstimatedBitrate(int bitrate_bps) {
   int64_t now_ms = clock_->TimeInMilliseconds();
 
   if (state_ == State::kWaitingForProbingResult) {
-    if ((now_ms - time_last_probing_initiated_ms_) >
+    // Continue probing if probing results indicate channel has greater
-        kMaxWaitingTimeForProbingResultMs) {
+    // capacity.
-      LOG(LS_INFO) << "kWaitingForProbingResult: timeout";
+    LOG(LS_INFO) << "Measured bitrate: " << bitrate_bps
-      state_ = State::kProbingComplete;
+                 << " Minimum to probe further: "
-      min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled;
+                 << min_bitrate_to_probe_further_bps_;
-    } else {
+    if (min_bitrate_to_probe_further_bps_ != kExponentialProbingDisabled &&
-      // Continue probing if probing results indicate channel has greater
+        bitrate_bps > min_bitrate_to_probe_further_bps_) {
-      // capacity.
+      // Double the probing bitrate and expect a minimum of 25% gain to
-      LOG(LS_INFO) << "Measured bitrate: " << bitrate_bps
+      // continue probing.
-                   << " Minimum to probe further: "
+      InitiateProbing(now_ms, {2 * bitrate_bps},
-                   << min_bitrate_to_probe_further_bps_;
+                      bitrate_bps * kRepeatedProbeMinPercentage / 100);
-      if (min_bitrate_to_probe_further_bps_ != kExponentialProbingDisabled &&
-          bitrate_bps > min_bitrate_to_probe_further_bps_) {
-        // Double the probing bitrate and expect a minimum of 25% gain to
-        // continue probing.
-        InitiateProbing(now_ms, {2 * bitrate_bps},
-                        bitrate_bps * kRepeatedProbeMinPercentage / 100);
-      } else {
-        // Stop exponential probing.
-        state_ = State::kProbingComplete;
-        min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled;
-      }
     }
   }
 
@@ -182,6 +171,16 @@ void ProbeController::EnablePeriodicAlrProbing(bool enable) {
 void ProbeController::Process() {
   rtc::CritScope cs(&critsect_);
 
+  int64_t now_ms = clock_->TimeInMilliseconds();
+
+  if (state_ == State::kWaitingForProbingResult &&
+      (now_ms - time_last_probing_initiated_ms_) >
+          kMaxWaitingTimeForProbingResultMs) {
+    LOG(LS_INFO) << "kWaitingForProbingResult: timeout";
+    state_ = State::kProbingComplete;
+    min_bitrate_to_probe_further_bps_ = kExponentialProbingDisabled;
+  }
+
   if (state_ != State::kProbingComplete || !enable_periodic_alr_probing_)
     return;
 
@@ -189,7 +188,6 @@ void ProbeController::Process() {
   rtc::Optional<int64_t> alr_start_time =
       pacer_->GetApplicationLimitedRegionStartTime();
   if (alr_start_time) {
-    int64_t now_ms = clock_->TimeInMilliseconds();
     int64_t next_probe_time_ms =
         std::max(*alr_start_time, time_last_probing_initiated_ms_) +
         kAlrPeriodicProbingIntervalMs;

webrtc/modules/congestion_controller/probe_controller_unittest.cc

@@ -72,6 +72,7 @@ TEST_F(ProbeControllerTest, InitiatesProbingOnMaxBitrateIncrease) {
   // Long enough to time out exponential probing.
   clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs);
   probe_controller_->SetEstimatedBitrate(kStartBitrateBps);
+  probe_controller_->Process();
 
   EXPECT_CALL(pacer_, CreateProbeCluster(kMaxBitrateBps + 100, _));
   probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps,
@@ -82,6 +83,12 @@ TEST_F(ProbeControllerTest, TestExponentialProbing) {
   probe_controller_->SetBitrates(kMinBitrateBps, kStartBitrateBps,
                                  kMaxBitrateBps);
 
+  // Repeated probe should only be sent when estimated bitrate climbs above 4 *
+  // kStartBitrateBps = 1200.
+  EXPECT_CALL(pacer_, CreateProbeCluster(_, _)).Times(0);
+  probe_controller_->SetEstimatedBitrate(1000);
+  testing::Mock::VerifyAndClearExpectations(&pacer_);
+
   EXPECT_CALL(pacer_, CreateProbeCluster(2 * 1800, _));
   probe_controller_->SetEstimatedBitrate(1800);
 }
@@ -92,7 +99,9 @@ TEST_F(ProbeControllerTest, TestExponentialProbingTimeout) {
 
   // Advance far enough to cause a time out in waiting for probing result.
   clock_.AdvanceTimeMilliseconds(kExponentialProbingTimeoutMs);
-  EXPECT_CALL(pacer_, CreateProbeCluster(2 * 1800, _)).Times(0);
+  probe_controller_->Process();
+
+  EXPECT_CALL(pacer_, CreateProbeCluster(_, _)).Times(0);
   probe_controller_->SetEstimatedBitrate(1800);
 }
 
@@ -110,6 +119,7 @@ TEST_F(ProbeControllerTest, ProbeAfterEstimateDropInAlr) {
       .WillRepeatedly(
           Return(rtc::Optional<int64_t>(clock_.TimeInMilliseconds())));
   clock_.AdvanceTimeMilliseconds(kAlrProbeInterval + 1);
+  probe_controller_->Process();
   probe_controller_->SetEstimatedBitrate(50);
 }