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}
diff --git a/net/dcsctp/rx/data_tracker_test.cc b/net/dcsctp/rx/data_tracker_test.cc
index 07192fd..2abc444 100644
--- a/net/dcsctp/rx/data_tracker_test.cc
+++ b/net/dcsctp/rx/data_tracker_test.cc
@@ -42,7 +42,7 @@
}),
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)) {
diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc
index 32bcdaa..e47d055 100644
--- a/net/dcsctp/socket/dcsctp_socket.cc
+++ b/net/dcsctp/socket/dcsctp_socket.cc
@@ -921,7 +921,7 @@
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 @@
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 @@
}
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 @@
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
diff --git a/net/dcsctp/socket/dcsctp_socket.h b/net/dcsctp/socket/dcsctp_socket.h
index f91eb3e..2240ff6 100644
--- a/net/dcsctp/socket/dcsctp_socket.h
+++ b/net/dcsctp/socket/dcsctp_socket.h
@@ -155,9 +155,9 @@
// Closes the association, because of too many retransmission errors.
void CloseConnectionBecauseOfTooManyTransmissionErrors();
// Timer expiration handlers
- absl::optional<DurationMs> OnInitTimerExpiry();
- absl::optional<DurationMs> OnCookieTimerExpiry();
- absl::optional<DurationMs> OnShutdownTimerExpiry();
+ DurationMs OnInitTimerExpiry();
+ DurationMs OnCookieTimerExpiry();
+ 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
diff --git a/net/dcsctp/socket/heartbeat_handler.cc b/net/dcsctp/socket/heartbeat_handler.cc
index 902dff9..8d1d04a 100644
--- a/net/dcsctp/socket/heartbeat_handler.cc
+++ b/net/dcsctp/socket/heartbeat_handler.cc
@@ -164,7 +164,7 @@
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 @@
<< 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
diff --git a/net/dcsctp/socket/heartbeat_handler.h b/net/dcsctp/socket/heartbeat_handler.h
index 318b029..d232a49 100644
--- a/net/dcsctp/socket/heartbeat_handler.h
+++ b/net/dcsctp/socket/heartbeat_handler.h
@@ -50,8 +50,8 @@
void HandleHeartbeatAck(HeartbeatAckChunk chunk);
private:
- absl::optional<DurationMs> OnIntervalTimerExpiry();
- absl::optional<DurationMs> OnTimeoutTimerExpiry();
+ DurationMs OnIntervalTimerExpiry();
+ DurationMs OnTimeoutTimerExpiry();
const absl::string_view log_prefix_;
Context* ctx_;
diff --git a/net/dcsctp/socket/stream_reset_handler.cc b/net/dcsctp/socket/stream_reset_handler.cc
index 2094309..ac3f95b 100644
--- a/net/dcsctp/socket/stream_reset_handler.cc
+++ b/net/dcsctp/socket/stream_reset_handler.cc
@@ -347,13 +347,13 @@
}
}
-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
diff --git a/net/dcsctp/socket/stream_reset_handler.h b/net/dcsctp/socket/stream_reset_handler.h
index c335130..2f604f3 100644
--- a/net/dcsctp/socket/stream_reset_handler.h
+++ b/net/dcsctp/socket/stream_reset_handler.h
@@ -211,7 +211,7 @@
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_;
diff --git a/net/dcsctp/socket/stream_reset_handler_test.cc b/net/dcsctp/socket/stream_reset_handler_test.cc
index 091d717..4b12970 100644
--- a/net/dcsctp/socket/stream_reset_handler_test.cc
+++ b/net/dcsctp/socket/stream_reset_handler_test.cc
@@ -97,11 +97,11 @@
}),
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(),
diff --git a/net/dcsctp/socket/transmission_control_block.cc b/net/dcsctp/socket/transmission_control_block.cc
index 0621b48..c403b06 100644
--- a/net/dcsctp/socket/transmission_control_block.cc
+++ b/net/dcsctp/socket/transmission_control_block.cc
@@ -125,7 +125,7 @@
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 @@
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() {
diff --git a/net/dcsctp/socket/transmission_control_block.h b/net/dcsctp/socket/transmission_control_block.h
index 46a39d5..240454a 100644
--- a/net/dcsctp/socket/transmission_control_block.h
+++ b/net/dcsctp/socket/transmission_control_block.h
@@ -149,9 +149,9 @@
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_;
diff --git a/net/dcsctp/timer/timer.cc b/net/dcsctp/timer/timer.cc
index 208f26f..1c23d6d 100644
--- a/net/dcsctp/timer/timer.cc
+++ b/net/dcsctp/timer/timer.cc
@@ -109,9 +109,10 @@
timeout_->Start(duration, MakeTimeoutId(id_, generation_));
}
- absl::optional<DurationMs> new_duration = on_expired_();
- if (new_duration.has_value() && new_duration != duration_) {
- duration_ = new_duration.value();
+ DurationMs new_duration = on_expired_();
+ RTC_DCHECK(new_duration != DurationMs::InfiniteDuration());
+ if (new_duration > DurationMs(0) && new_duration != duration_) {
+ duration_ = new_duration;
if (is_running_) {
// Restart it with new duration.
timeout_->Stop();
diff --git a/net/dcsctp/timer/timer.h b/net/dcsctp/timer/timer.h
index 95aae57..6159737 100644
--- a/net/dcsctp/timer/timer.h
+++ b/net/dcsctp/timer/timer.h
@@ -102,10 +102,11 @@
// 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
- // will be set as `duration` and used as base duration when the timer is
- // restarted and as input to the backoff algorithm.
- using OnExpired = std::function<absl::optional<DurationMs>()>;
+ // When expired, the timer handler can optionally return a new non-zero
+ // duration which will be set as `duration` and used as base duration when the
+ // timer is restarted and as input to the backoff algorithm. If zero is
+ // 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;
diff --git a/net/dcsctp/timer/timer_test.cc b/net/dcsctp/timer/timer_test.cc
index 9387616..e0e210e 100644
--- a/net/dcsctp/timer/timer_test.cc
+++ b/net/dcsctp/timer/timer_test.cc
@@ -29,7 +29,7 @@
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 @@
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 @@
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 @@
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 @@
});
// 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 @@
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));
diff --git a/net/dcsctp/tx/retransmission_queue_test.cc b/net/dcsctp/tx/retransmission_queue_test.cc
index d50494f..096bb12 100644
--- a/net/dcsctp/tx/retransmission_queue_test.cc
+++ b/net/dcsctp/tx/retransmission_queue_test.cc
@@ -74,7 +74,7 @@
}),
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(
