dcsctp: Simplify interface for unchanged timeout

When a timer expires, it can optionally return a new expiration value.
Clearly, that value can't be zero, as that would make it expire
immediately again.

To simplify the interface, and make it easier to migrate to
rtc::TimeDelta, change it from an optional value to an always-present
value that - if zero - means that the expiration time should be
unchanged.

This is just an internal refactoring, and not part of any external
interface.

Bug: webrtc:15593
Change-Id: I6e7010d2dbe774ccb260e14fd6b9861c319e2411
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/325281
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41045}

net/dcsctp/rx/data_tracker_test.cc

@@ -42,7 +42,7 @@ class DataTrackerTest : public testing::Test {
         }),
         timer_(timer_manager_.CreateTimer(
             "test/delayed_ack",
-            []() { return absl::nullopt; },
+            []() { return DurationMs(0); },
             TimerOptions(DurationMs(0)))),
         tracker_(
             std::make_unique<DataTracker>("log: ", timer_.get(), kInitialTSN)) {

net/dcsctp/socket/dcsctp_socket.cc

@@ -921,7 +921,7 @@ bool DcSctpSocket::HandleUnrecognizedChunk(
   return continue_processing;
 }
 
-absl::optional<DurationMs> DcSctpSocket::OnInitTimerExpiry() {
+DurationMs DcSctpSocket::OnInitTimerExpiry() {
   RTC_DLOG(LS_VERBOSE) << log_prefix() << "Timer " << t1_init_->name()
                        << " has expired: " << t1_init_->expiration_count()
                        << "/" << t1_init_->options().max_restarts.value_or(-1);
@@ -933,10 +933,10 @@ absl::optional<DurationMs> DcSctpSocket::OnInitTimerExpiry() {
     InternalClose(ErrorKind::kTooManyRetries, "No INIT_ACK received");
   }
   RTC_DCHECK(IsConsistent());
-  return absl::nullopt;
+  return DurationMs(0);
 }
 
-absl::optional<DurationMs> DcSctpSocket::OnCookieTimerExpiry() {
+DurationMs DcSctpSocket::OnCookieTimerExpiry() {
   // https://tools.ietf.org/html/rfc4960#section-4
   // "If the T1-cookie timer expires, the endpoint MUST retransmit COOKIE
   // ECHO and restart the T1-cookie timer without changing state.  This MUST
@@ -957,10 +957,10 @@ absl::optional<DurationMs> DcSctpSocket::OnCookieTimerExpiry() {
   }
 
   RTC_DCHECK(IsConsistent());
-  return absl::nullopt;
+  return DurationMs(0);
 }
 
-absl::optional<DurationMs> DcSctpSocket::OnShutdownTimerExpiry() {
+DurationMs DcSctpSocket::OnShutdownTimerExpiry() {
   RTC_DLOG(LS_VERBOSE) << log_prefix() << "Timer " << t2_shutdown_->name()
                        << " has expired: " << t2_shutdown_->expiration_count()
                        << "/"
@@ -980,7 +980,7 @@ absl::optional<DurationMs> DcSctpSocket::OnShutdownTimerExpiry() {
 
     InternalClose(ErrorKind::kTooManyRetries, "No SHUTDOWN_ACK received");
     RTC_DCHECK(IsConsistent());
-    return absl::nullopt;
+    return DurationMs(0);
   }
 
   // https://tools.ietf.org/html/rfc4960#section-9.2

net/dcsctp/socket/dcsctp_socket.h

@@ -155,9 +155,9 @@ class DcSctpSocket : public DcSctpSocketInterface {
   // Closes the association, because of too many retransmission errors.
   void CloseConnectionBecauseOfTooManyTransmissionErrors();
   // Timer expiration handlers
-  absl::optional<DurationMs> OnInitTimerExpiry();
+  DurationMs OnInitTimerExpiry();
-  absl::optional<DurationMs> OnCookieTimerExpiry();
+  DurationMs OnCookieTimerExpiry();
-  absl::optional<DurationMs> OnShutdownTimerExpiry();
+  DurationMs OnShutdownTimerExpiry();
   void OnSentPacket(rtc::ArrayView<const uint8_t> packet,
                     SendPacketStatus status);
   // Sends SHUTDOWN or SHUTDOWN-ACK if the socket is shutting down and if all

net/dcsctp/socket/heartbeat_handler.cc

@@ -164,7 +164,7 @@ void HeartbeatHandler::HandleHeartbeatAck(HeartbeatAckChunk chunk) {
   ctx_->ClearTxErrorCounter();
 }
 
-absl::optional<DurationMs> HeartbeatHandler::OnIntervalTimerExpiry() {
+DurationMs HeartbeatHandler::OnIntervalTimerExpiry() {
   if (ctx_->is_connection_established()) {
     HeartbeatInfo info(ctx_->callbacks().TimeMillis());
     timeout_timer_->set_duration(ctx_->current_rto());
@@ -183,14 +183,14 @@ absl::optional<DurationMs> HeartbeatHandler::OnIntervalTimerExpiry() {
         << log_prefix_
         << "Will not send HEARTBEAT when connection not established";
   }
-  return absl::nullopt;
+  return DurationMs(0);
 }
 
-absl::optional<DurationMs> HeartbeatHandler::OnTimeoutTimerExpiry() {
+DurationMs HeartbeatHandler::OnTimeoutTimerExpiry() {
   // Note that the timeout timer is not restarted. It will be started again when
   // the interval timer expires.
   RTC_DCHECK(!timeout_timer_->is_running());
   ctx_->IncrementTxErrorCounter("HEARTBEAT timeout");
-  return absl::nullopt;
+  return DurationMs(0);
 }
 }  // namespace dcsctp

net/dcsctp/socket/heartbeat_handler.h

@@ -50,8 +50,8 @@ class HeartbeatHandler {
   void HandleHeartbeatAck(HeartbeatAckChunk chunk);
 
  private:
-  absl::optional<DurationMs> OnIntervalTimerExpiry();
+  DurationMs OnIntervalTimerExpiry();
-  absl::optional<DurationMs> OnTimeoutTimerExpiry();
+  DurationMs OnTimeoutTimerExpiry();
 
   const absl::string_view log_prefix_;
   Context* ctx_;

net/dcsctp/socket/stream_reset_handler.cc

@@ -347,13 +347,13 @@ void StreamResetHandler::ResetStreams(
   }
 }
 
-absl::optional<DurationMs> StreamResetHandler::OnReconfigTimerExpiry() {
+DurationMs StreamResetHandler::OnReconfigTimerExpiry() {
   if (current_request_->has_been_sent()) {
     // There is an outstanding request, which timed out while waiting for a
     // response.
     if (!ctx_->IncrementTxErrorCounter("RECONFIG timeout")) {
       // Timed out. The connection will close after processing the timers.
-      return absl::nullopt;
+      return DurationMs(0);
     }
   } else {
     // There is no outstanding request, but there is a prepared one. This means

net/dcsctp/socket/stream_reset_handler.h

@@ -211,7 +211,7 @@ class StreamResetHandler {
   void HandleResponse(const ParameterDescriptor& descriptor);
 
   // Expiration handler for the Reconfig timer.
-  absl::optional<DurationMs> OnReconfigTimerExpiry();
+  DurationMs OnReconfigTimerExpiry();
 
   const absl::string_view log_prefix_;
   Context* ctx_;

net/dcsctp/socket/stream_reset_handler_test.cc

@@ -97,11 +97,11 @@ class StreamResetHandlerTest : public testing::Test {
         }),
         delayed_ack_timer_(timer_manager_.CreateTimer(
             "test/delayed_ack",
-            []() { return absl::nullopt; },
+            []() { return DurationMs(0); },
             TimerOptions(DurationMs(0)))),
         t3_rtx_timer_(timer_manager_.CreateTimer(
             "test/t3_rtx",
-            []() { return absl::nullopt; },
+            []() { return DurationMs(0); },
             TimerOptions(DurationMs(0)))),
         data_tracker_(std::make_unique<DataTracker>("log: ",
                                                     delayed_ack_timer_.get(),

net/dcsctp/socket/transmission_control_block.cc

@@ -125,7 +125,7 @@ void TransmissionControlBlock::ObserveRTT(DurationMs rtt) {
   delayed_ack_timer_->set_duration(delayed_ack_tmo);
 }
 
-absl::optional<DurationMs> TransmissionControlBlock::OnRtxTimerExpiry() {
+DurationMs TransmissionControlBlock::OnRtxTimerExpiry() {
   TimeMs now = callbacks_.TimeMillis();
   RTC_DLOG(LS_INFO) << log_prefix_ << "Timer " << t3_rtx_->name()
                     << " has expired";
@@ -139,13 +139,13 @@ absl::optional<DurationMs> TransmissionControlBlock::OnRtxTimerExpiry() {
       SendBufferedPackets(now);
     }
   }
-  return absl::nullopt;
+  return DurationMs(0);
 }
 
-absl::optional<DurationMs> TransmissionControlBlock::OnDelayedAckTimerExpiry() {
+DurationMs TransmissionControlBlock::OnDelayedAckTimerExpiry() {
   data_tracker_.HandleDelayedAckTimerExpiry();
   MaybeSendSack();
-  return absl::nullopt;
+  return DurationMs(0);
 }
 
 void TransmissionControlBlock::MaybeSendSack() {

net/dcsctp/socket/transmission_control_block.h

@@ -149,9 +149,9 @@ class TransmissionControlBlock : public Context {
 
  private:
   // Will be called when the retransmission timer (t3-rtx) expires.
-  absl::optional<DurationMs> OnRtxTimerExpiry();
+  DurationMs OnRtxTimerExpiry();
   // Will be called when the delayed ack timer expires.
-  absl::optional<DurationMs> OnDelayedAckTimerExpiry();
+  DurationMs OnDelayedAckTimerExpiry();
 
   const absl::string_view log_prefix_;
   const DcSctpOptions options_;

net/dcsctp/timer/timer.cc

@@ -109,9 +109,10 @@ void Timer::Trigger(TimerGeneration generation) {
       timeout_->Start(duration, MakeTimeoutId(id_, generation_));
     }
 
-    absl::optional<DurationMs> new_duration = on_expired_();
+    DurationMs new_duration = on_expired_();
-    if (new_duration.has_value() && new_duration != duration_) {
+    RTC_DCHECK(new_duration != DurationMs::InfiniteDuration());
-      duration_ = new_duration.value();
+    if (new_duration > DurationMs(0) && new_duration != duration_) {
+      duration_ = new_duration;
       if (is_running_) {
         // Restart it with new duration.
         timeout_->Stop();

net/dcsctp/timer/timer.h

@@ -102,10 +102,11 @@ class Timer {
   // The maximum timer duration - one day.
   static constexpr DurationMs kMaxTimerDuration = DurationMs(24 * 3600 * 1000);
 
-  // When expired, the timer handler can optionally return a new duration which
+  // When expired, the timer handler can optionally return a new non-zero
-  // will be set as `duration` and used as base duration when the timer is
+  // duration which will be set as `duration` and used as base duration when the
-  // restarted and as input to the backoff algorithm.
+  // timer is restarted and as input to the backoff algorithm. If zero is
-  using OnExpired = std::function<absl::optional<DurationMs>()>;
+  // returned, the current duration is used.
+  using OnExpired = std::function<DurationMs()>;
 
   // TimerManager will have pointers to these instances, so they must not move.
   Timer(const Timer&) = delete;

net/dcsctp/timer/timer_test.cc

@@ -29,7 +29,7 @@ class TimerTest : public testing::Test {
         manager_([this](webrtc::TaskQueueBase::DelayPrecision precision) {
           return timeout_manager_.CreateTimeout(precision);
         }) {
-    ON_CALL(on_expired_, Call).WillByDefault(Return(absl::nullopt));
+    ON_CALL(on_expired_, Call).WillByDefault(Return(DurationMs(0)));
   }
 
   void AdvanceTimeAndRunTimers(DurationMs duration) {
@@ -48,7 +48,7 @@ class TimerTest : public testing::Test {
   TimeMs now_ = TimeMs(0);
   FakeTimeoutManager timeout_manager_;
   TimerManager manager_;
-  testing::MockFunction<absl::optional<DurationMs>()> on_expired_;
+  testing::MockFunction<DurationMs()> on_expired_;
 };
 
 TEST_F(TimerTest, TimerIsInitiallyStopped) {
@@ -366,7 +366,7 @@ TEST_F(TimerTest, TimerCanBeStartedFromWithinExpirationHandler) {
     EXPECT_TRUE(t1->is_running());
     t1->set_duration(DurationMs(5000));
     t1->Start();
-    return absl::nullopt;
+    return DurationMs(0);
   });
   AdvanceTimeAndRunTimers(DurationMs(1000));
 
@@ -378,7 +378,7 @@ TEST_F(TimerTest, TimerCanBeStartedFromWithinExpirationHandler) {
     EXPECT_TRUE(t1->is_running());
     t1->set_duration(DurationMs(5000));
     t1->Start();
-    return absl::make_optional(DurationMs(8000));
+    return DurationMs(8000);
   });
   AdvanceTimeAndRunTimers(DurationMs(1));
 
@@ -435,12 +435,12 @@ TEST(TimerManagerTest, TimerManagerPassesPrecisionToCreateTimeoutMethod) {
   });
   // Default TimerOptions.
   manager.CreateTimer(
-      "test_timer", []() { return absl::optional<DurationMs>(); },
+      "test_timer", []() { return DurationMs(0); },
       TimerOptions(DurationMs(123)));
   EXPECT_EQ(create_timer_precison, webrtc::TaskQueueBase::DelayPrecision::kLow);
   // High precision TimerOptions.
   manager.CreateTimer(
-      "test_timer", []() { return absl::optional<DurationMs>(); },
+      "test_timer", []() { return DurationMs(0); },
       TimerOptions(DurationMs(123), TimerBackoffAlgorithm::kExponential,
                    absl::nullopt, DurationMs::InfiniteDuration(),
                    webrtc::TaskQueueBase::DelayPrecision::kHigh));
@@ -448,7 +448,7 @@ TEST(TimerManagerTest, TimerManagerPassesPrecisionToCreateTimeoutMethod) {
             webrtc::TaskQueueBase::DelayPrecision::kHigh);
   // Low precision TimerOptions.
   manager.CreateTimer(
-      "test_timer", []() { return absl::optional<DurationMs>(); },
+      "test_timer", []() { return DurationMs(0); },
       TimerOptions(DurationMs(123), TimerBackoffAlgorithm::kExponential,
                    absl::nullopt, DurationMs::InfiniteDuration(),
                    webrtc::TaskQueueBase::DelayPrecision::kLow));

net/dcsctp/tx/retransmission_queue_test.cc

@@ -74,7 +74,7 @@ class RetransmissionQueueTest : public testing::Test {
         }),
         timer_(timer_manager_.CreateTimer(
             "test/t3_rtx",
-            []() { return absl::nullopt; },
+            []() { return DurationMs(0); },
             TimerOptions(options_.rto_initial))) {}
 
   std::function<SendQueue::DataToSend(TimeMs, size_t)> CreateChunk(