From 39b2739a058d6afcdb8127fb8cc9a5f20cbc3cca Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Wed, 12 Jun 2019 13:28:59 -0700 Subject: [PATCH 1/6] Refactor to move Task Queue to its own class - This is to help with sharing task queue among multiple message loops going forward. - currently there is 1:1 mapping between task queue and message loop, we are still maintaining the semantics for this change. --- ci/licenses_golden/licenses_flutter | 2 + fml/BUILD.gn | 2 + fml/message_loop_impl.cc | 79 ++++++------------------- fml/message_loop_impl.h | 15 +---- fml/message_loop_task_queue.cc | 92 +++++++++++++++++++++++++++++ fml/message_loop_task_queue.h | 72 ++++++++++++++++++++++ 6 files changed, 189 insertions(+), 73 deletions(-) create mode 100644 fml/message_loop_task_queue.cc create mode 100644 fml/message_loop_task_queue.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index e69dd41f3f3d2..8ed21a8a8f3c3 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -134,6 +134,8 @@ FILE: ../../../flutter/fml/message_loop.cc FILE: ../../../flutter/fml/message_loop.h FILE: ../../../flutter/fml/message_loop_impl.cc FILE: ../../../flutter/fml/message_loop_impl.h +FILE: ../../../flutter/fml/message_loop_task_queue.cc +FILE: ../../../flutter/fml/message_loop_task_queue.h FILE: ../../../flutter/fml/message_loop_unittests.cc FILE: ../../../flutter/fml/message_unittests.cc FILE: ../../../flutter/fml/native_library.h diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 506d4c3508561..528c4e1b9842a 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -46,6 +46,8 @@ source_set("fml") { "message_loop.h", "message_loop_impl.cc", "message_loop_impl.h", + "message_loop_task_queue.cc", + "message_loop_task_queue.h", "native_library.h", "paths.cc", "paths.h", diff --git a/fml/message_loop_impl.cc b/fml/message_loop_impl.cc index f1efe0a36e8d4..5d8f4b9984c00 100644 --- a/fml/message_loop_impl.cc +++ b/fml/message_loop_impl.cc @@ -39,13 +39,22 @@ fml::RefPtr MessageLoopImpl::Create() { #endif } -MessageLoopImpl::MessageLoopImpl() : order_(0), terminated_(false) {} +MessageLoopImpl::MessageLoopImpl() : terminated_(false) { + task_queue_ = std::make_unique(); +} MessageLoopImpl::~MessageLoopImpl() = default; void MessageLoopImpl::PostTask(fml::closure task, fml::TimePoint target_time) { FML_DCHECK(task != nullptr); - RegisterTask(task, target_time); + FML_DCHECK(task != nullptr); + if (terminated_) { + // If the message loop has already been terminated, PostTask should destruct + // |task| synchronously within this function. + return; + } + const auto wake_up = task_queue_->RegisterTask(task, target_time); + WakeUp(wake_up); } void MessageLoopImpl::AddTaskObserver(intptr_t key, fml::closure callback) { @@ -53,16 +62,14 @@ void MessageLoopImpl::AddTaskObserver(intptr_t key, fml::closure callback) { FML_DCHECK(MessageLoop::GetCurrent().GetLoopImpl().get() == this) << "Message loop task observer must be added on the same thread as the " "loop."; - std::lock_guard observers_lock(observers_mutex_); - task_observers_[key] = std::move(callback); + task_queue_->AddTaskObserver(key, callback); } void MessageLoopImpl::RemoveTaskObserver(intptr_t key) { FML_DCHECK(MessageLoop::GetCurrent().GetLoopImpl().get() == this) << "Message loop task observer must be removed from the same thread as " "the loop."; - std::lock_guard observers_lock(observers_mutex_); - task_observers_.erase(key); + task_queue_->RemoveTaskObserver(key); } void MessageLoopImpl::DoRun() { @@ -88,8 +95,7 @@ void MessageLoopImpl::DoRun() { // should be destructed on the message loop's thread. We have just returned // from the implementations |Run| method which we know is on the correct // thread. Drop all pending tasks on the floor. - std::lock_guard lock(delayed_tasks_mutex_); - delayed_tasks_ = {}; + task_queue_->Dispose(); } void MessageLoopImpl::DoTerminate() { @@ -109,31 +115,8 @@ void MessageLoopImpl::SwapTaskQueues(const fml::RefPtr& other) std::unique_lock t2(other->tasks_flushing_mutex_, std::defer_lock); - // task_observers locks - std::unique_lock o1(observers_mutex_, std::defer_lock); - std::unique_lock o2(other->observers_mutex_, std::defer_lock); - - // delayed_tasks locks - std::unique_lock d1(delayed_tasks_mutex_, std::defer_lock); - std::unique_lock d2(other->delayed_tasks_mutex_, std::defer_lock); - - std::lock(t1, t2, o1, o2, d1, d2); - - std::swap(task_observers_, other->task_observers_); - std::swap(delayed_tasks_, other->delayed_tasks_); -} - -void MessageLoopImpl::RegisterTask(fml::closure task, - fml::TimePoint target_time) { - FML_DCHECK(task != nullptr); - if (terminated_) { - // If the message loop has already been terminated, PostTask should destruct - // |task| synchronously within this function. - return; - } - std::lock_guard lock(delayed_tasks_mutex_); - delayed_tasks_.push({++order_, std::move(task), target_time}); - WakeUp(delayed_tasks_.top().GetTargetTime()); + std::lock(t1, t2); + task_queue_->Swap(*other->task_queue_); } void MessageLoopImpl::FlushTasks(FlushType type) { @@ -148,36 +131,12 @@ void MessageLoopImpl::FlushTasks(FlushType type) { // will lead us to run invocations on the wrong thread. std::lock_guard task_flush_lock(tasks_flushing_mutex_); - { - std::lock_guard lock(delayed_tasks_mutex_); - - if (delayed_tasks_.empty()) { - return; - } - - auto now = fml::TimePoint::Now(); - while (!delayed_tasks_.empty()) { - const auto& top = delayed_tasks_.top(); - if (top.GetTargetTime() > now) { - break; - } - invocations.emplace_back(std::move(top.GetTask())); - delayed_tasks_.pop(); - if (type == FlushType::kSingle) { - break; - } - } - - WakeUp(delayed_tasks_.empty() ? fml::TimePoint::Max() - : delayed_tasks_.top().GetTargetTime()); - } + const auto wake_up = task_queue_->GetTasksToRunNow(type, invocations); + WakeUp(wake_up); for (const auto& invocation : invocations) { invocation(); - std::lock_guard observers_lock(observers_mutex_); - for (const auto& observer : task_observers_) { - observer.second(); - } + task_queue_->NotifyObservers(); } } diff --git a/fml/message_loop_impl.h b/fml/message_loop_impl.h index e4aad707fa5a4..8fa85aed007ce 100644 --- a/fml/message_loop_impl.h +++ b/fml/message_loop_impl.h @@ -17,6 +17,7 @@ #include "flutter/fml/macros.h" #include "flutter/fml/memory/ref_counted.h" #include "flutter/fml/message_loop.h" +#include "flutter/fml/message_loop_task_queue.h" #include "flutter/fml/synchronization/thread_annotations.h" #include "flutter/fml/time/time_point.h" @@ -61,21 +62,9 @@ class MessageLoopImpl : public fml::RefCountedThreadSafe { private: std::mutex tasks_flushing_mutex_; - std::mutex observers_mutex_; - std::map task_observers_ - FML_GUARDED_BY(observers_mutex_); - - std::mutex delayed_tasks_mutex_; - DelayedTaskQueue delayed_tasks_ FML_GUARDED_BY(delayed_tasks_mutex_); - size_t order_ FML_GUARDED_BY(delayed_tasks_mutex_); + std::unique_ptr task_queue_; std::atomic_bool terminated_; - void RegisterTask(fml::closure task, fml::TimePoint target_time); - - enum class FlushType { - kSingle, - kAll, - }; void FlushTasks(FlushType type); FML_DISALLOW_COPY_AND_ASSIGN(MessageLoopImpl); diff --git a/fml/message_loop_task_queue.cc b/fml/message_loop_task_queue.cc new file mode 100644 index 0000000000000..4d7b48e1ae154 --- /dev/null +++ b/fml/message_loop_task_queue.cc @@ -0,0 +1,92 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#define FML_USED_ON_EMBEDDER + +#include "flutter/fml/message_loop_task_queue.h" + +namespace fml { + +MessageLoopTaskQueue::MessageLoopTaskQueue() : order_(0) {} + +MessageLoopTaskQueue::~MessageLoopTaskQueue() = default; + +void MessageLoopTaskQueue::Dispose() { + std::lock_guard lock(delayed_tasks_mutex_); + delayed_tasks_ = {}; +} + +fml::TimePoint MessageLoopTaskQueue::RegisterTask(fml::closure task, + fml::TimePoint target_time) { + std::lock_guard lock(delayed_tasks_mutex_); + delayed_tasks_.push({++order_, std::move(task), target_time}); + return delayed_tasks_.top().GetTargetTime(); +} + +bool MessageLoopTaskQueue::HasPendingTasks() { + std::lock_guard lock(delayed_tasks_mutex_); + return !delayed_tasks_.empty(); +} + +fml::TimePoint MessageLoopTaskQueue::GetTasksToRunNow( + FlushType type, + std::vector& invocations) { + std::lock_guard lock(delayed_tasks_mutex_); + + const auto now = fml::TimePoint::Now(); + while (!delayed_tasks_.empty()) { + const auto& top = delayed_tasks_.top(); + if (top.GetTargetTime() > now) { + break; + } + invocations.emplace_back(std::move(top.GetTask())); + delayed_tasks_.pop(); + if (type == FlushType::kSingle) { + break; + } + } + + if (delayed_tasks_.empty()) { + return fml::TimePoint::Max(); + } else { + return delayed_tasks_.top().GetTargetTime(); + } +} + +void MessageLoopTaskQueue::AddTaskObserver(intptr_t key, + fml::closure callback) { + std::lock_guard observers_lock(observers_mutex_); + task_observers_[key] = std::move(callback); +} + +void MessageLoopTaskQueue::RemoveTaskObserver(intptr_t key) { + std::lock_guard observers_lock(observers_mutex_); + task_observers_.erase(key); +} + +void MessageLoopTaskQueue::NotifyObservers() { + std::lock_guard observers_lock(observers_mutex_); + for (const auto& observer : task_observers_) { + observer.second(); + } +} + +// Thread safety analysis disabled as it does not account for defered locks. +void MessageLoopTaskQueue::Swap(MessageLoopTaskQueue& other) + FML_NO_THREAD_SAFETY_ANALYSIS { + // task_observers locks + std::unique_lock o1(observers_mutex_, std::defer_lock); + std::unique_lock o2(other.observers_mutex_, std::defer_lock); + + // delayed_tasks locks + std::unique_lock d1(delayed_tasks_mutex_, std::defer_lock); + std::unique_lock d2(other.delayed_tasks_mutex_, std::defer_lock); + + std::lock(o1, o2, d1, d2); + + std::swap(task_observers_, other.task_observers_); + std::swap(delayed_tasks_, other.delayed_tasks_); +} + +} // namespace fml diff --git a/fml/message_loop_task_queue.h b/fml/message_loop_task_queue.h new file mode 100644 index 0000000000000..63c2f8a09c2d6 --- /dev/null +++ b/fml/message_loop_task_queue.h @@ -0,0 +1,72 @@ +// Copyright 2013 The Flutter 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_MESSAGE_LOOP_TASK_QUEUE_H_ +#define FLUTTER_FML_MESSAGE_LOOP_TASK_QUEUE_H_ + +#include +#include + +#include "flutter/fml/closure.h" +#include "flutter/fml/delayed_task.h" +#include "flutter/fml/macros.h" +#include "flutter/fml/memory/ref_counted.h" +#include "flutter/fml/synchronization/thread_annotations.h" + +namespace fml { + +enum class FlushType { + kSingle, + kAll, +}; + +// This class keeps track of all the tasks and observers that +// need to be run on it's MessageLoopImpl. +class MessageLoopTaskQueue { + public: + // Lifecycle. + + MessageLoopTaskQueue(); + + ~MessageLoopTaskQueue(); + + void Dispose(); + + // Tasks methods. + + fml::TimePoint RegisterTask(fml::closure task, fml::TimePoint target_time); + + bool HasPendingTasks(); + + // Returns the wake up time. + fml::TimePoint GetTasksToRunNow(FlushType type, + std::vector& invocations); + + // Observers methods. + + void AddTaskObserver(intptr_t key, fml::closure callback); + + void RemoveTaskObserver(intptr_t key); + + void NotifyObservers(); + + // Misc. + + void Swap(MessageLoopTaskQueue& other); + + private: + std::mutex observers_mutex_; + std::map task_observers_ + FML_GUARDED_BY(observers_mutex_); + + std::mutex delayed_tasks_mutex_; + DelayedTaskQueue delayed_tasks_ FML_GUARDED_BY(delayed_tasks_mutex_); + size_t order_ FML_GUARDED_BY(delayed_tasks_mutex_); + + FML_DISALLOW_COPY_ASSIGN_AND_MOVE(MessageLoopTaskQueue); +}; + +} // namespace fml + +#endif // FLUTTER_FML_MESSAGE_LOOP_TASK_QUEUE_H_ From 55e6584fe94fdb71a8b2c2146f60456c09752b52 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Wed, 12 Jun 2019 13:38:15 -0700 Subject: [PATCH 2/6] Add mutex include --- fml/message_loop_task_queue.h | 1 + 1 file changed, 1 insertion(+) diff --git a/fml/message_loop_task_queue.h b/fml/message_loop_task_queue.h index 63c2f8a09c2d6..020200d760117 100644 --- a/fml/message_loop_task_queue.h +++ b/fml/message_loop_task_queue.h @@ -6,6 +6,7 @@ #define FLUTTER_FML_MESSAGE_LOOP_TASK_QUEUE_H_ #include +#include #include #include "flutter/fml/closure.h" From 0d25713d6253fc193103a988028ce748113f0c10 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Wed, 12 Jun 2019 15:11:59 -0700 Subject: [PATCH 3/6] Most of the waking up changes minus test failures --- fml/message_loop_impl.cc | 8 +++----- fml/message_loop_task_queue.cc | 22 ++++++++++++++++------ fml/message_loop_task_queue.h | 17 ++++++++++++----- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/fml/message_loop_impl.cc b/fml/message_loop_impl.cc index 5d8f4b9984c00..9e74f2639b6d7 100644 --- a/fml/message_loop_impl.cc +++ b/fml/message_loop_impl.cc @@ -41,6 +41,7 @@ fml::RefPtr MessageLoopImpl::Create() { MessageLoopImpl::MessageLoopImpl() : terminated_(false) { task_queue_ = std::make_unique(); + task_queue_->SetLoop(this); } MessageLoopImpl::~MessageLoopImpl() = default; @@ -53,8 +54,7 @@ void MessageLoopImpl::PostTask(fml::closure task, fml::TimePoint target_time) { // |task| synchronously within this function. return; } - const auto wake_up = task_queue_->RegisterTask(task, target_time); - WakeUp(wake_up); + task_queue_->RegisterTask(task, target_time); } void MessageLoopImpl::AddTaskObserver(intptr_t key, fml::closure callback) { @@ -130,9 +130,7 @@ void MessageLoopImpl::FlushTasks(FlushType type) { // gather invocations -> Swap -> execute invocations // will lead us to run invocations on the wrong thread. std::lock_guard task_flush_lock(tasks_flushing_mutex_); - - const auto wake_up = task_queue_->GetTasksToRunNow(type, invocations); - WakeUp(wake_up); + task_queue_->GetTasksToRunNow(type, invocations); for (const auto& invocation : invocations) { invocation(); diff --git a/fml/message_loop_task_queue.cc b/fml/message_loop_task_queue.cc index 4d7b48e1ae154..886ada760eca0 100644 --- a/fml/message_loop_task_queue.cc +++ b/fml/message_loop_task_queue.cc @@ -5,6 +5,7 @@ #define FML_USED_ON_EMBEDDER #include "flutter/fml/message_loop_task_queue.h" +#include "flutter/fml/message_loop_impl.h" namespace fml { @@ -17,11 +18,11 @@ void MessageLoopTaskQueue::Dispose() { delayed_tasks_ = {}; } -fml::TimePoint MessageLoopTaskQueue::RegisterTask(fml::closure task, - fml::TimePoint target_time) { +void MessageLoopTaskQueue::RegisterTask(fml::closure task, + fml::TimePoint target_time) { std::lock_guard lock(delayed_tasks_mutex_); delayed_tasks_.push({++order_, std::move(task), target_time}); - return delayed_tasks_.top().GetTargetTime(); + WakeUp(delayed_tasks_.top().GetTargetTime()); } bool MessageLoopTaskQueue::HasPendingTasks() { @@ -29,7 +30,7 @@ bool MessageLoopTaskQueue::HasPendingTasks() { return !delayed_tasks_.empty(); } -fml::TimePoint MessageLoopTaskQueue::GetTasksToRunNow( +void MessageLoopTaskQueue::GetTasksToRunNow( FlushType type, std::vector& invocations) { std::lock_guard lock(delayed_tasks_mutex_); @@ -48,12 +49,17 @@ fml::TimePoint MessageLoopTaskQueue::GetTasksToRunNow( } if (delayed_tasks_.empty()) { - return fml::TimePoint::Max(); + WakeUp(fml::TimePoint::Max()); } else { - return delayed_tasks_.top().GetTargetTime(); + WakeUp(delayed_tasks_.top().GetTargetTime()); } } +void MessageLoopTaskQueue::WakeUp(fml::TimePoint time) { + FML_CHECK(loop_.get()) << "Loop should be set by calling SetLoop"; + loop_->WakeUp(time); +} + void MessageLoopTaskQueue::AddTaskObserver(intptr_t key, fml::closure callback) { std::lock_guard observers_lock(observers_mutex_); @@ -89,4 +95,8 @@ void MessageLoopTaskQueue::Swap(MessageLoopTaskQueue& other) std::swap(delayed_tasks_, other.delayed_tasks_); } +void MessageLoopTaskQueue::SetLoop(fml::MessageLoopImpl* loop) { + loop_.reset(loop); +} + } // namespace fml diff --git a/fml/message_loop_task_queue.h b/fml/message_loop_task_queue.h index 020200d760117..6582406de880b 100644 --- a/fml/message_loop_task_queue.h +++ b/fml/message_loop_task_queue.h @@ -22,8 +22,11 @@ enum class FlushType { kAll, }; +class MessageLoopImpl; + // This class keeps track of all the tasks and observers that -// need to be run on it's MessageLoopImpl. +// need to be run on it's MessageLoopImpl. This also wakes up the +// loop at the required times. class MessageLoopTaskQueue { public: // Lifecycle. @@ -36,13 +39,11 @@ class MessageLoopTaskQueue { // Tasks methods. - fml::TimePoint RegisterTask(fml::closure task, fml::TimePoint target_time); + void RegisterTask(fml::closure task, fml::TimePoint target_time); bool HasPendingTasks(); - // Returns the wake up time. - fml::TimePoint GetTasksToRunNow(FlushType type, - std::vector& invocations); + void GetTasksToRunNow(FlushType type, std::vector& invocations); // Observers methods. @@ -56,7 +57,13 @@ class MessageLoopTaskQueue { void Swap(MessageLoopTaskQueue& other); + void SetLoop(fml::MessageLoopImpl* loop); + private: + void WakeUp(fml::TimePoint time); + + std::unique_ptr loop_; + std::mutex observers_mutex_; std::map task_observers_ FML_GUARDED_BY(observers_mutex_); From a1a63c9bdafecdc6790a57b29c8ec5d95a09dc04 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Thu, 13 Jun 2019 11:47:11 -0700 Subject: [PATCH 4/6] Refactor MessageLoopImpl to be Wakeable - Makes testing easier by letting us putting a TestWakeable - Also move the waking up logic to the task queue --- ci/licenses_golden/licenses_flutter | 1 + fml/BUILD.gn | 1 + fml/message_loop_impl.cc | 2 +- fml/message_loop_impl.h | 6 +++--- fml/message_loop_task_queue.cc | 9 +++++---- fml/message_loop_task_queue.h | 7 +++---- fml/message_loop_task_queue_unittests.cc | 25 +++++++++++++++++++++--- fml/wakeable.h | 21 ++++++++++++++++++++ 8 files changed, 57 insertions(+), 15 deletions(-) create mode 100644 fml/wakeable.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index 873841def76fa..a8242e8f56f66 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -224,6 +224,7 @@ FILE: ../../../flutter/fml/trace_event.h FILE: ../../../flutter/fml/unique_fd.cc FILE: ../../../flutter/fml/unique_fd.h FILE: ../../../flutter/fml/unique_object.h +FILE: ../../../flutter/fml/wakeable.h FILE: ../../../flutter/lib/io/dart_io.cc FILE: ../../../flutter/lib/io/dart_io.h FILE: ../../../flutter/lib/snapshot/libraries.json diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 538699ad0ae1e..0e23e9724e427 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -77,6 +77,7 @@ source_set("fml") { "unique_fd.cc", "unique_fd.h", "unique_object.h", + "wakeable.h", ] public_deps = [] diff --git a/fml/message_loop_impl.cc b/fml/message_loop_impl.cc index 9e74f2639b6d7..5ee993778f142 100644 --- a/fml/message_loop_impl.cc +++ b/fml/message_loop_impl.cc @@ -41,7 +41,7 @@ fml::RefPtr MessageLoopImpl::Create() { MessageLoopImpl::MessageLoopImpl() : terminated_(false) { task_queue_ = std::make_unique(); - task_queue_->SetLoop(this); + task_queue_->SetWakeable(this); } MessageLoopImpl::~MessageLoopImpl() = default; diff --git a/fml/message_loop_impl.h b/fml/message_loop_impl.h index 8fa85aed007ce..3ff5d9fcf533d 100644 --- a/fml/message_loop_impl.h +++ b/fml/message_loop_impl.h @@ -20,10 +20,12 @@ #include "flutter/fml/message_loop_task_queue.h" #include "flutter/fml/synchronization/thread_annotations.h" #include "flutter/fml/time/time_point.h" +#include "flutter/fml/wakeable.h" namespace fml { -class MessageLoopImpl : public fml::RefCountedThreadSafe { +class MessageLoopImpl : public Wakeable, + public fml::RefCountedThreadSafe { public: static fml::RefPtr Create(); @@ -33,8 +35,6 @@ class MessageLoopImpl : public fml::RefCountedThreadSafe { virtual void Terminate() = 0; - virtual void WakeUp(fml::TimePoint time_point) = 0; - void PostTask(fml::closure task, fml::TimePoint target_time); void AddTaskObserver(intptr_t key, fml::closure callback); diff --git a/fml/message_loop_task_queue.cc b/fml/message_loop_task_queue.cc index d443da866896a..0ca6df8339fa0 100644 --- a/fml/message_loop_task_queue.cc +++ b/fml/message_loop_task_queue.cc @@ -56,8 +56,9 @@ void MessageLoopTaskQueue::GetTasksToRunNow( } void MessageLoopTaskQueue::WakeUp(fml::TimePoint time) { - FML_CHECK(loop_.get()) << "Loop should be set by calling SetLoop"; - loop_->WakeUp(time); + if (wakeable_.get()) { + wakeable_->WakeUp(time); + } } size_t MessageLoopTaskQueue::GetNumPendingTasks() { @@ -100,8 +101,8 @@ void MessageLoopTaskQueue::Swap(MessageLoopTaskQueue& other) std::swap(delayed_tasks_, other.delayed_tasks_); } -void MessageLoopTaskQueue::SetLoop(fml::MessageLoopImpl* loop) { - loop_.reset(loop); +void MessageLoopTaskQueue::SetWakeable(fml::Wakeable* wakeable) { + wakeable_.reset(wakeable); } } // namespace fml diff --git a/fml/message_loop_task_queue.h b/fml/message_loop_task_queue.h index ef1892e640aab..a73e0b402329a 100644 --- a/fml/message_loop_task_queue.h +++ b/fml/message_loop_task_queue.h @@ -14,6 +14,7 @@ #include "flutter/fml/macros.h" #include "flutter/fml/memory/ref_counted.h" #include "flutter/fml/synchronization/thread_annotations.h" +#include "flutter/fml/wakeable.h" namespace fml { @@ -22,8 +23,6 @@ enum class FlushType { kAll, }; -class MessageLoopImpl; - // This class keeps track of all the tasks and observers that // need to be run on it's MessageLoopImpl. This also wakes up the // loop at the required times. @@ -59,12 +58,12 @@ class MessageLoopTaskQueue { void Swap(MessageLoopTaskQueue& other); - void SetLoop(fml::MessageLoopImpl* loop); + void SetWakeable(fml::Wakeable* wakeable); private: void WakeUp(fml::TimePoint time); - std::unique_ptr loop_; + std::unique_ptr wakeable_; std::mutex observers_mutex_; std::map task_observers_ diff --git a/fml/message_loop_task_queue_unittests.cc b/fml/message_loop_task_queue_unittests.cc index 06077bcb5a265..8c55860573a2a 100644 --- a/fml/message_loop_task_queue_unittests.cc +++ b/fml/message_loop_task_queue_unittests.cc @@ -7,18 +7,37 @@ #include "flutter/fml/message_loop_task_queue.h" #include "gtest/gtest.h" +class TestWakeable : public fml::Wakeable { + public: + using WakeUpCall = std::function; + + TestWakeable(WakeUpCall call) : wake_up_call_(call) {} + + void WakeUp(fml::TimePoint time_point) override { + wake_up_call_(time_point); + } + + private: + WakeUpCall wake_up_call_; +}; + TEST(MessageLoopTaskQueue, StartsWithNoPendingTasks) { auto task_queue = std::make_unique(); ASSERT_FALSE(task_queue->HasPendingTasks()); } TEST(MessageLoopTaskQueue, RegisterOneTask) { - auto task_queue = std::make_unique(); const auto time = fml::TimePoint::Max(); - const auto wake_time = task_queue->RegisterTask([] {}, time); + + auto task_queue = std::make_unique(); + task_queue->SetWakeable(new TestWakeable([&time](fml::TimePoint wake_time) { + ASSERT_TRUE(wake_time == time); + })); + + task_queue->RegisterTask([] {}, time); ASSERT_TRUE(task_queue->HasPendingTasks()); ASSERT_TRUE(task_queue->GetNumPendingTasks() == 1); - ASSERT_TRUE(wake_time == time); + } TEST(MessageLoopTaskQueue, RegisterTwoTasksAndCount) { diff --git a/fml/wakeable.h b/fml/wakeable.h new file mode 100644 index 0000000000000..a4cf28a076df6 --- /dev/null +++ b/fml/wakeable.h @@ -0,0 +1,21 @@ +// Copyright 2019 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_WAKEABLE_H_ +#define FLUTTER_FML_WAKEABLE_H_ + +#include "flutter/fml/time/time_point.h" + +namespace fml { + +class Wakeable { + public: + virtual ~Wakeable() {} + + virtual void WakeUp(fml::TimePoint time_point) = 0; +}; + +} // namespace fml + +#endif // FLUTTER_FML_WAKEABLE_H_ From 7aaef58d13e9914a53f1feaf50f92419947e7f0b Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Thu, 13 Jun 2019 13:08:26 -0700 Subject: [PATCH 5/6] add tests --- fml/message_loop_task_queue.cc | 4 +- fml/message_loop_task_queue.h | 2 +- fml/message_loop_task_queue_unittests.cc | 62 +++++++++++++++++++++--- 3 files changed, 57 insertions(+), 11 deletions(-) diff --git a/fml/message_loop_task_queue.cc b/fml/message_loop_task_queue.cc index 0ca6df8339fa0..ef3ac3348e7eb 100644 --- a/fml/message_loop_task_queue.cc +++ b/fml/message_loop_task_queue.cc @@ -56,7 +56,7 @@ void MessageLoopTaskQueue::GetTasksToRunNow( } void MessageLoopTaskQueue::WakeUp(fml::TimePoint time) { - if (wakeable_.get()) { + if (wakeable_) { wakeable_->WakeUp(time); } } @@ -102,7 +102,7 @@ void MessageLoopTaskQueue::Swap(MessageLoopTaskQueue& other) } void MessageLoopTaskQueue::SetWakeable(fml::Wakeable* wakeable) { - wakeable_.reset(wakeable); + wakeable_ = wakeable; } } // namespace fml diff --git a/fml/message_loop_task_queue.h b/fml/message_loop_task_queue.h index a73e0b402329a..8be179311c32b 100644 --- a/fml/message_loop_task_queue.h +++ b/fml/message_loop_task_queue.h @@ -63,7 +63,7 @@ class MessageLoopTaskQueue { private: void WakeUp(fml::TimePoint time); - std::unique_ptr wakeable_; + Wakeable* wakeable_ = NULL; std::mutex observers_mutex_; std::map task_observers_ diff --git a/fml/message_loop_task_queue_unittests.cc b/fml/message_loop_task_queue_unittests.cc index 8c55860573a2a..a38a6e1047705 100644 --- a/fml/message_loop_task_queue_unittests.cc +++ b/fml/message_loop_task_queue_unittests.cc @@ -5,17 +5,17 @@ #define FML_USED_ON_EMBEDDER #include "flutter/fml/message_loop_task_queue.h" +#include "flutter/fml/synchronization/count_down_latch.h" +#include "flutter/fml/synchronization/waitable_event.h" #include "gtest/gtest.h" class TestWakeable : public fml::Wakeable { public: - using WakeUpCall = std::function; + using WakeUpCall = std::function; TestWakeable(WakeUpCall call) : wake_up_call_(call) {} - void WakeUp(fml::TimePoint time_point) override { - wake_up_call_(time_point); - } + void WakeUp(fml::TimePoint time_point) override { wake_up_call_(time_point); } private: WakeUpCall wake_up_call_; @@ -30,14 +30,12 @@ TEST(MessageLoopTaskQueue, RegisterOneTask) { const auto time = fml::TimePoint::Max(); auto task_queue = std::make_unique(); - task_queue->SetWakeable(new TestWakeable([&time](fml::TimePoint wake_time) { - ASSERT_TRUE(wake_time == time); - })); + task_queue->SetWakeable(new TestWakeable( + [&time](fml::TimePoint wake_time) { ASSERT_TRUE(wake_time == time); })); task_queue->RegisterTask([] {}, time); ASSERT_TRUE(task_queue->HasPendingTasks()); ASSERT_TRUE(task_queue->GetNumPendingTasks() == 1); - } TEST(MessageLoopTaskQueue, RegisterTwoTasksAndCount) { @@ -87,3 +85,51 @@ TEST(MessageLoopTaskQueue, AddRemoveNotifyObservers) { task_queue->NotifyObservers(); ASSERT_TRUE(test_val == 0); } + +TEST(MessageLoopTaskQueue, WakeUpIndependentOfTime) { + auto task_queue = std::make_unique(); + + int num_wakes = 0; + task_queue->SetWakeable(new TestWakeable( + [&num_wakes](fml::TimePoint wake_time) { ++num_wakes; })); + + task_queue->RegisterTask([]() {}, fml::TimePoint::Now()); + task_queue->RegisterTask([]() {}, fml::TimePoint::Max()); + + ASSERT_TRUE(num_wakes == 2); +} + +TEST(MessageLoopTaskQueue, WakeUpWithMaxIfNoInvocations) { + auto task_queue = std::make_unique(); + fml::AutoResetWaitableEvent ev; + + task_queue->SetWakeable(new TestWakeable([&ev](fml::TimePoint wake_time) { + ASSERT_TRUE(wake_time == fml::TimePoint::Max()); + ev.Signal(); + })); + + std::vector invocations; + task_queue->GetTasksToRunNow(fml::FlushType::kAll, invocations); + ev.Wait(); +} + +TEST(MessageLoopTaskQueue, WokenUpWithNewerTime) { + auto task_queue = std::make_unique(); + fml::CountDownLatch latch(2); + + fml::TimePoint expected = fml::TimePoint::Max(); + + task_queue->SetWakeable( + new TestWakeable([&latch, &expected](fml::TimePoint wake_time) { + ASSERT_TRUE(wake_time == expected); + latch.CountDown(); + })); + + task_queue->RegisterTask([]() {}, fml::TimePoint::Max()); + + const auto now = fml::TimePoint::Now(); + expected = now; + task_queue->RegisterTask([]() {}, now); + + latch.Wait(); +} From 3bdcc31ebcd776a536688565183cd87c40b39434 Mon Sep 17 00:00:00 2001 From: Kaushik Iska Date: Thu, 13 Jun 2019 13:48:06 -0700 Subject: [PATCH 6/6] Fix formatting and license --- fml/BUILD.gn | 4 ++-- fml/wakeable.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/fml/BUILD.gn b/fml/BUILD.gn index 0e23e9724e427..de37396fe3614 100644 --- a/fml/BUILD.gn +++ b/fml/BUILD.gn @@ -1,5 +1,5 @@ -# Copyright 2013 The Flutter Authors.All rights reserved. -# Use of this source code is governed by a BSD - style license that can be +# Copyright 2013 The Flutter Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. import("//build/fuchsia/sdk.gni") diff --git a/fml/wakeable.h b/fml/wakeable.h index a4cf28a076df6..2b36139817644 100644 --- a/fml/wakeable.h +++ b/fml/wakeable.h @@ -1,4 +1,4 @@ -// Copyright 2019 The Chromium Authors. All rights reserved. +// Copyright 2013 The Flutter Authors. All rights reserved. // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file.