From cb2a4ffb2bbd09958ad62b7016ac551a20d6dc62 Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Wed, 10 Apr 2019 07:47:00 +0000 Subject: [PATCH] Reland "Remove TaskQueue constructor that uses GlobalTaskQueueFactory" This reverts commit 57f2a5485a4c83c8045d0dbe9db6281d6bcda847. Reason for revert: the breakage addressed with a separate change https://webrtc-review.googlesource.com/c/src/+/131398 Original change's description: > Revert "Remove TaskQueue constructor that uses GlobalTaskQueueFactory" > > This reverts commit 7b7485b796ad77809e3343f3256013488b418235. > > Reason for revert: Breaks Chrome autoroll > > video/video_stream_decoder_impl.cc:28:7: error: no matching constructor for initialization of 'rtc::TaskQueue' > bookkeeping_queue_("video_stream_decoder_bookkeeping_queue"), > > Original change's description: > > Remove TaskQueue constructor that uses GlobalTaskQueueFactory > > > > Bug: webrtc:10284 > > Change-Id: I9547fb7110222ce3a3c2323ae2a004024eab911e > > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/130471 > > Commit-Queue: Danil Chapovalov > > Reviewed-by: Karl Wiberg > > Cr-Commit-Position: refs/heads/master@{#27464} > > TBR=danilchap@webrtc.org,kwiberg@webrtc.org > > # Not skipping CQ checks because original CL landed > 1 day ago. > > Bug: webrtc:10284 > Change-Id: I7684f7c7d5501cc910ac9f9daa8ccf6bdb10f8e1 > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/131338 > Reviewed-by: Florent Castelli > Reviewed-by: Mirko Bonadei > Commit-Queue: Mirko Bonadei > Cr-Commit-Position: refs/heads/master@{#27491} TBR=danilchap@webrtc.org,mbonadei@webrtc.org,kwiberg@webrtc.org,orphis@webrtc.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: webrtc:10284 Change-Id: I0a0544d4b82adaec468d3445b6554a7b94d52db5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/132225 Commit-Queue: Danil Chapovalov Reviewed-by: Danil Chapovalov Reviewed-by: Florent Castelli Cr-Commit-Position: refs/heads/master@{#27537} --- rtc_base/BUILD.gn | 1 - rtc_base/task_queue.cc | 5 ----- rtc_base/task_queue.h | 2 -- test/fuzzers/BUILD.gn | 3 +-- .../audio_processing_configs_fuzzer.cc | 20 ++++++++++++++++--- test/fuzzers/webrtc_fuzzer_main.cc | 9 --------- 6 files changed, 18 insertions(+), 22 deletions(-) diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 8c69b85dd2..2e7017d1fe 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -480,7 +480,6 @@ rtc_source_set("rtc_task_queue") { deps = [ ":macromagic", "../api/task_queue", - "../api/task_queue:global_task_queue_factory", "system:rtc_export", "task_utils:to_queued_task", "//third_party/abseil-cpp/absl/memory", diff --git a/rtc_base/task_queue.cc b/rtc_base/task_queue.cc index 46ab495d48..965a4d8c69 100644 --- a/rtc_base/task_queue.cc +++ b/rtc_base/task_queue.cc @@ -9,7 +9,6 @@ */ #include "rtc_base/task_queue.h" -#include "api/task_queue/global_task_queue_factory.h" #include "api/task_queue/task_queue_base.h" namespace rtc { @@ -18,10 +17,6 @@ TaskQueue::TaskQueue( std::unique_ptr task_queue) : impl_(task_queue.release()) {} -TaskQueue::TaskQueue(const char* queue_name, Priority priority) - : TaskQueue(webrtc::GlobalTaskQueueFactory().CreateTaskQueue(queue_name, - priority)) {} - TaskQueue::~TaskQueue() { // There might running task that tries to rescheduler itself to the TaskQueue // and not yet aware TaskQueue destructor is called. diff --git a/rtc_base/task_queue.h b/rtc_base/task_queue.h index 8d2a89ee21..01fcb78bd3 100644 --- a/rtc_base/task_queue.h +++ b/rtc_base/task_queue.h @@ -80,8 +80,6 @@ class RTC_LOCKABLE RTC_EXPORT TaskQueue { explicit TaskQueue(std::unique_ptr task_queue); - explicit TaskQueue(const char* queue_name, - Priority priority = Priority::NORMAL); ~TaskQueue(); // Used for DCHECKing the current queue. diff --git a/test/fuzzers/BUILD.gn b/test/fuzzers/BUILD.gn index c887108abd..cd755a42d5 100644 --- a/test/fuzzers/BUILD.gn +++ b/test/fuzzers/BUILD.gn @@ -15,9 +15,7 @@ rtc_static_library("webrtc_fuzzer_main") { "webrtc_fuzzer_main.cc", ] deps = [ - "../../api/task_queue:global_task_queue_factory", "../../rtc_base:rtc_base_approved", - "../../rtc_base:rtc_task_queue_stdlib", "//testing/libfuzzer:libfuzzer_main", ] @@ -512,6 +510,7 @@ webrtc_fuzzer_test("audio_processing_fuzzer") { "../../modules/audio_processing/aec_dump:aec_dump_impl", "../../rtc_base:rtc_base_approved", "../../rtc_base:rtc_task_queue", + "../../rtc_base:rtc_task_queue_stdlib", "../../rtc_base:safe_minmax", "../../system_wrappers:field_trial", "//third_party/abseil-cpp/absl/memory", diff --git a/test/fuzzers/audio_processing_configs_fuzzer.cc b/test/fuzzers/audio_processing_configs_fuzzer.cc index 89c0552aab..58d1e465bf 100644 --- a/test/fuzzers/audio_processing_configs_fuzzer.cc +++ b/test/fuzzers/audio_processing_configs_fuzzer.cc @@ -18,6 +18,7 @@ #include "rtc_base/arraysize.h" #include "rtc_base/numerics/safe_minmax.h" #include "rtc_base/task_queue.h" +#include "rtc_base/task_queue_stdlib.h" #include "system_wrappers/include/field_trial.h" #include "test/fuzzers/audio_processing_fuzzer_helper.h" #include "test/fuzzers/fuzz_data_helper.h" @@ -148,6 +149,19 @@ std::unique_ptr CreateApm(test::FuzzDataHelper* fuzz_data, return apm; } + +TaskQueueFactory* GetTaskQueueFactory() { + // Chromium hijacked DefaultTaskQueueFactory with own implementation, but + // unable to use it without base::test::ScopedTaskEnvironment. Actual used + // task queue implementation shouldn't matter for the purpose of this fuzzer, + // so use stdlib implementation: that one is multiplatform. + // When bugs.webrtc.org/10284 is resolved and chromium stops hijacking + // DefaultTaskQueueFactory, Stdlib can be replaced with default one. + static TaskQueueFactory* const factory = + CreateTaskQueueStdlibFactory().release(); + return factory; +} + } // namespace void FuzzOneInput(const uint8_t* data, size_t size) { @@ -159,9 +173,9 @@ void FuzzOneInput(const uint8_t* data, size_t size) { // for field_trial.h. Hence it's created here and not in CreateApm. std::string field_trial_string = ""; - std::unique_ptr worker_queue( - new rtc::TaskQueue("rtc-low-prio", rtc::TaskQueue::Priority::LOW)); - auto apm = CreateApm(&fuzz_data, &field_trial_string, worker_queue.get()); + rtc::TaskQueue worker_queue(GetTaskQueueFactory()->CreateTaskQueue( + "rtc-low-prio", rtc::TaskQueue::Priority::LOW)); + auto apm = CreateApm(&fuzz_data, &field_trial_string, &worker_queue); if (apm) { FuzzAudioProcessing(&fuzz_data, std::move(apm)); diff --git a/test/fuzzers/webrtc_fuzzer_main.cc b/test/fuzzers/webrtc_fuzzer_main.cc index 3e05be632a..a52dd231be 100644 --- a/test/fuzzers/webrtc_fuzzer_main.cc +++ b/test/fuzzers/webrtc_fuzzer_main.cc @@ -12,9 +12,7 @@ // It's intended to set sane defaults, such as removing logging for further // fuzzing efficiency. -#include "api/task_queue/global_task_queue_factory.h" #include "rtc_base/logging.h" -#include "rtc_base/task_queue_stdlib.h" namespace { bool g_initialized = false; @@ -28,13 +26,6 @@ void InitializeWebRtcFuzzDefaults() { rtc::LogMessage::LogToDebug(rtc::LS_NONE); #endif // !defined(WEBRTC_CHROMIUM_BUILD) - // Chromium hijacked DefaultTaskQueueFactory with own implementation, but - // unable to use it without base::test::ScopedTaskEnvironment. Actual used - // task queue implementation shouldn't matter for the purpose of the fuzzers, - // so use stdlib implementation: that one is multiplatform. This is a - // temporary solution until bugs.webrtc.org/10284 is resolved. - webrtc::SetGlobalTaskQueueFactory(webrtc::CreateTaskQueueStdlibFactory()); - g_initialized = true; } } // namespace