Create a WeakPtrFactory for use on the UI thread in VsyncWaiter (#13781)

Author: George Wright

ci/licenses_golden/licenses_flutter

@@ -1002,6 +1002,7 @@ FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_recorder.cc
 FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_recorder.h
 FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter.cc
 FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter.h
+FILE: ../../../flutter/shell/platform/fuchsia/flutter/vsync_waiter_unittests.cc
 FILE: ../../../flutter/shell/platform/fuchsia/flutter/vulkan_surface.cc
 FILE: ../../../flutter/shell/platform/fuchsia/flutter/vulkan_surface.h
 FILE: ../../../flutter/shell/platform/fuchsia/flutter/vulkan_surface_pool.cc

shell/platform/fuchsia/flutter/BUILD.gn

@@ -260,15 +260,24 @@ executable("flutter_runner_unittests") {
     "fuchsia_intl.h",
     "fuchsia_intl_unittest.cc",
     "logging.h",
+    "loop.cc",
+    "loop.h",
     "platform_view.cc",
     "platform_view.h",
     "platform_view_unittest.cc",
     "surface.cc",
     "surface.h",
+    "task_observers.cc",
+    "task_observers.h",
+    "task_runner_adapter.cc",
+    "task_runner_adapter.h",
+    "thread.cc",
+    "thread.h",
     "vsync_recorder.cc",
     "vsync_recorder.h",
     "vsync_waiter.cc",
     "vsync_waiter.h",
+    "vsync_waiter_unittests.cc",
   ]
 
   # This is needed for //third_party/googletest for linking zircon symbols.

shell/platform/fuchsia/flutter/vsync_waiter.cc

@@ -5,6 +5,8 @@
 #include "vsync_waiter.h"
 
 #include <lib/async/default.h>
+#include "flutter/fml/make_copyable.h"
+#include "flutter/fml/synchronization/waitable_event.h"
 #include "flutter/fml/trace_event.h"
 
 #include "vsync_recorder.h"
@@ -17,7 +19,8 @@ VsyncWaiter::VsyncWaiter(std::string debug_label,
     : flutter::VsyncWaiter(task_runners),
       debug_label_(std::move(debug_label)),
       session_wait_(session_present_handle, SessionPresentSignal),
-      weak_factory_(this) {
+      weak_factory_(this),
+      weak_factory_ui_(nullptr) {
   auto wait_handler = [&](async_dispatcher_t* dispatcher,   //
                           async::Wait* wait,                //
                           zx_status_t status,               //
@@ -33,11 +36,29 @@ VsyncWaiter::VsyncWaiter(std::string debug_label,
     FireCallbackNow();
   };
 
+  // Generate a WeakPtrFactory for use with the UI thread. This does not need
+  // to wait on a latch because we only ever use the WeakPtrFactory on the UI
+  // thread so we have ordering guarantees (see ::AwaitVSync())
+  fml::TaskRunner::RunNowOrPostTask(
+      task_runners_.GetUITaskRunner(), fml::MakeCopyable([this]() mutable {
+        this->weak_factory_ui_ =
+            std::make_unique<fml::WeakPtrFactory<VsyncWaiter>>(this);
+      }));
   session_wait_.set_handler(wait_handler);
 }
 
 VsyncWaiter::~VsyncWaiter() {
   session_wait_.Cancel();
+
+  fml::AutoResetWaitableEvent ui_latch;
+  fml::TaskRunner::RunNowOrPostTask(
+      task_runners_.GetUITaskRunner(),
+      fml::MakeCopyable(
+          [weak_factory_ui = std::move(weak_factory_ui_), &ui_latch]() mutable {
+            weak_factory_ui.reset();
+            ui_latch.Signal();
+          }));
+  ui_latch.Wait();
 }
 
 static fml::TimePoint SnapToNextPhase(fml::TimePoint value,
@@ -57,7 +78,13 @@ void VsyncWaiter::AwaitVSync() {
   fml::TimePoint next_vsync = SnapToNextPhase(now, vsync_info.presentation_time,
                                               vsync_info.presentation_interval);
   task_runners_.GetUITaskRunner()->PostDelayedTask(
-      [self = weak_factory_.GetWeakPtr()] {
+      [& weak_factory_ui = this->weak_factory_ui_] {
+        if (!weak_factory_ui) {
+          FML_LOG(WARNING) << "WeakPtrFactory for VsyncWaiter is null, likely "
+                              "due to the VsyncWaiter being destroyed.";
+          return;
+        }
+        auto self = weak_factory_ui->GetWeakPtr();
         if (self) {
           self->FireCallbackWhenSessionAvailable();
         }

shell/platform/fuchsia/flutter/vsync_waiter.h

@@ -29,6 +29,10 @@ class VsyncWaiter final : public flutter::VsyncWaiter {
   async::Wait session_wait_;
   fml::WeakPtrFactory<VsyncWaiter> weak_factory_;
 
+  // For accessing the VsyncWaiter via the UI thread, necessary for the callback
+  // for AwaitVSync()
+  std::unique_ptr<fml::WeakPtrFactory<VsyncWaiter>> weak_factory_ui_;
+
   // |flutter::VsyncWaiter|
   void AwaitVSync() override;
 

shell/platform/fuchsia/flutter/vsync_waiter_unittests.cc

@@ -0,0 +1,87 @@
+// 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.
+
+#include <array>
+
+#include <gtest/gtest.h>
+#include <lib/zx/event.h>
+#include <zircon/syscalls.h>
+
+#include "flutter/fml/synchronization/waitable_event.h"
+#include "flutter/shell/common/thread_host.h"
+#include "flutter/shell/common/vsync_waiter.h"
+#include "flutter/shell/platform/fuchsia/flutter/task_runner_adapter.h"
+#include "flutter/shell/platform/fuchsia/flutter/thread.h"
+#include "flutter/shell/platform/fuchsia/flutter/vsync_waiter.h"
+
+namespace flutter_runner_test {
+
+class VsyncWaiterTest : public testing::Test {
+ public:
+  VsyncWaiterTest() {}
+
+  ~VsyncWaiterTest() = default;
+
+  std::unique_ptr<flutter::VsyncWaiter> CreateVsyncWaiter(
+      flutter::TaskRunners task_runners) {
+    return std::make_unique<flutter_runner::VsyncWaiter>(
+        "VsyncWaiterTest", vsync_event_.get(), task_runners);
+  }
+
+  void SignalVsyncEvent() {
+    auto status =
+        zx_object_signal(vsync_event_.get(), 0,
+                         flutter_runner::VsyncWaiter::SessionPresentSignal);
+    EXPECT_EQ(status, ZX_OK);
+  }
+
+ protected:
+  void SetUp() override {
+    auto status = zx::event::create(0, &vsync_event_);
+    EXPECT_EQ(status, ZX_OK);
+  }
+
+ private:
+  zx::event vsync_event_;
+};
+
+TEST_F(VsyncWaiterTest, AwaitVsync) {
+  std::array<std::unique_ptr<flutter_runner::Thread>, 3> threads;
+
+  for (auto& thread : threads) {
+    thread.reset(new flutter_runner::Thread());
+  }
+
+  async::Loop loop(&kAsyncLoopConfigAttachToThread);
+
+  const flutter::TaskRunners task_runners(
+      "VsyncWaiterTests",  // Dart thread labels
+      flutter_runner::CreateFMLTaskRunner(
+          async_get_default_dispatcher()),  // platform
+      flutter_runner::CreateFMLTaskRunner(threads[0]->dispatcher()),  // gpu
+      flutter_runner::CreateFMLTaskRunner(threads[1]->dispatcher()),  // ui
+      flutter_runner::CreateFMLTaskRunner(threads[2]->dispatcher())   // io
+  );
+
+  auto vsync_waiter = CreateVsyncWaiter(std::move(task_runners));
+
+  fml::AutoResetWaitableEvent latch;
+  vsync_waiter->AsyncWaitForVsync(
+      [&latch](fml::TimePoint frame_start_time,
+               fml::TimePoint frame_target_time) { latch.Signal(); });
+  SignalVsyncEvent();
+
+  bool did_timeout =
+      latch.WaitWithTimeout(fml::TimeDelta::FromMilliseconds(5000));
+
+  // False indicates we were signalled rather than timed out
+  EXPECT_FALSE(did_timeout);
+
+  vsync_waiter.reset();
+  for (const auto& thread : threads) {
+    thread->Quit();
+  }
+}
+
+}  // namespace flutter_runner_test