From d00405f89accc218f061a240fe7ada4a07079c73 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 25 Feb 2019 15:06:13 +0100 Subject: [PATCH] Drop support for link-time injection of the rtc::TaskQueue::Impl Bug: webrtc:10191 Change-Id: I1b975e8a2230dd45828a4e7f4d5a86f61164445a Reviewed-on: https://webrtc-review.googlesource.com/c/124121 Commit-Queue: Danil Chapovalov Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#26870} --- api/task_queue/BUILD.gn | 25 ++++++----------- api/task_queue/DEPS | 5 ---- api/task_queue/task_queue.cc | 13 --------- api/task_queue/task_queue_base.h | 32 ++++++++++++---------- api/task_queue/task_queue_factory.h | 4 +-- api/task_queue/task_queue_impl.cc | 31 --------------------- api/task_queue/task_queue_impl.h | 40 ---------------------------- api/task_queue/task_queue_priority.h | 21 --------------- rtc_base/BUILD.gn | 5 ---- rtc_base/task_queue.h | 17 +++--------- 10 files changed, 32 insertions(+), 161 deletions(-) delete mode 100644 api/task_queue/task_queue_impl.cc delete mode 100644 api/task_queue/task_queue_impl.h delete mode 100644 api/task_queue/task_queue_priority.h diff --git a/api/task_queue/BUILD.gn b/api/task_queue/BUILD.gn index e7e3aec32f..333c872540 100644 --- a/api/task_queue/BUILD.gn +++ b/api/task_queue/BUILD.gn @@ -12,34 +12,26 @@ rtc_source_set("task_queue") { visibility = [ "*" ] public = [ "queued_task.h", - "task_queue_priority.h", - ] -} - -# TODO(bugs.webrtc.org/10191): Merge the target into task_queue target above -# when support for link-time injection is dropped. -rtc_source_set("task_queue_factory") { - visibility = [ "*" ] - public = [ "task_queue_base.h", "task_queue_factory.h", ] sources = [ "task_queue_base.cc", - "task_queue_impl.cc", - "task_queue_impl.h", ] deps = [ - ":task_queue", "../../rtc_base:checks", - "../../rtc_base:rtc_task_queue_api", "//third_party/abseil-cpp/absl/base:config", "//third_party/abseil-cpp/absl/base:core_headers", "//third_party/abseil-cpp/absl/strings", ] } +# TODO(danilchap): Remove this empty target when downstream project stop depending on it. +rtc_source_set("task_queue_factory") { + visibility = [ "*" ] +} + rtc_source_set("task_queue_test") { visibility = [ "*" ] testonly = true @@ -49,7 +41,6 @@ rtc_source_set("task_queue_test") { ] deps = [ ":task_queue", - ":task_queue_factory", "../../rtc_base:rtc_event", "../../rtc_base:rtc_task_queue_api", "../../rtc_base:timeutils", @@ -65,7 +56,7 @@ rtc_source_set("default_task_queue_factory") { "default_task_queue_factory.h", ] deps = [ - ":task_queue_factory", + ":task_queue", ] # TODO(bugs.webrtc.org/10284): Include implementation unconditionally when @@ -85,7 +76,7 @@ rtc_source_set("default_task_queue_factory_impl") { "../../rtc_base:rtc_task_queue_impl", ] deps = [ - ":task_queue_factory", + ":task_queue", ] if (rtc_enable_libevent) { sources = [ @@ -141,7 +132,7 @@ rtc_source_set("global_task_queue_factory") { ] deps = [ ":default_task_queue_factory", - ":task_queue_factory", + ":task_queue", "../../rtc_base:checks", "../../rtc_base:rtc_task_queue_api", ] diff --git a/api/task_queue/DEPS b/api/task_queue/DEPS index 26f86a29ad..9cb6b1fe79 100644 --- a/api/task_queue/DEPS +++ b/api/task_queue/DEPS @@ -1,9 +1,4 @@ specific_include_rules = { - # temporary include to support both rtc::TaskQueue::Current() and - # webrtc::TaskQueueBase::Current() for implementaions of the TaskQueueBase. - "task_queue_impl\.h": [ - "+rtc_base/task_queue.h", - ], "task_queue_test\.h": [ "+test/gtest.h", ], diff --git a/api/task_queue/task_queue.cc b/api/task_queue/task_queue.cc index 4a33d48f8d..06358502a3 100644 --- a/api/task_queue/task_queue.cc +++ b/api/task_queue/task_queue.cc @@ -15,10 +15,6 @@ namespace rtc { TaskQueue::TaskQueue(const char* queue_name, Priority priority) - // For backward compatibility impl_ need to be scoped_refptr, - // But this implementation treat impl_ as - // std::unique_ptr abusing - // fact that both classes are wrappers around raw pointer. : impl_(webrtc::GlobalTaskQueueFactory() .CreateTaskQueue(queue_name, priority) .release()) { @@ -26,16 +22,7 @@ TaskQueue::TaskQueue(const char* queue_name, Priority priority) } TaskQueue::~TaskQueue() { - // TODO(danilchap): change impl_ to webrtc::TaskQueueBase* when dependenent - // projects stop using link-injection to override task queue and thus do not - // rely on exact TaskQueue layout. - // There might running task that tries to rescheduler itself to the TaskQueue - // and not yet away TaskQueue destructor is called. - // Calling back to TaskQueue::PostTask need impl_ pointer still be valid, so - // Start the destruction first, ... impl_->Delete(); - // release the pointer later. - const_cast&>(impl_).release(); } // static diff --git a/api/task_queue/task_queue_base.h b/api/task_queue/task_queue_base.h index 175253b3ef..b1b5cc7f1b 100644 --- a/api/task_queue/task_queue_base.h +++ b/api/task_queue/task_queue_base.h @@ -13,7 +13,12 @@ #include #include "api/task_queue/queued_task.h" -#include "api/task_queue/task_queue_impl.h" + +// TODO(bugs.webrtc.org/10191): Remove when +// rtc::TaskQueue* rtc::TaskQueue::Current() is unused. +namespace rtc { +class TaskQueue; +} // namespace rtc namespace webrtc { @@ -21,10 +26,7 @@ namespace webrtc { // in FIFO order and that tasks never overlap. Tasks may always execute on the // same worker thread and they may not. To DCHECK that tasks are executing on a // known task queue, use IsCurrent(). -// TODO(bugs.webrtc.org/10191): Remove inheritence from rtc::TaskQueue::Impl -// when all implementations switch to use TaskQueueFactory instead of link-time -// injection. -class TaskQueueBase : public rtc::TaskQueue::Impl { +class TaskQueueBase { public: // Starts destruction of the task queue. // On return ensures no task are running and no new tasks are able to start @@ -35,7 +37,7 @@ class TaskQueueBase : public rtc::TaskQueue::Impl { // TaskQueue is deallocated and thus should not call any methods after Delete. // Code running on the TaskQueue should not call Delete, but can assume // TaskQueue still exists and may call other methods, e.g. PostTask. - void Delete() override = 0; + virtual void Delete() = 0; // Schedules a task to execute. Tasks are executed in FIFO order. // If |task->Run()| returns true, task is deleted on the task queue @@ -45,19 +47,17 @@ class TaskQueueBase : public rtc::TaskQueue::Impl { // TaskQueue or it may happen asynchronously after TaskQueue is deleted. // This may vary from one implementation to the next so assumptions about // lifetimes of pending tasks should not be made. - void PostTask(std::unique_ptr task) override = 0; + virtual void PostTask(std::unique_ptr task) = 0; // Schedules a task to execute a specified number of milliseconds from when // the call is made. The precision should be considered as "best effort" // and in some cases, such as on Windows when all high precision timers have // been used up, can be off by as much as 15 millseconds. - void PostDelayedTask(std::unique_ptr task, - uint32_t milliseconds) override = 0; + virtual void PostDelayedTask(std::unique_ptr task, + uint32_t milliseconds) = 0; - // Until all TaskQueue implementations switch to using CurrentTaskQueueSetter - // below, this function may return nullptr even if code is executed by a - // TaskQueue. Keep using rtc::TaskQueue::Current() until bugs.webrtc.org/10191 - // is resolved. + // Returns the task queue that is running the current thread. + // Returns nullptr if this thread is not associated with any task queue. static TaskQueueBase* Current(); bool IsCurrent() const { return Current() == this; } @@ -75,7 +75,11 @@ class TaskQueueBase : public rtc::TaskQueue::Impl { // Users of the TaskQueue should call Delete instead of directly deleting // this object. - ~TaskQueueBase() override = default; + virtual ~TaskQueueBase() = default; + + private: + friend class rtc::TaskQueue; + rtc::TaskQueue* task_queue_ = nullptr; }; struct TaskQueueDeleter { diff --git a/api/task_queue/task_queue_factory.h b/api/task_queue/task_queue_factory.h index 682713856e..b68ab33f69 100644 --- a/api/task_queue/task_queue_factory.h +++ b/api/task_queue/task_queue_factory.h @@ -14,15 +14,15 @@ #include "absl/strings/string_view.h" #include "api/task_queue/task_queue_base.h" -#include "api/task_queue/task_queue_priority.h" namespace webrtc { +// The implementation of this interface must be thread-safe. class TaskQueueFactory { public: // TaskQueue priority levels. On some platforms these will map to thread // priorities, on others such as Mac and iOS, GCD queue priorities. - using Priority = ::webrtc::TaskQueuePriority; + enum class Priority { NORMAL = 0, HIGH, LOW }; virtual ~TaskQueueFactory() = default; virtual std::unique_ptr CreateTaskQueue( diff --git a/api/task_queue/task_queue_impl.cc b/api/task_queue/task_queue_impl.cc deleted file mode 100644 index 0024085093..0000000000 --- a/api/task_queue/task_queue_impl.cc +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Copyright 2019 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. - */ -#include "api/task_queue/task_queue_impl.h" - -#include "rtc_base/checks.h" - -namespace rtc { - -// Fake ref counting: implementers of the TaskQueueBase shouldn't expect it is -// stored in a refererence counter pointer. -void TaskQueue::Impl::AddRef() { - // AddRef should be called exactly once by rtc::TaskQueue constructor when - // raw pointer converted into scoped_refptr, - // just before TaskQueue constructor assign task_queue_ member. - RTC_CHECK(task_queue_ == nullptr); -} - -void TaskQueue::Impl::Release() { - // TaskQueue destructor manually destroyes this object, thus Release should - // never be called. - RTC_CHECK(false); -} - -} // namespace rtc diff --git a/api/task_queue/task_queue_impl.h b/api/task_queue/task_queue_impl.h deleted file mode 100644 index a3b676d906..0000000000 --- a/api/task_queue/task_queue_impl.h +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright 2019 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_TASK_QUEUE_IMPL_H_ -#define API_TASK_QUEUE_TASK_QUEUE_IMPL_H_ - -#include - -#include "api/task_queue/queued_task.h" -#include "rtc_base/task_queue.h" - -// TODO(danilchap): Remove Impl and dependency on rtc::TaskQueue when custom -// implementations switch to use global factories that creates TaskQueue -// instead of using link-time injection. -class rtc::TaskQueue::Impl { - public: - virtual void Delete() = 0; - virtual void PostTask(std::unique_ptr task) = 0; - virtual void PostDelayedTask(std::unique_ptr task, - uint32_t milliseconds) = 0; - - void AddRef(); - void Release(); - - protected: - virtual ~Impl() = default; - - private: - friend class rtc::TaskQueue; - rtc::TaskQueue* task_queue_ = nullptr; -}; - -#endif // API_TASK_QUEUE_TASK_QUEUE_IMPL_H_ diff --git a/api/task_queue/task_queue_priority.h b/api/task_queue/task_queue_priority.h deleted file mode 100644 index a8696ec149..0000000000 --- a/api/task_queue/task_queue_priority.h +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright 2019 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_TASK_QUEUE_PRIORITY_H_ -#define API_TASK_QUEUE_TASK_QUEUE_PRIORITY_H_ - -namespace webrtc { - -// TODO(bugs.webrtc.org/10191): Move as member class of TaskQueueFactory when -// rtc::TaskQueue would be able to depende on it. -enum class TaskQueuePriority { NORMAL = 0, HIGH, LOW }; - -} // namespace webrtc - -#endif // API_TASK_QUEUE_TASK_QUEUE_PRIORITY_H_ diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 0c66af898c..8a8ad32196 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -498,7 +498,6 @@ rtc_source_set("rtc_task_queue_api") { ] deps = [ ":macromagic", - "../api:scoped_refptr", "../api/task_queue", "system:rtc_export", "//third_party/abseil-cpp/absl/memory", @@ -522,7 +521,6 @@ if (rtc_enable_libevent) { ":safe_conversions", ":timeutils", "../api/task_queue", - "../api/task_queue:task_queue_factory", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", ] @@ -543,7 +541,6 @@ if (is_mac || is_ios) { ":checks", ":logging", "../api/task_queue", - "../api/task_queue:task_queue_factory", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", ] @@ -568,7 +565,6 @@ if (is_win) { ":safe_conversions", ":timeutils", "../api/task_queue", - "../api/task_queue:task_queue_factory", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", ] @@ -592,7 +588,6 @@ rtc_source_set("rtc_task_queue_stdlib") { ":safe_conversions", ":timeutils", "../api/task_queue", - "../api/task_queue:task_queue_factory", "//third_party/abseil-cpp/absl/memory", "//third_party/abseil-cpp/absl/strings", ] diff --git a/rtc_base/task_queue.h b/rtc_base/task_queue.h index d6198a8474..d5be38b214 100644 --- a/rtc_base/task_queue.h +++ b/rtc_base/task_queue.h @@ -17,9 +17,9 @@ #include #include "absl/memory/memory.h" -#include "api/scoped_refptr.h" #include "api/task_queue/queued_task.h" -#include "api/task_queue/task_queue_priority.h" +#include "api/task_queue/task_queue_base.h" +#include "api/task_queue/task_queue_factory.h" #include "rtc_base/constructor_magic.h" #include "rtc_base/system/rtc_export.h" #include "rtc_base/thread_annotations.h" @@ -143,8 +143,7 @@ class RTC_LOCKABLE RTC_EXPORT TaskQueue { public: // TaskQueue priority levels. On some platforms these will map to thread // priorities, on others such as Mac and iOS, GCD queue priorities. - using Priority = ::webrtc::TaskQueuePriority; - class Impl; + using Priority = ::webrtc::TaskQueueFactory::Priority; explicit TaskQueue(const char* queue_name, Priority priority = Priority::NORMAL); @@ -188,15 +187,7 @@ class RTC_LOCKABLE RTC_EXPORT TaskQueue { } private: - // TODO(danilchap): Remove when external implementaions of TaskQueue remove - // these two functions. - void PostTaskAndReply(std::unique_ptr task, - std::unique_ptr reply, - TaskQueue* reply_queue); - void PostTaskAndReply(std::unique_ptr task, - std::unique_ptr reply); - - const scoped_refptr impl_; + webrtc::TaskQueueBase* const impl_; RTC_DISALLOW_COPY_AND_ASSIGN(TaskQueue); };