From b951dc6f4c856001f7adcb79158e0f1f1bd8efba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Wed, 26 Jan 2022 18:38:13 +0100 Subject: [PATCH] Allow specifying delayed task precision of dcsctp::Timer. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Context: The timer precision of PostDelayedTask() is about to be lowered to include up to 17 ms leeway. In order not to break use cases that require high precision timers, PostDelayedHighPrecisionTask() will continue to have the same precision that PostDelayedTask() has today. webrtc::TaskQueueBase has an enum (kLow, kHigh) to decide which precision to use when calling PostDelayedTaskWithPrecision(). See go/postdelayedtask-precision-in-webrtc for motivation and a table of delayed task use cases in WebRTC that are "high" or "low" precision. Most timers in DCSCTP are believed to only be needing low precision (see table), but the delayed_ack_timer_ of DataTracker[1] is an example of a use case that is likely to break if the timer precision is lowered (if ACK is sent too late, retransmissions may occur). So this is considered a high precision use case. This CL makes it possible to specify the precision of dcsctp::Timer. In a follow-up CL we will update delayed_ack_timer_ to kHigh precision. [1] https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/net/dcsctp/rx/data_tracker.cc;l=340 Bug: webrtc:13604 Change-Id: I8eec5ce37044096978b5dd1985fbb00bc0d8fb7e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249081 Reviewed-by: Victor Boivie Reviewed-by: Tomas Gunnarsson Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#35809} --- api/task_queue/test/BUILD.gn | 18 +++++++++ api/task_queue/test/mock_task_queue_base.h | 32 ++++++++++++++++ media/BUILD.gn | 1 + media/sctp/dcsctp_transport.cc | 5 ++- media/sctp/dcsctp_transport.h | 4 +- net/dcsctp/fuzzers/BUILD.gn | 1 + net/dcsctp/fuzzers/dcsctp_fuzzers.h | 5 ++- net/dcsctp/public/BUILD.gn | 4 +- net/dcsctp/public/dcsctp_socket.h | 20 +++++++++- net/dcsctp/rx/BUILD.gn | 1 + net/dcsctp/rx/data_tracker_test.cc | 5 ++- net/dcsctp/socket/BUILD.gn | 3 ++ net/dcsctp/socket/callback_deferrer.cc | 5 ++- net/dcsctp/socket/callback_deferrer.h | 4 +- net/dcsctp/socket/dcsctp_socket.cc | 5 ++- .../socket/dcsctp_socket_network_test.cc | 6 ++- net/dcsctp/socket/heartbeat_handler_test.cc | 5 ++- .../socket/mock_dcsctp_socket_callbacks.h | 5 ++- .../socket/stream_reset_handler_test.cc | 5 ++- net/dcsctp/timer/BUILD.gn | 3 ++ net/dcsctp/timer/fake_timeout.h | 12 ++++-- net/dcsctp/timer/task_queue_timeout.cc | 7 +++- net/dcsctp/timer/task_queue_timeout.h | 10 +++-- net/dcsctp/timer/task_queue_timeout_test.cc | 38 +++++++++++++++++++ net/dcsctp/timer/timer.cc | 2 +- net/dcsctp/timer/timer.h | 23 +++++++++-- net/dcsctp/timer/timer_test.cc | 34 ++++++++++++++++- net/dcsctp/tx/BUILD.gn | 1 + net/dcsctp/tx/retransmission_queue_test.cc | 5 ++- 29 files changed, 238 insertions(+), 31 deletions(-) create mode 100644 api/task_queue/test/BUILD.gn create mode 100644 api/task_queue/test/mock_task_queue_base.h diff --git a/api/task_queue/test/BUILD.gn b/api/task_queue/test/BUILD.gn new file mode 100644 index 0000000000..fa82dd090e --- /dev/null +++ b/api/task_queue/test/BUILD.gn @@ -0,0 +1,18 @@ +# Copyright (c) 2018 The WebRTC project authors. All Rights Reserved. +# +# Use of this source code is governed by a BSD-style license +# that can be found in the LICENSE file in the root of the source +# tree. An additional intellectual property rights grant can be found +# in the file PATENTS. All contributing project authors may +# be found in the AUTHORS file in the root of the source tree. + +import("../../../webrtc.gni") + +rtc_library("mock_task_queue_base") { + testonly = true + sources = [ "mock_task_queue_base.h" ] + deps = [ + "../../../api/task_queue:task_queue", + "../../../test:test_support", + ] +} diff --git a/api/task_queue/test/mock_task_queue_base.h b/api/task_queue/test/mock_task_queue_base.h new file mode 100644 index 0000000000..68c5c052ed --- /dev/null +++ b/api/task_queue/test/mock_task_queue_base.h @@ -0,0 +1,32 @@ +/* + * Copyright 2022 The WebRTC Project Authors. All rights reserved. + * + * Use of this source code is governed by a BSD-style license + * that can be found in the LICENSE file in the root of the source + * tree. An additional intellectual property rights grant can be found + * in the file PATENTS. All contributing project authors may + * be found in the AUTHORS file in the root of the source tree. + */ + +#ifndef API_TASK_QUEUE_TEST_MOCK_TASK_QUEUE_BASE_H_ +#define API_TASK_QUEUE_TEST_MOCK_TASK_QUEUE_BASE_H_ + +#include + +#include "api/task_queue/task_queue_base.h" +#include "test/gmock.h" + +namespace webrtc { + +class MockTaskQueueBase : public TaskQueueBase { + public: + MOCK_METHOD0(Delete, void()); + MOCK_METHOD1(PostTask, void(std::unique_ptr)); + MOCK_METHOD2(PostDelayedTask, void(std::unique_ptr, uint32_t)); + MOCK_METHOD2(PostDelayedHighPrecisionTask, + void(std::unique_ptr, uint32_t)); +}; + +} // namespace webrtc + +#endif // API_TASK_QUEUE_TEST_MOCK_TASK_QUEUE_BASE_H_ diff --git a/media/BUILD.gn b/media/BUILD.gn index 6ae156b108..493b883949 100644 --- a/media/BUILD.gn +++ b/media/BUILD.gn @@ -392,6 +392,7 @@ if (rtc_build_dcsctp) { deps = [ ":rtc_data_sctp_transport_internal", "../api:array_view", + "../api/task_queue:task_queue", "../media:rtc_media_base", "../net/dcsctp/public:factory", "../net/dcsctp/public:socket", diff --git a/media/sctp/dcsctp_transport.cc b/media/sctp/dcsctp_transport.cc index f1dc1640dc..38d2799f36 100644 --- a/media/sctp/dcsctp_transport.cc +++ b/media/sctp/dcsctp_transport.cc @@ -359,8 +359,9 @@ SendPacketStatus DcSctpTransport::SendPacketWithStatus( return SendPacketStatus::kSuccess; } -std::unique_ptr DcSctpTransport::CreateTimeout() { - return task_queue_timeout_factory_.CreateTimeout(); +std::unique_ptr DcSctpTransport::CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) { + return task_queue_timeout_factory_.CreateTimeout(precision); } dcsctp::TimeMs DcSctpTransport::TimeMillis() { diff --git a/media/sctp/dcsctp_transport.h b/media/sctp/dcsctp_transport.h index c8c5199396..11c2f829c5 100644 --- a/media/sctp/dcsctp_transport.h +++ b/media/sctp/dcsctp_transport.h @@ -17,6 +17,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "media/sctp/sctp_transport_internal.h" #include "net/dcsctp/public/dcsctp_options.h" #include "net/dcsctp/public/dcsctp_socket.h" @@ -61,7 +62,8 @@ class DcSctpTransport : public cricket::SctpTransportInternal, // dcsctp::DcSctpSocketCallbacks dcsctp::SendPacketStatus SendPacketWithStatus( rtc::ArrayView data) override; - std::unique_ptr CreateTimeout() override; + std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) override; dcsctp::TimeMs TimeMillis() override; uint32_t GetRandomInt(uint32_t low, uint32_t high) override; void OnTotalBufferedAmountLow() override; diff --git a/net/dcsctp/fuzzers/BUILD.gn b/net/dcsctp/fuzzers/BUILD.gn index 9edbae44d7..c00a02251c 100644 --- a/net/dcsctp/fuzzers/BUILD.gn +++ b/net/dcsctp/fuzzers/BUILD.gn @@ -12,6 +12,7 @@ rtc_library("dcsctp_fuzzers") { testonly = true deps = [ "../../../api:array_view", + "../../../api/task_queue:task_queue", "../../../rtc_base", "../../../rtc_base:checks", "../../../rtc_base:rtc_base_approved", diff --git a/net/dcsctp/fuzzers/dcsctp_fuzzers.h b/net/dcsctp/fuzzers/dcsctp_fuzzers.h index f3de0722f4..90cfa35099 100644 --- a/net/dcsctp/fuzzers/dcsctp_fuzzers.h +++ b/net/dcsctp/fuzzers/dcsctp_fuzzers.h @@ -16,6 +16,7 @@ #include #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/public/dcsctp_socket.h" namespace dcsctp { @@ -58,7 +59,9 @@ class FuzzerCallbacks : public DcSctpSocketCallbacks { void SendPacket(rtc::ArrayView data) override { sent_packets_.emplace_back(std::vector(data.begin(), data.end())); } - std::unique_ptr CreateTimeout() override { + std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) override { + // The fuzzer timeouts don't implement |precision|. return std::make_unique(active_timeouts_); } TimeMs TimeMillis() override { return TimeMs(42); } diff --git a/net/dcsctp/public/BUILD.gn b/net/dcsctp/public/BUILD.gn index b07ca1b3d8..4fad23e9b8 100644 --- a/net/dcsctp/public/BUILD.gn +++ b/net/dcsctp/public/BUILD.gn @@ -10,8 +10,8 @@ import("../../../webrtc.gni") rtc_source_set("types") { deps = [ - "../../../rtc_base:rtc_base_approved", "../../../api:array_view", + "../../../rtc_base:rtc_base_approved", ] sources = [ "dcsctp_message.h", @@ -23,9 +23,9 @@ rtc_source_set("types") { rtc_source_set("socket") { deps = [ - "../../../rtc_base:rtc_base_approved", ":types", "../../../api:array_view", + "../../../api/task_queue:task_queue", "../../../rtc_base", "../../../rtc_base:checks", "../../../rtc_base:rtc_base_approved", diff --git a/net/dcsctp/public/dcsctp_socket.h b/net/dcsctp/public/dcsctp_socket.h index 2b56094ac1..7001585389 100644 --- a/net/dcsctp/public/dcsctp_socket.h +++ b/net/dcsctp/public/dcsctp_socket.h @@ -17,6 +17,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/public/dcsctp_handover_state.h" #include "net/dcsctp/public/dcsctp_message.h" #include "net/dcsctp/public/dcsctp_options.h" @@ -264,9 +265,26 @@ class DcSctpSocketCallbacks { // Called when the library wants to create a Timeout. The callback must return // an object that implements that interface. // + // Low precision tasks are scheduled more efficiently by using leeway to + // reduce Idle Wake Ups and is the preferred precision whenever possible. High + // precision timeouts do not have this leeway, but is still limited by OS + // timer precision. At the time of writing, kLow's additional leeway may be up + // to 17 ms, but please see webrtc::TaskQueueBase::DelayPrecision for + // up-to-date information. + // // Note that it's NOT ALLOWED to call into this library from within this // callback. - virtual std::unique_ptr CreateTimeout() = 0; + virtual std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) { + // TODO(hbos): When dependencies have migrated to this new signature, make + // this pure virtual and delete the other version. + return CreateTimeout(); + } + // TODO(hbos): When dependencies have migrated to the other signature, delete + // this version. + virtual std::unique_ptr CreateTimeout() { + return CreateTimeout(webrtc::TaskQueueBase::DelayPrecision::kLow); + } // Returns the current time in milliseconds (from any epoch). // diff --git a/net/dcsctp/rx/BUILD.gn b/net/dcsctp/rx/BUILD.gn index c5023a5907..de96678a3e 100644 --- a/net/dcsctp/rx/BUILD.gn +++ b/net/dcsctp/rx/BUILD.gn @@ -104,6 +104,7 @@ if (rtc_include_tests) { ":reassembly_streams", ":traditional_reassembly_streams", "../../../api:array_view", + "../../../api/task_queue:task_queue", "../../../rtc_base:checks", "../../../rtc_base:gunit_helpers", "../../../rtc_base:rtc_base_approved", diff --git a/net/dcsctp/rx/data_tracker_test.cc b/net/dcsctp/rx/data_tracker_test.cc index 6140241f4e..3d8380a05c 100644 --- a/net/dcsctp/rx/data_tracker_test.cc +++ b/net/dcsctp/rx/data_tracker_test.cc @@ -15,6 +15,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/common/handover_testing.h" #include "net/dcsctp/packet/chunk/sack_chunk.h" #include "net/dcsctp/timer/fake_timeout.h" @@ -36,7 +37,9 @@ class DataTrackerTest : public testing::Test { protected: DataTrackerTest() : timeout_manager_([this]() { return now_; }), - timer_manager_([this]() { return timeout_manager_.CreateTimeout(); }), + timer_manager_([this](webrtc::TaskQueueBase::DelayPrecision precision) { + return timeout_manager_.CreateTimeout(precision); + }), timer_(timer_manager_.CreateTimer( "test/delayed_ack", []() { return absl::nullopt; }, diff --git a/net/dcsctp/socket/BUILD.gn b/net/dcsctp/socket/BUILD.gn index 1a1d11f547..e8c64b09b8 100644 --- a/net/dcsctp/socket/BUILD.gn +++ b/net/dcsctp/socket/BUILD.gn @@ -136,6 +136,7 @@ rtc_library("dcsctp_socket") { "../../../api:array_view", "../../../api:refcountedbase", "../../../api:scoped_refptr", + "../../../api/task_queue:task_queue", "../../../rtc_base", "../../../rtc_base:checks", "../../../rtc_base:rtc_base_approved", @@ -180,6 +181,7 @@ if (rtc_include_tests) { sources = [ "mock_dcsctp_socket_callbacks.h" ] deps = [ "../../../api:array_view", + "../../../api/task_queue:task_queue", "../../../rtc_base:logging", "../../../rtc_base:rtc_base_approved", "../../../test:test_support", @@ -224,6 +226,7 @@ if (rtc_include_tests) { "../../../api:array_view", "../../../api:create_network_emulation_manager", "../../../api:network_emulation_manager_api", + "../../../api/task_queue:task_queue", "../../../api/units:time_delta", "../../../call:simulated_network", "../../../rtc_base:checks", diff --git a/net/dcsctp/socket/callback_deferrer.cc b/net/dcsctp/socket/callback_deferrer.cc index b4af10e88a..34d809cbef 100644 --- a/net/dcsctp/socket/callback_deferrer.cc +++ b/net/dcsctp/socket/callback_deferrer.cc @@ -61,9 +61,10 @@ SendPacketStatus CallbackDeferrer::SendPacketWithStatus( return underlying_.SendPacketWithStatus(data); } -std::unique_ptr CallbackDeferrer::CreateTimeout() { +std::unique_ptr CallbackDeferrer::CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) { // Will not be deferred - call directly. - return underlying_.CreateTimeout(); + return underlying_.CreateTimeout(precision); } TimeMs CallbackDeferrer::TimeMillis() { diff --git a/net/dcsctp/socket/callback_deferrer.h b/net/dcsctp/socket/callback_deferrer.h index 918b1df32d..863093c5c5 100644 --- a/net/dcsctp/socket/callback_deferrer.h +++ b/net/dcsctp/socket/callback_deferrer.h @@ -21,6 +21,7 @@ #include "api/array_view.h" #include "api/ref_counted_base.h" #include "api/scoped_refptr.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/public/dcsctp_message.h" #include "net/dcsctp/public/dcsctp_socket.h" #include "rtc_base/ref_counted_object.h" @@ -62,7 +63,8 @@ class CallbackDeferrer : public DcSctpSocketCallbacks { // Implementation of DcSctpSocketCallbacks SendPacketStatus SendPacketWithStatus( rtc::ArrayView data) override; - std::unique_ptr CreateTimeout() override; + std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) override; TimeMs TimeMillis() override; uint32_t GetRandomInt(uint32_t low, uint32_t high) override; void OnMessageReceived(DcSctpMessage message) override; diff --git a/net/dcsctp/socket/dcsctp_socket.cc b/net/dcsctp/socket/dcsctp_socket.cc index 27a9ff44c5..9f38ad8aec 100644 --- a/net/dcsctp/socket/dcsctp_socket.cc +++ b/net/dcsctp/socket/dcsctp_socket.cc @@ -22,6 +22,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/packet/chunk/abort_chunk.h" #include "net/dcsctp/packet/chunk/chunk.h" #include "net/dcsctp/packet/chunk/cookie_ack_chunk.h" @@ -162,7 +163,9 @@ DcSctpSocket::DcSctpSocket(absl::string_view log_prefix, packet_observer_(std::move(packet_observer)), options_(options), callbacks_(callbacks), - timer_manager_([this]() { return callbacks_.CreateTimeout(); }), + timer_manager_([this](webrtc::TaskQueueBase::DelayPrecision precision) { + return callbacks_.CreateTimeout(precision); + }), t1_init_(timer_manager_.CreateTimer( "t1-init", absl::bind_front(&DcSctpSocket::OnInitTimerExpiry, this), diff --git a/net/dcsctp/socket/dcsctp_socket_network_test.cc b/net/dcsctp/socket/dcsctp_socket_network_test.cc index bfd748ac0f..486679ae15 100644 --- a/net/dcsctp/socket/dcsctp_socket_network_test.cc +++ b/net/dcsctp/socket/dcsctp_socket_network_test.cc @@ -18,6 +18,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "api/test/create_network_emulation_manager.h" #include "api/test/network_emulation_manager.h" #include "api/units/time_delta.h" @@ -187,8 +188,9 @@ class SctpActor : public rtc::MessageHandlerAutoCleanup, emulated_socket_.SendPacket(data); } - std::unique_ptr CreateTimeout() override { - return timeout_factory_.CreateTimeout(); + std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) override { + return timeout_factory_.CreateTimeout(precision); } TimeMs TimeMillis() override { return TimeMs(rtc::TimeMillis()); } diff --git a/net/dcsctp/socket/heartbeat_handler_test.cc b/net/dcsctp/socket/heartbeat_handler_test.cc index 83a7355ac9..faa0e3da06 100644 --- a/net/dcsctp/socket/heartbeat_handler_test.cc +++ b/net/dcsctp/socket/heartbeat_handler_test.cc @@ -13,6 +13,7 @@ #include #include +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/packet/chunk/heartbeat_ack_chunk.h" #include "net/dcsctp/packet/chunk/heartbeat_request_chunk.h" #include "net/dcsctp/packet/parameter/heartbeat_info_parameter.h" @@ -44,7 +45,9 @@ class HeartbeatHandlerTestBase : public testing::Test { explicit HeartbeatHandlerTestBase(DurationMs heartbeat_interval) : options_(MakeOptions(heartbeat_interval)), context_(&callbacks_), - timer_manager_([this]() { return callbacks_.CreateTimeout(); }), + timer_manager_([this](webrtc::TaskQueueBase::DelayPrecision precision) { + return callbacks_.CreateTimeout(precision); + }), handler_("log: ", options_, &context_, &timer_manager_) {} void AdvanceTime(DurationMs duration) { diff --git a/net/dcsctp/socket/mock_dcsctp_socket_callbacks.h b/net/dcsctp/socket/mock_dcsctp_socket_callbacks.h index a49a0b3325..1e30777e89 100644 --- a/net/dcsctp/socket/mock_dcsctp_socket_callbacks.h +++ b/net/dcsctp/socket/mock_dcsctp_socket_callbacks.h @@ -20,6 +20,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/public/dcsctp_message.h" #include "net/dcsctp/public/dcsctp_socket.h" #include "net/dcsctp/public/timeout.h" @@ -87,7 +88,9 @@ class MockDcSctpSocketCallbacks : public DcSctpSocketCallbacks { (rtc::ArrayView data), (override)); - std::unique_ptr CreateTimeout() override { + std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) override { + // The fake timeout manager does not implement |precision|. return timeout_manager_.CreateTimeout(); } diff --git a/net/dcsctp/socket/stream_reset_handler_test.cc b/net/dcsctp/socket/stream_reset_handler_test.cc index 894f486388..6f6874f2a0 100644 --- a/net/dcsctp/socket/stream_reset_handler_test.cc +++ b/net/dcsctp/socket/stream_reset_handler_test.cc @@ -17,6 +17,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/common/handover_testing.h" #include "net/dcsctp/common/internal_types.h" #include "net/dcsctp/packet/chunk/reconfig_chunk.h" @@ -87,7 +88,9 @@ class StreamResetHandlerTest : public testing::Test { protected: StreamResetHandlerTest() : ctx_(&callbacks_), - timer_manager_([this]() { return callbacks_.CreateTimeout(); }), + timer_manager_([this](webrtc::TaskQueueBase::DelayPrecision precision) { + return callbacks_.CreateTimeout(precision); + }), delayed_ack_timer_(timer_manager_.CreateTimer( "test/delayed_ack", []() { return absl::nullopt; }, diff --git a/net/dcsctp/timer/BUILD.gn b/net/dcsctp/timer/BUILD.gn index 856125c8c4..4470903ffc 100644 --- a/net/dcsctp/timer/BUILD.gn +++ b/net/dcsctp/timer/BUILD.gn @@ -11,6 +11,7 @@ import("../../../webrtc.gni") rtc_library("timer") { deps = [ "../../../api:array_view", + "../../../api/task_queue:task_queue", "../../../rtc_base", "../../../rtc_base:checks", "../../../rtc_base:rtc_base_approved", @@ -57,6 +58,8 @@ if (rtc_include_tests) { ":task_queue_timeout", ":timer", "../../../api:array_view", + "../../../api/task_queue:task_queue", + "../../../api/task_queue/test:mock_task_queue_base", "../../../rtc_base:checks", "../../../rtc_base:gunit_helpers", "../../../rtc_base:rtc_base_approved", diff --git a/net/dcsctp/timer/fake_timeout.h b/net/dcsctp/timer/fake_timeout.h index e8f50d93cb..f87227577b 100644 --- a/net/dcsctp/timer/fake_timeout.h +++ b/net/dcsctp/timer/fake_timeout.h @@ -18,6 +18,7 @@ #include #include "absl/types/optional.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/public/timeout.h" #include "rtc_base/checks.h" #include "rtc_base/containers/flat_set.h" @@ -27,8 +28,8 @@ namespace dcsctp { // A timeout used in tests. class FakeTimeout : public Timeout { public: - explicit FakeTimeout(std::function get_time, - std::function on_delete) + FakeTimeout(std::function get_time, + std::function on_delete) : get_time_(std::move(get_time)), on_delete_(std::move(on_delete)) {} ~FakeTimeout() override { on_delete_(this); } @@ -68,12 +69,17 @@ class FakeTimeoutManager { explicit FakeTimeoutManager(std::function get_time) : get_time_(std::move(get_time)) {} - std::unique_ptr CreateTimeout() { + std::unique_ptr CreateTimeout() { auto timer = std::make_unique( get_time_, [this](FakeTimeout* timer) { timers_.erase(timer); }); timers_.insert(timer.get()); return timer; } + std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision) { + // FakeTimeout does not support implement |precision|. + return CreateTimeout(); + } // NOTE: This can't return a vector, as calling EvaluateHasExpired requires // calling socket->HandleTimeout directly afterwards, as the owning Timer diff --git a/net/dcsctp/timer/task_queue_timeout.cc b/net/dcsctp/timer/task_queue_timeout.cc index 6d3054eeb8..4d1dc1907b 100644 --- a/net/dcsctp/timer/task_queue_timeout.cc +++ b/net/dcsctp/timer/task_queue_timeout.cc @@ -16,8 +16,10 @@ namespace dcsctp { TaskQueueTimeoutFactory::TaskQueueTimeout::TaskQueueTimeout( - TaskQueueTimeoutFactory& parent) + TaskQueueTimeoutFactory& parent, + webrtc::TaskQueueBase::DelayPrecision precision) : parent_(parent), + precision_(precision), pending_task_safety_flag_(webrtc::PendingTaskSafetyFlag::Create()) {} TaskQueueTimeoutFactory::TaskQueueTimeout::~TaskQueueTimeout() { @@ -54,7 +56,8 @@ void TaskQueueTimeoutFactory::TaskQueueTimeout::Start(DurationMs duration_ms, } posted_task_expiration_ = timeout_expiration_; - parent_.task_queue_.PostDelayedTask( + parent_.task_queue_.PostDelayedTaskWithPrecision( + precision_, webrtc::ToQueuedTask( pending_task_safety_flag_, [timeout_id, this]() { diff --git a/net/dcsctp/timer/task_queue_timeout.h b/net/dcsctp/timer/task_queue_timeout.h index e8d12df592..600b292443 100644 --- a/net/dcsctp/timer/task_queue_timeout.h +++ b/net/dcsctp/timer/task_queue_timeout.h @@ -45,14 +45,17 @@ class TaskQueueTimeoutFactory { on_expired_(std::move(on_expired)) {} // Creates an implementation of `Timeout`. - std::unique_ptr CreateTimeout() { - return std::make_unique(*this); + std::unique_ptr CreateTimeout( + webrtc::TaskQueueBase::DelayPrecision precision = + webrtc::TaskQueueBase::DelayPrecision::kLow) { + return std::make_unique(*this, precision); } private: class TaskQueueTimeout : public Timeout { public: - explicit TaskQueueTimeout(TaskQueueTimeoutFactory& parent); + TaskQueueTimeout(TaskQueueTimeoutFactory& parent, + webrtc::TaskQueueBase::DelayPrecision precision); ~TaskQueueTimeout(); void Start(DurationMs duration_ms, TimeoutID timeout_id) override; @@ -60,6 +63,7 @@ class TaskQueueTimeoutFactory { private: TaskQueueTimeoutFactory& parent_; + const webrtc::TaskQueueBase::DelayPrecision precision_; // A safety flag to ensure that posted tasks to the task queue don't // reference these object when they go out of scope. Note that this safety // flag will be re-created if the scheduled-but-not-yet-expired task is not diff --git a/net/dcsctp/timer/task_queue_timeout_test.cc b/net/dcsctp/timer/task_queue_timeout_test.cc index 9d3846953b..da01ecf991 100644 --- a/net/dcsctp/timer/task_queue_timeout_test.cc +++ b/net/dcsctp/timer/task_queue_timeout_test.cc @@ -11,13 +11,18 @@ #include +#include "api/task_queue/queued_task.h" +#include "api/task_queue/task_queue_base.h" +#include "api/task_queue/test/mock_task_queue_base.h" #include "rtc_base/gunit.h" #include "test/gmock.h" #include "test/time_controller/simulated_time_controller.h" namespace dcsctp { namespace { +using ::testing::_; using ::testing::MockFunction; +using ::testing::NiceMock; class TaskQueueTimeoutTest : public testing::Test { protected: @@ -111,5 +116,38 @@ TEST_F(TaskQueueTimeoutTest, KilledBeforeExpired) { EXPECT_CALL(on_expired_, Call).Times(0); AdvanceTime(DurationMs(1000)); } + +TEST(TaskQueueTimeoutWithMockTaskQueueTest, CanSetTimeoutPrecisionToLow) { + NiceMock mock_task_queue; + EXPECT_CALL(mock_task_queue, PostDelayedTask(_, _)); + TaskQueueTimeoutFactory factory( + mock_task_queue, []() { return TimeMs(1337); }, + [](TimeoutID timeout_id) {}); + std::unique_ptr timeout = + factory.CreateTimeout(webrtc::TaskQueueBase::DelayPrecision::kLow); + timeout->Start(DurationMs(1), TimeoutID(1)); +} + +TEST(TaskQueueTimeoutWithMockTaskQueueTest, CanSetTimeoutPrecisionToHigh) { + NiceMock mock_task_queue; + EXPECT_CALL(mock_task_queue, PostDelayedHighPrecisionTask(_, _)); + TaskQueueTimeoutFactory factory( + mock_task_queue, []() { return TimeMs(1337); }, + [](TimeoutID timeout_id) {}); + std::unique_ptr timeout = + factory.CreateTimeout(webrtc::TaskQueueBase::DelayPrecision::kHigh); + timeout->Start(DurationMs(1), TimeoutID(1)); +} + +TEST(TaskQueueTimeoutWithMockTaskQueueTest, TimeoutPrecisionIsLowByDefault) { + NiceMock mock_task_queue; + EXPECT_CALL(mock_task_queue, PostDelayedTask(_, _)); + TaskQueueTimeoutFactory factory( + mock_task_queue, []() { return TimeMs(1337); }, + [](TimeoutID timeout_id) {}); + std::unique_ptr timeout = factory.CreateTimeout(); + timeout->Start(DurationMs(1), TimeoutID(1)); +} + } // namespace } // namespace dcsctp diff --git a/net/dcsctp/timer/timer.cc b/net/dcsctp/timer/timer.cc index deee7db0c5..bde07638a5 100644 --- a/net/dcsctp/timer/timer.cc +++ b/net/dcsctp/timer/timer.cc @@ -144,7 +144,7 @@ std::unique_ptr TimerManager::CreateTimer(absl::string_view name, // after 800 million reconnections on a single socket. Ensure this will never // happen. RTC_CHECK_NE(*id, std::numeric_limits::max()); - std::unique_ptr timeout = create_timeout_(); + std::unique_ptr timeout = create_timeout_(options.precision); RTC_CHECK(timeout != nullptr); auto timer = absl::WrapUnique(new Timer( id, name, std::move(on_expired), [this, id]() { timers_.erase(id); }, diff --git a/net/dcsctp/timer/timer.h b/net/dcsctp/timer/timer.h index 49dff34e4c..31b496dc81 100644 --- a/net/dcsctp/timer/timer.h +++ b/net/dcsctp/timer/timer.h @@ -21,6 +21,7 @@ #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/public/timeout.h" #include "rtc_base/strong_alias.h" @@ -52,10 +53,21 @@ struct TimerOptions { TimerBackoffAlgorithm backoff_algorithm, absl::optional max_restarts, absl::optional max_backoff_duration) + : TimerOptions(duration, + backoff_algorithm, + max_restarts, + max_backoff_duration, + webrtc::TaskQueueBase::DelayPrecision::kLow) {} + TimerOptions(DurationMs duration, + TimerBackoffAlgorithm backoff_algorithm, + absl::optional max_restarts, + absl::optional max_backoff_duration, + webrtc::TaskQueueBase::DelayPrecision precision) : duration(duration), backoff_algorithm(backoff_algorithm), max_restarts(max_restarts), - max_backoff_duration(max_backoff_duration) {} + max_backoff_duration(max_backoff_duration), + precision(precision) {} // The initial timer duration. Can be overridden with `set_duration`. const DurationMs duration; @@ -67,6 +79,8 @@ struct TimerOptions { const absl::optional max_restarts; // The maximum timeout value for exponential backoff. const absl::optional max_backoff_duration; + // The precision of the webrtc::TaskQueueBase used for scheduling. + const webrtc::TaskQueueBase::DelayPrecision precision; }; // A high-level timer (in contrast to the low-level `Timeout` class). @@ -172,7 +186,8 @@ class Timer { class TimerManager { public: explicit TimerManager( - std::function()> create_timeout) + std::function( + webrtc::TaskQueueBase::DelayPrecision)> create_timeout) : create_timeout_(std::move(create_timeout)) {} // Creates a timer with name `name` that will expire (when started) after @@ -185,7 +200,9 @@ class TimerManager { void HandleTimeout(TimeoutID timeout_id); private: - const std::function()> create_timeout_; + const std::function( + webrtc::TaskQueueBase::DelayPrecision)> + create_timeout_; std::map timers_; TimerID next_id_ = TimerID(0); }; diff --git a/net/dcsctp/timer/timer_test.cc b/net/dcsctp/timer/timer_test.cc index 5550f7fabf..4aebe65b48 100644 --- a/net/dcsctp/timer/timer_test.cc +++ b/net/dcsctp/timer/timer_test.cc @@ -12,6 +12,7 @@ #include #include "absl/types/optional.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/public/timeout.h" #include "net/dcsctp/timer/fake_timeout.h" #include "rtc_base/gunit.h" @@ -25,7 +26,9 @@ class TimerTest : public testing::Test { protected: TimerTest() : timeout_manager_([this]() { return now_; }), - manager_([this]() { return timeout_manager_.CreateTimeout(); }) { + manager_([this](webrtc::TaskQueueBase::DelayPrecision precision) { + return timeout_manager_.CreateTimeout(precision); + }) { ON_CALL(on_expired_, Call).WillByDefault(Return(absl::nullopt)); } @@ -423,5 +426,34 @@ TEST_F(TimerTest, DurationStaysWithinMaxTimerBackOffDuration) { AdvanceTimeAndRunTimers(DurationMs(1)); } +TEST(TimerManagerTest, TimerManagerPassesPrecisionToCreateTimeoutMethod) { + FakeTimeoutManager timeout_manager([&]() { return TimeMs(0); }); + absl::optional create_timer_precison; + TimerManager manager([&](webrtc::TaskQueueBase::DelayPrecision precision) { + create_timer_precison = precision; + return timeout_manager.CreateTimeout(precision); + }); + // Default TimerOptions. + manager.CreateTimer( + "test_timer", []() { return absl::optional(); }, + TimerOptions(DurationMs(123))); + EXPECT_EQ(create_timer_precison, webrtc::TaskQueueBase::DelayPrecision::kLow); + // High precision TimerOptions. + manager.CreateTimer( + "test_timer", []() { return absl::optional(); }, + TimerOptions(DurationMs(123), TimerBackoffAlgorithm::kExponential, + absl::nullopt, absl::nullopt, + webrtc::TaskQueueBase::DelayPrecision::kHigh)); + EXPECT_EQ(create_timer_precison, + webrtc::TaskQueueBase::DelayPrecision::kHigh); + // Low precision TimerOptions. + manager.CreateTimer( + "test_timer", []() { return absl::optional(); }, + TimerOptions(DurationMs(123), TimerBackoffAlgorithm::kExponential, + absl::nullopt, absl::nullopt, + webrtc::TaskQueueBase::DelayPrecision::kLow)); + EXPECT_EQ(create_timer_precison, webrtc::TaskQueueBase::DelayPrecision::kLow); +} + } // namespace } // namespace dcsctp diff --git a/net/dcsctp/tx/BUILD.gn b/net/dcsctp/tx/BUILD.gn index 9191ca46b1..44dbd0b8f8 100644 --- a/net/dcsctp/tx/BUILD.gn +++ b/net/dcsctp/tx/BUILD.gn @@ -146,6 +146,7 @@ if (rtc_include_tests) { ":rr_send_queue", ":send_queue", "../../../api:array_view", + "../../../api/task_queue:task_queue", "../../../rtc_base:checks", "../../../rtc_base:gunit_helpers", "../../../rtc_base:rtc_base_approved", diff --git a/net/dcsctp/tx/retransmission_queue_test.cc b/net/dcsctp/tx/retransmission_queue_test.cc index 01655556de..8294a3111d 100644 --- a/net/dcsctp/tx/retransmission_queue_test.cc +++ b/net/dcsctp/tx/retransmission_queue_test.cc @@ -18,6 +18,7 @@ #include "absl/types/optional.h" #include "api/array_view.h" +#include "api/task_queue/task_queue_base.h" #include "net/dcsctp/common/handover_testing.h" #include "net/dcsctp/common/math.h" #include "net/dcsctp/packet/chunk/data_chunk.h" @@ -63,7 +64,9 @@ class RetransmissionQueueTest : public testing::Test { : options_(MakeOptions()), gen_(MID(42)), timeout_manager_([this]() { return now_; }), - timer_manager_([this]() { return timeout_manager_.CreateTimeout(); }), + timer_manager_([this](webrtc::TaskQueueBase::DelayPrecision precision) { + return timeout_manager_.CreateTimeout(precision); + }), timer_(timer_manager_.CreateTimer( "test/t3_rtx", []() { return absl::nullopt; },