From 92c357ed1537d189e22b16899da17933975488ac Mon Sep 17 00:00:00 2001 From: Chinmay Garde Date: Thu, 23 Mar 2017 15:29:33 -0700 Subject: [PATCH] Allow registering multiple task observers per message loop. (#3509) --- fml/BUILD.gn | 1 + fml/message_loop.cc | 8 ++++++-- fml/message_loop.h | 5 +++-- fml/message_loop_impl.cc | 22 +++++++++++++++++----- fml/message_loop_impl.h | 7 +++++-- fml/message_loop_unittests.cc | 19 ++++++++++++++++++- fml/task_observer.h | 21 +++++++++++++++++++++ sky/engine/web/Sky.cpp | 23 +++++++++++++++++++++-- travis/licenses_golden/licenses_flutter | 1 + 9 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 fml/task_observer.h diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 285fdd6e329e6..c8fe26e3b1e18 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -13,6 +13,7 @@ source_set("fml") { "message_loop_impl.cc", "message_loop_impl.h", "paths.h", + "task_observer.h", "task_runner.cc", "task_runner.h", "thread.cc", diff --git a/fml/message_loop.cc b/fml/message_loop.cc index d2888d75e55ee..87fc88e8f2916 100644 --- a/fml/message_loop.cc +++ b/fml/message_loop.cc @@ -63,8 +63,12 @@ ftl::RefPtr MessageLoop::GetLoopImpl() const { return loop_; } -void MessageLoop::SetTaskObserver(TaskObserver observer) { - loop_->SetTaskObserver(std::move(observer)); +void MessageLoop::AddTaskObserver(TaskObserver* observer) { + loop_->AddTaskObserver(observer); +} + +void MessageLoop::RemoveTaskObserver(TaskObserver* observer) { + loop_->RemoveTaskObserver(observer); } } // namespace fml diff --git a/fml/message_loop.h b/fml/message_loop.h index 04853deb0df78..cf49b4186aacb 100644 --- a/fml/message_loop.h +++ b/fml/message_loop.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_FML_MESSAGE_LOOP_H_ #define FLUTTER_FML_MESSAGE_LOOP_H_ +#include "flutter/fml/task_observer.h" #include "lib/ftl/macros.h" #include "lib/ftl/tasks/task_runner.h" @@ -23,9 +24,9 @@ class MessageLoop { void Terminate(); - using TaskObserver = std::function; + void AddTaskObserver(TaskObserver* observer); - void SetTaskObserver(TaskObserver observer); + void RemoveTaskObserver(TaskObserver* observer); ftl::RefPtr GetTaskRunner() const; diff --git a/fml/message_loop_impl.cc b/fml/message_loop_impl.cc index eaaa3b5a95079..56282a1a22d8b 100644 --- a/fml/message_loop_impl.cc +++ b/fml/message_loop_impl.cc @@ -4,6 +4,9 @@ #include "flutter/fml/message_loop_impl.h" +#include +#include + #include "lib/ftl/build_config.h" #if OS_MACOSX @@ -46,11 +49,20 @@ void MessageLoopImpl::RunExpiredTasksNow() { WakeUp(RunExpiredTasksAndGetNextWake()); } -void MessageLoopImpl::SetTaskObserver(MessageLoop::TaskObserver observer) { +void MessageLoopImpl::AddTaskObserver(TaskObserver* observer) { + FTL_DCHECK(observer != nullptr); FTL_DCHECK(MessageLoop::GetCurrent().GetLoopImpl().get() == this) - << "Message loop task observer must be set on the same thread as the " + << "Message loop task observer must be added on the same thread as the " "loop."; - task_observer_ = observer; + task_observers_.insert(observer); +} + +void MessageLoopImpl::RemoveTaskObserver(TaskObserver* observer) { + FTL_DCHECK(observer != nullptr); + FTL_DCHECK(MessageLoop::GetCurrent().GetLoopImpl().get() == this) + << "Message loop task observer must be removed from the same thread as " + "the loop."; + task_observers_.erase(observer); } void MessageLoopImpl::DoRun() { @@ -121,8 +133,8 @@ ftl::TimePoint MessageLoopImpl::RunExpiredTasksAndGetNextWake() { for (const auto& invocation : invocations) { invocation(); - if (task_observer_) { - task_observer_(); + for (const auto& observer : task_observers_) { + observer->DidProcessTask(); } } diff --git a/fml/message_loop_impl.h b/fml/message_loop_impl.h index b018b2fa1b98e..572778e0cce34 100644 --- a/fml/message_loop_impl.h +++ b/fml/message_loop_impl.h @@ -8,6 +8,7 @@ #include #include #include +#include #include #include "flutter/fml/message_loop.h" @@ -34,7 +35,9 @@ class MessageLoopImpl : public ftl::RefCountedThreadSafe { void PostTask(ftl::Closure task, ftl::TimePoint target_time); - void SetTaskObserver(MessageLoop::TaskObserver observer); + void AddTaskObserver(TaskObserver* observer); + + void RemoveTaskObserver(TaskObserver* observer); void DoRun(); @@ -67,7 +70,7 @@ class MessageLoopImpl : public ftl::RefCountedThreadSafe { using DelayedTaskQueue = std:: priority_queue, DelayedTaskCompare>; - MessageLoop::TaskObserver task_observer_; + std::set task_observers_; ftl::Mutex delayed_tasks_mutex_; DelayedTaskQueue delayed_tasks_ FTL_GUARDED_BY(delayed_tasks_mutex_); size_t order_ FTL_GUARDED_BY(delayed_tasks_mutex_); diff --git a/fml/message_loop_unittests.cc b/fml/message_loop_unittests.cc index ea4d2781c0fbb..1b36691315284 100644 --- a/fml/message_loop_unittests.cc +++ b/fml/message_loop_unittests.cc @@ -238,6 +238,22 @@ TEST(MessageLoop, TIME_SENSITIVE(MultipleDelayedTasksWithDecreasingDeltas)) { ASSERT_EQ(checked, count); } +class CustomTaskObserver : public fml::TaskObserver { + public: + CustomTaskObserver(std::function lambda) : lambda_(lambda){}; + + ~CustomTaskObserver() override = default; + + void DidProcessTask() { + if (lambda_) { + lambda_(); + } + }; + + private: + std::function lambda_; +}; + TEST(MessageLoop, TaskObserverFire) { bool started = false; bool terminated = false; @@ -247,7 +263,7 @@ TEST(MessageLoop, TaskObserverFire) { auto& loop = fml::MessageLoop::GetCurrent(); size_t task_count = 0; size_t obs_count = 0; - loop.SetTaskObserver([&obs_count]() { obs_count++; }); + CustomTaskObserver obs([&obs_count]() { obs_count++; }); for (size_t i = 0; i < count; i++) { loop.GetTaskRunner()->PostTask([&terminated, count, i, &task_count]() { ASSERT_EQ(task_count, i); @@ -258,6 +274,7 @@ TEST(MessageLoop, TaskObserverFire) { } }); } + loop.AddTaskObserver(&obs); loop.Run(); ASSERT_EQ(task_count, count); ASSERT_EQ(obs_count, count); diff --git a/fml/task_observer.h b/fml/task_observer.h new file mode 100644 index 0000000000000..b22a9bc43a907 --- /dev/null +++ b/fml/task_observer.h @@ -0,0 +1,21 @@ +// Copyright 2017 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_FML_TASK_OBSERVER_H_ +#define FLUTTER_FML_TASK_OBSERVER_H_ + +#include "lib/ftl/macros.h" + +namespace fml { + +class TaskObserver { + public: + virtual ~TaskObserver() = default; + + virtual void DidProcessTask() = 0; +}; + +} // namespace fml + +#endif // FLUTTER_FML_TASK_OBSERVER_H_ diff --git a/sky/engine/web/Sky.cpp b/sky/engine/web/Sky.cpp index 4e09b793dab52..192cebd5a82b5 100644 --- a/sky/engine/web/Sky.cpp +++ b/sky/engine/web/Sky.cpp @@ -72,12 +72,31 @@ void removeMessageLoopObservers() { #else // defined(OS_FUCHSIA) +class RunMicrotasksTaskObserver : public fml::TaskObserver { + public: + RunMicrotasksTaskObserver() = default; + + ~RunMicrotasksTaskObserver() override = default; + + void DidProcessTask() override { didProcessTask(); } +}; + +// FIXME(chinmaygarde): The awkward use of the global here is be cause we cannot +// introduce the fml::TaskObserver subclass in common code because Fuchsia does +// not support the same. Unify the API and remove hack. +static RunMicrotasksTaskObserver* g_run_microtasks_task_observer = nullptr; + void addMessageLoopObservers() { - fml::MessageLoop::GetCurrent().SetTaskObserver(&didProcessTask); + g_run_microtasks_task_observer = new RunMicrotasksTaskObserver(); + fml::MessageLoop::GetCurrent().AddTaskObserver( + g_run_microtasks_task_observer); } void removeMessageLoopObservers() { - fml::MessageLoop::GetCurrent().SetTaskObserver(nullptr); + fml::MessageLoop::GetCurrent().RemoveTaskObserver( + g_run_microtasks_task_observer); + delete g_run_microtasks_task_observer; + g_run_microtasks_task_observer = nullptr; } #endif // defined(OS_FUCHSIA) diff --git a/travis/licenses_golden/licenses_flutter b/travis/licenses_golden/licenses_flutter index 7b65041940dda..881148e0dec52 100644 --- a/travis/licenses_golden/licenses_flutter +++ b/travis/licenses_golden/licenses_flutter @@ -1940,6 +1940,7 @@ FILE: ../../../flutter/fml/platform/linux/message_loop_linux.h FILE: ../../../flutter/fml/platform/linux/paths_linux.cc FILE: ../../../flutter/fml/platform/linux/timerfd.cc FILE: ../../../flutter/fml/platform/linux/timerfd.h +FILE: ../../../flutter/fml/task_observer.h FILE: ../../../flutter/fml/task_runner.cc FILE: ../../../flutter/fml/task_runner.h FILE: ../../../flutter/fml/thread.cc