From 7c1ddb760c613e97eda2e757dbfdbefe04174aa7 Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 23 Oct 2023 14:38:13 +0200 Subject: [PATCH] Support initializing a SequenceChecker with a provided TaskQueue. Bug: none Change-Id: I5106f29ab7f9ed8530626f33f6259eb7aeb9e779 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/324260 Reviewed-by: Danil Chapovalov Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#40993} --- api/sequence_checker.h | 14 ++++++++++++++ api/sequence_checker_unittest.cc | 7 +++++++ .../synchronization/sequence_checker_internal.cc | 5 +++++ .../synchronization/sequence_checker_internal.h | 2 ++ 4 files changed, 28 insertions(+) diff --git a/api/sequence_checker.h b/api/sequence_checker.h index 33e0f3c074..5ff5860371 100644 --- a/api/sequence_checker.h +++ b/api/sequence_checker.h @@ -46,8 +46,22 @@ class RTC_LOCKABLE SequenceChecker public: enum InitialState : bool { kDetached = false, kAttached = true }; + // TODO(tommi): We could maybe join these two ctors and have fewer factory + // functions. At the moment they're separate to minimize code changes when + // we added the second ctor as well as avoiding to have unnecessary code at + // the SequenceChecker which much only run for the SequenceCheckerImpl + // implementation. + // In theory we could have something like: + // + // SequenceChecker(InitialState initial_state = kAttached, + // TaskQueueBase* attached_queue = TaskQueueBase::Current()); + // + // But the problem with that is having the call to `Current()` exist for + // `SequenceCheckerDoNothing`. explicit SequenceChecker(InitialState initial_state = kAttached) : Impl(initial_state) {} + explicit SequenceChecker(TaskQueueBase* attached_queue) + : Impl(attached_queue) {} // Returns true if sequence checker is attached to the current sequence. bool IsCurrent() const { return Impl::IsCurrent(); } diff --git a/api/sequence_checker_unittest.cc b/api/sequence_checker_unittest.cc index f117926d73..e54f33dba9 100644 --- a/api/sequence_checker_unittest.cc +++ b/api/sequence_checker_unittest.cc @@ -83,6 +83,13 @@ TEST(SequenceCheckerTest, DetachFromThreadAndUseOnTaskQueue) { queue.SendTask([&] { EXPECT_TRUE(sequence_checker.IsCurrent()); }); } +TEST(SequenceCheckerTest, InitializeForDifferentTaskQueue) { + TaskQueueForTest queue; + SequenceChecker sequence_checker(queue.Get()); + EXPECT_EQ(sequence_checker.IsCurrent(), !RTC_DCHECK_IS_ON); + queue.SendTask([&] { EXPECT_TRUE(sequence_checker.IsCurrent()); }); +} + TEST(SequenceCheckerTest, DetachFromTaskQueueAndUseOnThread) { TaskQueueForTest queue; queue.SendTask([] { diff --git a/rtc_base/synchronization/sequence_checker_internal.cc b/rtc_base/synchronization/sequence_checker_internal.cc index 3e205b91d5..4b9583deb2 100644 --- a/rtc_base/synchronization/sequence_checker_internal.cc +++ b/rtc_base/synchronization/sequence_checker_internal.cc @@ -22,6 +22,11 @@ SequenceCheckerImpl::SequenceCheckerImpl(bool attach_to_current_thread) valid_thread_(rtc::CurrentThreadRef()), valid_queue_(TaskQueueBase::Current()) {} +SequenceCheckerImpl::SequenceCheckerImpl(TaskQueueBase* attached_queue) + : attached_(attached_queue != nullptr), + valid_thread_(rtc::PlatformThreadRef()), + valid_queue_(attached_queue) {} + bool SequenceCheckerImpl::IsCurrent() const { const TaskQueueBase* const current_queue = TaskQueueBase::Current(); const rtc::PlatformThreadRef current_thread = rtc::CurrentThreadRef(); diff --git a/rtc_base/synchronization/sequence_checker_internal.h b/rtc_base/synchronization/sequence_checker_internal.h index 22503027a5..a23ac08885 100644 --- a/rtc_base/synchronization/sequence_checker_internal.h +++ b/rtc_base/synchronization/sequence_checker_internal.h @@ -31,6 +31,7 @@ namespace webrtc_sequence_checker_internal { class RTC_EXPORT SequenceCheckerImpl { public: explicit SequenceCheckerImpl(bool attach_to_current_thread); + explicit SequenceCheckerImpl(TaskQueueBase* attached_queue); ~SequenceCheckerImpl() = default; bool IsCurrent() const; @@ -59,6 +60,7 @@ class RTC_EXPORT SequenceCheckerImpl { class SequenceCheckerDoNothing { public: explicit SequenceCheckerDoNothing(bool attach_to_current_thread) {} + explicit SequenceCheckerDoNothing(TaskQueueBase* attached_queue) {} bool IsCurrent() const { return true; } void Detach() {} };