Allow registering multiple task observers per message loop. (flutter#…

…3509)
agray86 · Mar 23, 2017 · 92c357e · 92c357e
1 parent d119d62
commit 92c357e
Show file tree

Hide file tree

Showing 9 changed files with 93 additions and 14 deletions.
diff --git 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
@@ -63,8 +63,12 @@ ftl::RefPtr<MessageLoopImpl> 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
@@ -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(void)>;
+  void AddTaskObserver(TaskObserver* observer);
 
-  void SetTaskObserver(TaskObserver observer);
+  void RemoveTaskObserver(TaskObserver* observer);
 
   ftl::RefPtr<ftl::TaskRunner> GetTaskRunner() const;
 

diff --git a/fml/message_loop_impl.cc b/fml/message_loop_impl.cc
@@ -4,6 +4,9 @@
 
 #include "flutter/fml/message_loop_impl.h"
 
+#include <algorithm>
+#include <vector>
+
 #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
@@ -8,6 +8,7 @@
 #include <atomic>
 #include <deque>
 #include <queue>
+#include <set>
 #include <utility>
 
 #include "flutter/fml/message_loop.h"
@@ -34,7 +35,9 @@ class MessageLoopImpl : public ftl::RefCountedThreadSafe<MessageLoopImpl> {
 
   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<MessageLoopImpl> {
   using DelayedTaskQueue = std::
       priority_queue<DelayedTask, std::deque<DelayedTask>, DelayedTaskCompare>;
 
-  MessageLoop::TaskObserver task_observer_;
+  std::set<TaskObserver*> 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
@@ -238,6 +238,22 @@ TEST(MessageLoop, TIME_SENSITIVE(MultipleDelayedTasksWithDecreasingDeltas)) {
   ASSERT_EQ(checked, count);
 }
 
+class CustomTaskObserver : public fml::TaskObserver {
+ public:
+  CustomTaskObserver(std::function<void()> lambda) : lambda_(lambda){};
+
+  ~CustomTaskObserver() override = default;
+
+  void DidProcessTask() {
+    if (lambda_) {
+      lambda_();
+    }
+  };
+
+ private:
+  std::function<void()> 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
@@ -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
@@ -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
@@ -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